Skip to content

fix: normalize same-origin absolute URLs for client-side navigation#336

Merged
james-elicx merged 3 commits intocloudflare:mainfrom
Divkix:fix/link-same-origin-absolute-url
Mar 8, 2026
Merged

fix: normalize same-origin absolute URLs for client-side navigation#336
james-elicx merged 3 commits intocloudflare:mainfrom
Divkix:fix/link-same-origin-absolute-url

Conversation

@Divkix
Copy link
Contributor

@Divkix Divkix commented Mar 8, 2026

Summary

Fixes #335

  • Adds toSameOriginPath() utility that extracts the local path (pathname + search + hash) from a same-origin absolute URL, returning null for truly external URLs or server-side contexts
  • Updates link.tsx handleClick() and prefetch paths to normalize same-origin URLs before the external URL bail-out
  • Updates navigation.ts navigateImpl() (App Router) to normalize before external URL check
  • Updates router.ts push()/replace() in both the useRouter hook and Router singleton (Pages Router)

Root Cause

isExternalUrl() treated any URL with a scheme as external — no same-origin comparison. When <Link href="http://localhost:3000/about"> was used and the origin matched, it triggered a full page reload instead of SPA navigation.

Test plan

  • Unit tests for toSameOriginPath() — same-origin returns local path, cross-origin returns null, server returns null, preserves search/hash
  • SSR rendering tests for <Link> with absolute URLs
  • isExternalUrl() still correctly identifies truly external URLs
  • pnpm test tests/link.test.ts — 35 tests pass
  • pnpm test tests/shims.test.ts — 568 tests pass
  • pnpm run typecheck — clean
  • pnpm run lint — clean

When <Link href="https://example.com/path"> is used and example.com
matches the current origin, vinext now does SPA navigation instead
of a full page reload. This matches Next.js behavior.

Adds toSameOriginPath() utility that extracts the local path from a
same-origin absolute URL. Applied consistently across:
- link.tsx handleClick() and prefetch paths
- navigation.ts navigateImpl() (App Router)
- router.ts push()/replace() (Pages Router hook + singleton)

Closes cloudflare#335
@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 8, 2026

Open in StackBlitz

npm i https://pkg.pr.new/vinext@336

commit: 87bccd7

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes client-side navigation for same-origin absolute URLs (e.g. http://localhost:3000/about) by normalizing them to local paths before “external URL” bail-outs, aligning behavior with Next.js and resolving #335.

Changes:

  • Added toSameOriginPath() utility to convert same-origin absolute URLs to pathname + search + hash (or return null for external/server contexts).
  • Updated Link, App Router navigation, and Pages Router push/replace to normalize same-origin absolute URLs before delegating to full-page navigation.
  • Added unit tests for toSameOriginPath() and SSR rendering checks for <Link> with absolute URLs.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/link.test.ts Adds tests for same-origin normalization utility and SSR output for absolute hrefs.
packages/vinext/src/shims/url-utils.ts Introduces shared toSameOriginPath() helper used by multiple navigation shims.
packages/vinext/src/shims/link.tsx Normalizes same-origin absolute URLs for prefetch/click handling to enable SPA navigation.
packages/vinext/src/shims/navigation.ts Normalizes same-origin absolute URLs before external URL handling in App Router navigation.
packages/vinext/src/shims/router.ts Normalizes same-origin absolute URLs before external URL handling in Pages Router push/replace.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Use window.location.origin as base for `//` URLs so they resolve
correctly for same-origin detection, while keeping `new URL(url)`
(no base) for http/https URLs to preserve existing behavior.

Addresses Copilot review comment on PR cloudflare#336.
@james-elicx
Copy link
Collaborator

/bigbonk review

Copy link
Contributor

@ask-bonk ask-bonk bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-scoped fix. The toSameOriginPath() utility is correct and the integration across all navigation paths (Link, App Router, Pages Router hook + singleton) is thorough and consistent.

All tests pass (36 link tests, 568 shim tests), typecheck and lint are clean.

Two minor observations below — neither blocks merge.

if (!observer) return;

observerCallbacks.set(node, () => prefetchUrl(localizedHref));
observerCallbacks.set(node, () => prefetchUrl(hrefToPrefetch));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: prefetchUrl() already normalizes same-origin URLs internally (lines 150-156). When the IntersectionObserver fires this callback, toSameOriginPath will be called a second time inside prefetchUrl. This is harmless (cheap, idempotent, correct) but the double normalization could be avoided by passing hrefToPrefetch here, which you already do — so it only re-normalizes if the href was already local. Just noting the interaction; no change needed.

// External links: let the browser handle it.
// Same-origin absolute URLs (e.g. http://localhost:3000/about) are
// normalized to local paths so they get client-side navigation.
let navigateHref = resolvedHref;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice detail: using resolvedHref (which incorporates the as prop) for the external check, and then feeding the normalized path into resolveRelativeHref / withBasePath downstream. The as-prop case is handled correctly because as would typically be a local path, but if someone passes an absolute URL as as, this now does the right thing.

@ask-bonk
Copy link
Contributor

ask-bonk bot commented Mar 8, 2026

Review submitted. Here's a summary of my findings:

Approved. This is a clean, well-scoped fix for #335.

What the PR does: Adds a toSameOriginPath() utility that uses new URL() + origin comparison to detect same-origin absolute URLs and extract their local path. This is applied consistently in all 4 navigation code paths: Link.handleClick(), Link prefetch (both IntersectionObserver setup and prefetchUrl()), App Router navigateImpl(), and Pages Router push/replace (both useRouter hook and Router singleton).

What I verified:

  • All 36 link tests and 568 shim tests pass
  • Typecheck clean, lint clean
  • The toSameOriginPath() implementation correctly handles: same-origin http/https, protocol-relative URLs (using window.location.origin as base for // URLs only), cross-origin, different ports, different schemes, server-side (returns null), and invalid URLs
  • SSR rendering correctly preserves the absolute URL in the href attribute (normalization only happens at click/navigate time, not render time)

One minor suggestion: The test comment on line 247 of tests/link.test.ts references anthropics/vinext instead of cloudflare/vinext — submitted as an inline suggestion.

github run

Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>
@james-elicx james-elicx enabled auto-merge (squash) March 8, 2026 08:51
@james-elicx
Copy link
Collaborator

Nice one!

@james-elicx james-elicx merged commit 6762472 into cloudflare:main Mar 8, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected next/link Behavior: <Link> with same-origin absolute URL triggers full page reload

3 participants