fix(link): preserve native URI scheme navigation#1268
Conversation
Link treated only http, https, and protocol-relative hrefs as browser-owned. Native schemes such as mailto:, tel:, and sms: were therefore locale-prefixed, eligible for prefetch normalization, and intercepted by client navigation. The URL boundary now mirrors Next.js absolute URL scheme classification while preserving vinext's protocol-relative handling. Link rendering, click interception, relative resolution, and prefetch normalization all share that classification so non-local schemes stay with the browser while same-origin absolute URLs remain routable. Adds focused regression coverage for native scheme rendering, click handling, and prefetch decisions.
commit: |
Router and navigation shims already had local external URL predicates with the same browser-owned URL semantics now used by Link. Reuse the shared url-utils predicate for Pages Router locale handling, Pages Router external detection, App Router external detection, and App Router prefetch normalization. Add focused shim coverage for native URI schemes through the Pages Router helpers.
|
/bigbonk review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean, well-scoped fix. The shared predicate correctly mirrors Next.js's isAbsoluteUrl (same regex, same charCode fast-path), and the consolidation across Link, router, navigation, and prefetch eliminates the classification drift that caused the bug.
I verified:
- Regex parity:
ABSOLUTE_URL_REGEXand theisAbsoluteUrlimplementation match Next.js'spackages/next/src/shared/lib/utils.tsexactly. resolveRelativeHreffix: This is where the core bug lived — the old code fell through tonew URL('mailto:...', base)which stripped the scheme and returned justhello@example.comas a relative path. The new early-return correctly preserves the full URI.withBasePathequivalence: The!path.startsWith("/")guard already excludedmailto:,tel:, etc., so the refactoring is semantically equivalent. TheisAbsoluteOrProtocolRelativeUrlcheck only matters for//protocol-relative URLs (which do start with/).- Click handler flow:
isAbsoluteOrProtocolRelativeUrl → toSameOriginAppPath → nullcorrectly returns beforepreventDefault(), letting the browser handle native schemes. - Prefetch flow:
new URL('mailto:...')parses withorigin: null, which fails the same-origin check, sogetLinkPrefetchHrefreturnsnullas expected. - Test coverage: Tests cover render output, click interception, prefetch normalization, locale handling, and router helper behavior across all three native schemes.
One minor observation below, but it doesn't block merge.
| path.startsWith("https://") || | ||
| path.startsWith("//") | ||
| ) { | ||
| if (!basePath || !path.startsWith("/") || isAbsoluteOrProtocolRelativeUrl(path)) { |
There was a problem hiding this comment.
Nit: since path.startsWith("/") is checked first, the isAbsoluteOrProtocolRelativeUrl(path) call can only ever match protocol-relative // paths — isAbsoluteUrl requires the first character to be a letter, which conflicts with /. So this is correct and equivalent to the old inline check, but the reader has to reason through two guards to see it. Might be worth a one-line comment like // only protocol-relative "//" can reach here; absolute schemes don't start with "/" to save the next person the mental detour. Not blocking.
|
Review submitted. Here's a summary: Approved the PR with one minor nit. The fix is correct and well-structured. The core bug was in The only comment was a readability nit on |
2 participants
Overview
packages/vinext/src/shims/url-utils.ts,packages/vinext/src/shims/link.tsx,packages/vinext/src/shims/link-prefetch.ts,packages/vinext/src/shims/router.ts,packages/vinext/src/shims/navigation.ts<Link href="mailto:...">,tel:,sms:, and other non-local schemes preserve native browser behavior while same-origin absolute URLs still use client navigation.Why
next/linkshould only intercept hrefs the framework router can actually route. Next.js treats scheme-bearing URLs as absolute, then uses local URL detection to decide whether the router owns them. Native schemes such asmailto:,tel:, andsms:are not local app routes, so preventing the browser default turns a valid native action into a broken client navigation.preventDefault()or RSC navigation.mailto:hello@example.cominto/fr/mailto:hello@example.com.null, so intent/viewport prefetch cannot fetch a mangled local target.url-utilspredicate instead of local regex copies.What Changed
<Link href="mailto:hello@example.com">clicklocale="fr"with native schemeMaintainer review path
packages/vinext/src/shims/url-utils.ts: review the shared absolute URL predicate and its use in basePath/relative resolution.packages/vinext/src/shims/link.tsx: review the locale and click-interception call sites that now use the shared predicate.packages/vinext/src/shims/link-prefetch.ts: review native scheme exclusion from prefetch normalization.packages/vinext/src/shims/router.tsandpackages/vinext/src/shims/navigation.ts: review reuse of the shared predicate in router/navigation helpers.tests/link.test.ts,tests/link-navigation.test.ts, andtests/shims.test.ts: review regression coverage for render output, click interception, prefetch behavior, and router helper behavior.Validation
vp test run tests/link.test.ts tests/link-navigation.test.ts tests/shims.test.tspassed: 988 tests.vp checkpassed: formatting, lint, and type checks clean.vp check --fixandknipsuccessfully.Risk / compatibility
isDangerousSchemepath before anchor rendering.Non-goals
config-matchers.ts.asvalidation parity beyond preserving the currentas ?? hrefbehavior.References
isAbsoluteUrlisLocalURLmailto:is valid Link href output and should not report an error.mailto:,tel:, andsms:are not valid relative router URLs.