fix(link): use localized href for SPA navigation#367
fix(link): use localized href for SPA navigation#367james-elicx merged 1 commit intocloudflare:mainfrom
Conversation
The locale prop correctly affected the rendered <a> href and prefetch path, but click navigation and the onNavigate callback used the non-localized resolvedHref. This meant clicking <Link href="/about" locale="fr"> would SPA-navigate to /about instead of /fr/about.
commit: |
|
/bigbonk review this pr please |
There was a problem hiding this comment.
Clean, correct fix. The bug is clear: localizedHref was already used for the rendered <a href> and the prefetch path, but the click handler and onNavigate callback were using resolvedHref (the non-localized version), causing SPA navigation to go to the wrong path when locale was specified.
The two-line change is well-scoped and consistent with how localizedHref is already used elsewhere in the component.
One minor note (not blocking): the external URL branch on lines 372-378 still calls toSameOriginPath(resolvedHref) rather than toSameOriginPath(localizedHref). This is actually fine for the common case (locale prefixes are applied to relative paths like /about, not absolute URLs), but applyLocaleToHref doesn't handle absolute URLs gracefully — it would produce something like /fr/http://localhost:3000/about. That's a pre-existing issue unrelated to this PR.
|
Review submitted — approved the PR. Summary: This is a clean two-line fix for a real bug where |
Summary
next/link'slocaleprop correctly affected the rendered<a>href and prefetch path, but SPA click navigation and theonNavigatecallback used the non-localizedresolvedHref<Link href="/about" locale="fr">would render<a href="/fr/about">and prefetch/fr/about, but SPA-navigate to/aboutlocalizedHrefinstead ofresolvedHrefin the click handler (line 370) andonNavigatecallback (line 386)Test plan
link.test.tslocale tests pass (36/36)