From 02572212bf8b641621c5520ee21e00d9ebdcc940 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Thu, 18 Jun 2026 01:34:32 +1000 Subject: [PATCH 1/5] fix(router): honor history state url on popstate Pages Router popstate handling previously derived the route to load from window.location. That loses the route URL stored in Next-shaped history state when url and as differ, so a stale Safari replay is ignored correctly but the next identical popstate still reloads the visible static route instead of the dynamic page route. Use state.as as the visible URL and state.url as the route fetch target for masked popstate entries, then force the HTML navigation path so data fetching is not keyed by the browser URL. Port the upstream Next.js ignore-invalid-popstateevent without-i18n coverage to lock in the stale-state behaviour. --- packages/vinext/src/shims/router.ts | 84 +++++++++++-- tests/pages-router-i18n-sticky-locale.test.ts | 112 ++++++++++++++++++ 2 files changed, 185 insertions(+), 11 deletions(-) diff --git a/packages/vinext/src/shims/router.ts b/packages/vinext/src/shims/router.ts index 263f8d0b1..5d8168181 100644 --- a/packages/vinext/src/shims/router.ts +++ b/packages/vinext/src/shims/router.ts @@ -1438,6 +1438,13 @@ function scheduleHardNavigationAndThrow(url: string, message: string): never { type NavigateClientOptions = { allowNotFoundResponse?: boolean; + /** + * Popstate can carry a Next.js history state whose `url` is the page route + * to load while `as` is the visible browser URL. The JSON fast path keys off + * the browser URL, so masked popstate transitions must use the HTML path + * against the explicit route URL instead. + */ + forceHtmlFetch?: boolean; /** * The history mode of the originating navigation. Used when a gSSP/gSP data * response carries a `__N_REDIRECT` marker so the re-entrant navigation to @@ -2071,7 +2078,7 @@ async function navigateClient( // data endpoint. Skip data navigation and middleware-redirect probing // (both would target the fictional masked URL) and fetch the resolved // error HTML directly, allowing a 404 response to hydrate. - if (options.allowNotFoundResponse === true) { + if (options.allowNotFoundResponse === true || options.forceHtmlFetch === true) { await navigateClientHtml(url, fetchUrl, controller, navId, assertStillCurrent, options); } else { let browserUrl = url; @@ -2698,10 +2705,55 @@ function getRouterStateKey(state: unknown): string | undefined { return typeof state.key === "string" ? state.key : undefined; } -function handlePagesRouterPopState(e: PopStateEvent): void { - const browserUrl = window.location.pathname + window.location.search; - const appUrl = stripBasePath(window.location.pathname, __basePath) + window.location.search; +type PopStatePath = { + browserUrl: string; + appUrl: string; + hash: string; +}; + +type PopStateNavigationTarget = { + visible: PopStatePath; + route: PopStatePath; + isMaskedRoute: boolean; +}; + +function resolvePopStatePath(path: string): PopStatePath | null { + try { + const href = normalizePathTrailingSlash( + toBrowserNavigationHref(path, window.location.href, __basePath), + __trailingSlash, + ); + const parsed = new URL(href, window.location.href); + const origin = getWindowOrigin(); + if (origin && parsed.origin !== origin) return null; + return { + browserUrl: parsed.pathname + parsed.search, + appUrl: stripBasePath(parsed.pathname, __basePath) + parsed.search, + hash: parsed.hash, + }; + } catch { + return null; + } +} + +function resolvePopStateNavigationTarget(state: unknown): PopStateNavigationTarget | null { + if (!isNextRouterState(state) || typeof state.as !== "string" || typeof state.url !== "string") { + return null; + } + + const visible = resolvePopStatePath(state.as); + const route = resolvePopStatePath(state.url); + if (!visible || !route) return null; + + return { + visible, + route, + isMaskedRoute: visible.browserUrl !== route.browserUrl, + }; +} + +function handlePagesRouterPopState(e: PopStateEvent): void { const state = e.state as unknown; const wasFirst = routerRuntimeState.isFirstPopStateEvent; routerRuntimeState.isFirstPopStateEvent = false; @@ -2753,8 +2805,18 @@ function handlePagesRouterPopState(e: PopStateEvent): void { } } + const stateTarget = resolvePopStateNavigationTarget(state); + const browserUrl = + stateTarget?.visible.browserUrl ?? window.location.pathname + window.location.search; + const appUrl = + stateTarget?.visible.appUrl ?? + stripBasePath(window.location.pathname, __basePath) + window.location.search; + const routeBrowserUrl = stateTarget?.route.browserUrl ?? browserUrl; + const visibleHash = stateTarget?.visible.hash || window.location.hash; + // Detect hash-only back/forward: pathname+search unchanged, only hash differs. - const isHashOnly = browserUrl === routerRuntimeState.lastPathnameAndSearch; + const isHashOnly = + stateTarget?.isMaskedRoute !== true && browserUrl === routerRuntimeState.lastPathnameAndSearch; const targetKey = getRouterStateKey(state); let forcedScroll: ScrollPosition | undefined; @@ -2781,7 +2843,7 @@ function handlePagesRouterPopState(e: PopStateEvent): void { // Check beforePopState callback if (routerRuntimeState.beforePopStateCb !== undefined) { const shouldContinue = routerRuntimeState.beforePopStateCb({ - url: appUrl, + url: stateTarget?.route.appUrl ?? appUrl, as: appUrl, options: { shallow: false }, }); @@ -2810,9 +2872,9 @@ function handlePagesRouterPopState(e: PopStateEvent): void { // path (.nextjs-ref/packages/next/src/shared/lib/router/router.ts around // L1381-1403 and L1780). The snapshot stays in sessionStorage, so a later // non-hash popstate to this entry still restores the saved position. - const hashUrl = appUrl + window.location.hash; + const hashUrl = appUrl + visibleHash; routerEvents.emit("hashChangeStart", hashUrl, { shallow: false }); - scrollToHashTarget(window.location.hash); + scrollToHashTarget(visibleHash); routerEvents.emit("hashChangeComplete", hashUrl, { shallow: false }); dispatchNavigateEvent(); return; @@ -2824,7 +2886,7 @@ function handlePagesRouterPopState(e: PopStateEvent): void { const stateLocale = isNextRouterState(state) ? state.options?.locale : undefined; const effectiveLocale = stateLocale ?? window.__VINEXT_LOCALE__; - const fullAppUrl = appUrl + window.location.hash; + const fullAppUrl = appUrl + visibleHash; routerEvents.emit("routeChangeStart", fullAppUrl, { shallow: false }); // Note: The browser has already updated window.location by the time popstate // fires, so this is not truly "before" the URL change. In Next.js the popstate @@ -2851,8 +2913,8 @@ function handlePagesRouterPopState(e: PopStateEvent): void { const result = await runNavigateClient( browserUrl, fullAppUrl, - getPagesHtmlFetchUrl(browserUrl, effectiveLocale), - { scroll: scrollTarget }, + getPagesHtmlFetchUrl(routeBrowserUrl, effectiveLocale), + { scroll: scrollTarget, forceHtmlFetch: stateTarget?.isMaskedRoute === true }, ); if (result === "completed") { routerEvents.emit("routeChangeComplete", fullAppUrl, { shallow: false }); diff --git a/tests/pages-router-i18n-sticky-locale.test.ts b/tests/pages-router-i18n-sticky-locale.test.ts index 326152bce..665b73931 100644 --- a/tests/pages-router-i18n-sticky-locale.test.ts +++ b/tests/pages-router-i18n-sticky-locale.test.ts @@ -114,6 +114,118 @@ function buildNavHtml( const PAGE_MODULE_URL = path.resolve(import.meta.dirname, "fixtures/client-navigation-page.tsx"); +// Ported from Next.js: +// test/e2e/ignore-invalid-popstateevent/without-i18n.test.ts +// https://github.com/vercel/next.js/blob/canary/test/e2e/ignore-invalid-popstateevent/without-i18n.test.ts +describe("Pages Router popstate stale-state filter (non-i18n parity)", () => { + async function installRuntime(win: ReturnType["win"]) { + const listeners = new Map void>(); + win.addEventListener = vi.fn((type: string, handler: (event: any) => void) => { + listeners.set(type, handler); + }) as any; + (globalThis as any).window = win; + vi.resetModules(); + await import("../packages/vinext/src/shims/router.js"); + const { installPagesRouterRuntime } = + await import("../packages/vinext/src/shims/pages-router-runtime.js"); + installPagesRouterRuntime(); + return listeners; + } + + async function expectFirstStalePopstateIgnoredThenSecondUsesStateUrl({ + initialPath, + state, + expectedFetchUrl, + }: { + initialPath: string; + state: { url: string; as: string; options: {}; __N: true; key: string }; + expectedFetchUrl: string; + }) { + const previousWindow = (globalThis as any).window; + const originalFetch = globalThis.fetch; + const originalCustomEvent = globalThis.CustomEvent; + const { win } = createNavWindow(); + const initialUrl = new URL(initialPath, "http://localhost"); + win.location.pathname = initialUrl.pathname; + win.location.search = initialUrl.search; + win.location.href = initialUrl.href; + win.__NEXT_DATA__ = { + ...win.__NEXT_DATA__, + page: "/static", + __vinext: { pageModuleUrl: "/@fs/pages/static.js" }, + }; + + let routeChangeStartCount = 0; + const fetchMock = vi.fn<(input: RequestInfo | URL, init?: RequestInit) => Promise>( + async () => new Response(buildNavHtml("/[dynamic]", PAGE_MODULE_URL), { status: 200 }), + ); + globalThis.fetch = fetchMock; + (globalThis as any).CustomEvent = class CustomEventMock { + constructor(public type: string) {} + } as any; + + try { + const listeners = await installRuntime(win); + const popstateHandler = listeners.get("popstate"); + expect(popstateHandler).toBeDefined(); + + const routerModule = await import("../packages/vinext/src/shims/router.js"); + routerModule.default.events.on("routeChangeStart", () => { + routeChangeStartCount += 1; + }); + + popstateHandler!({ state }); + await new Promise((r) => setTimeout(r, 0)); + expect(fetchMock).not.toHaveBeenCalled(); + expect(routeChangeStartCount).toBe(0); + expect(win.__NEXT_DATA__.page).toBe("/static"); + + popstateHandler!({ state }); + await vi.waitFor(() => expect(win.__NEXT_DATA__.page).toBe("/[dynamic]")); + expect(routeChangeStartCount).toBe(1); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fetchMock.mock.calls[0]![0]).toBe(expectedFetchUrl); + } finally { + vi.resetModules(); + if (previousWindow === undefined) { + delete (globalThis as any).window; + } else { + (globalThis as any).window = previousWindow; + } + globalThis.fetch = originalFetch; + (globalThis as any).CustomEvent = originalCustomEvent; + } + } + + it("ignores the first stale event without query params and processes the second", async () => { + await expectFirstStalePopstateIgnoredThenSecondUsesStateUrl({ + initialPath: "/static", + state: { + url: "/[dynamic]?", + as: "/static", + options: {}, + __N: true, + key: "", + }, + expectedFetchUrl: "/[dynamic]", + }); + }); + + it("ignores the first stale event with query params and processes the second", async () => { + await expectFirstStalePopstateIgnoredThenSecondUsesStateUrl({ + initialPath: "/static?param=1", + state: { + url: "/[dynamic]?param=1", + as: "/static?param=1", + options: {}, + __N: true, + key: "", + }, + expectedFetchUrl: "/[dynamic]?param=1", + }); + }); +}); + // Ported from Next.js test/e2e/ignore-invalid-popstateevent — Next.js writes // `{ url, as, options, __N: true, key }` on every pushState/replaceState so // the popstate handler can detect stale or non-Next events. From c5e09722d8bd65865bdcc75c3e0ed4eda5104855 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Fri, 19 Jun 2026 15:50:34 +1000 Subject: [PATCH 2/5] test(router): type masked-popstate fixtures The new i18n / basePath popstate tests typed their fetch mock and history state loosely, so `vp check` flagged the `mock.calls[0]` index and the state literals carried `any`. Type the fetch mock with its call signature, and introduce `MaskedHistoryState` / `PopStateListener` aliases so the ported popstate fixtures and the handler invocations are checked. --- tests/pages-router-i18n-sticky-locale.test.ts | 45 ++++++++++++------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/tests/pages-router-i18n-sticky-locale.test.ts b/tests/pages-router-i18n-sticky-locale.test.ts index 5a200ca6f..fd437b4af 100644 --- a/tests/pages-router-i18n-sticky-locale.test.ts +++ b/tests/pages-router-i18n-sticky-locale.test.ts @@ -114,13 +114,28 @@ function buildNavHtml( const PAGE_MODULE_URL = path.resolve(import.meta.dirname, "fixtures/client-navigation-page.tsx"); +// Router-shaped history state as Next.js writes it on push/replace +// (`{ url, as, options, __N, key }`). `url` is the route, `as` the address bar; +// a masked entry has `url !== as`. +type MaskedHistoryState = { + url: string; + as: string; + options: { locale?: string; shallow?: boolean }; + __N: true; + key: string; +}; + +// The popstate handler the runtime registers via `addEventListener`. The tests +// only ever drive it with a `{ state }` payload, so this is the honest surface. +type PopStateListener = (event: { state: unknown }) => void; + // Ported from Next.js: // test/e2e/ignore-invalid-popstateevent/without-i18n.test.ts // https://github.com/vercel/next.js/blob/canary/test/e2e/ignore-invalid-popstateevent/without-i18n.test.ts describe("Pages Router popstate stale-state filter (non-i18n parity)", () => { async function installRuntime(win: ReturnType["win"]) { - const listeners = new Map void>(); - win.addEventListener = vi.fn((type: string, handler: (event: any) => void) => { + const listeners = new Map(); + win.addEventListener = vi.fn((type: string, handler: PopStateListener) => { listeners.set(type, handler); }) as any; (globalThis as any).window = win; @@ -138,7 +153,7 @@ describe("Pages Router popstate stale-state filter (non-i18n parity)", () => { expectedFetchUrl, }: { initialPath: string; - state: { url: string; as: string; options: {}; __N: true; key: string }; + state: MaskedHistoryState; expectedFetchUrl: string; }) { const previousWindow = (globalThis as any).window; @@ -388,8 +403,8 @@ describe("Pages Router history state shape", () => { // https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/router.ts (around L935-942) describe("Pages Router popstate stale-state filter (i18n parity)", () => { async function installRuntime(win: ReturnType["win"]) { - const listeners = new Map void>(); - win.addEventListener = vi.fn((type: string, handler: (event: any) => void) => { + const listeners = new Map(); + win.addEventListener = vi.fn((type: string, handler: PopStateListener) => { listeners.set(type, handler); }) as any; (globalThis as any).window = win; @@ -496,7 +511,7 @@ describe("Pages Router popstate stale-state filter (i18n parity)", () => { }); let routeChangeStartCount = 0; - const fetchMock = vi.fn( + const fetchMock = vi.fn<(input: RequestInfo | URL, init?: RequestInit) => Promise>( async () => new Response( buildNavHtml( @@ -513,11 +528,11 @@ describe("Pages Router popstate stale-state filter (i18n parity)", () => { constructor(public type: string) {} } as any; - const state = { + const state: MaskedHistoryState = { url: "/[dynamic]?", as: "/static", options: { locale: "sv" }, - __N: true as const, + __N: true, key: "", }; @@ -702,7 +717,7 @@ describe("Pages Router popstate masked route under basePath", () => { __vinext: { pageModuleUrl: "/@fs/pages/static.js" }, }; - const fetchMock = vi.fn( + const fetchMock = vi.fn<(input: RequestInfo | URL, init?: RequestInit) => Promise>( async () => new Response(buildNavHtml("/[dynamic]", PAGE_MODULE_URL), { status: 200 }), ); globalThis.fetch = fetchMock; @@ -710,17 +725,17 @@ describe("Pages Router popstate masked route under basePath", () => { constructor(public type: string) {} } as any; - const listeners = new Map void>(); - win.addEventListener = vi.fn((type: string, handler: (event: any) => void) => { + const listeners = new Map(); + win.addEventListener = vi.fn((type: string, handler: PopStateListener) => { listeners.set(type, handler); }) as any; (globalThis as any).window = win; - const state = { + const state: MaskedHistoryState = { url: "/[dynamic]?", as: "/static", options: {}, - __N: true as const, + __N: true, key: "", }; @@ -823,8 +838,8 @@ describe("Pages Router locale stickiness on programmatic navigation", () => { // to close that gap. describe("Pages Router initial-entry history state", () => { async function installRuntime(win: ReturnType["win"]) { - const listeners = new Map void>(); - win.addEventListener = vi.fn((type: string, handler: (event: any) => void) => { + const listeners = new Map(); + win.addEventListener = vi.fn((type: string, handler: PopStateListener) => { listeners.set(type, handler); }) as any; (globalThis as any).window = win; From 7fc0797ae16989ef687367735d43cb91e577cb34 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Fri, 19 Jun 2026 16:18:08 +1000 Subject: [PATCH 3/5] test(router): cover masked popstate through the real write path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The masked-popstate tests construct `MaskedHistoryState` by hand and drive the captured popstate handler in isolation. That proves the consumer reads `state.url`, but nothing exercises the writer, so a future change to `updateHistory`/`Router.push` that stopped emitting `url` distinct from `as` would pass every existing test while breaking masked forward-navigation. Add a producer→consumer integration test: a masked `Router.push(href, as)` writes real history state via `updateHistory`, and that exact captured object is replayed through popstate. It asserts the producer wrote `url !== as` and the consumer fetched `state.url` (the route), not `state.as` (the address bar). The test reproduces the PR's bug through the real write path: after the push the router tracker `lastPathnameAndSearch` equals the visible URL, so the masked entry lands on the identical-URL case that the `isMaskedRoute` guard excludes from the hash-only fast path. Verified red on the pre-guard `isHashOnly` (0 fetches), green with the guard. --- tests/pages-router-i18n-sticky-locale.test.ts | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/tests/pages-router-i18n-sticky-locale.test.ts b/tests/pages-router-i18n-sticky-locale.test.ts index fd437b4af..8711ca3cc 100644 --- a/tests/pages-router-i18n-sticky-locale.test.ts +++ b/tests/pages-router-i18n-sticky-locale.test.ts @@ -245,6 +245,91 @@ describe("Pages Router popstate stale-state filter (non-i18n parity)", () => { }); }); +// End-to-end producer→consumer regression. The hand-built-state tests above +// isolate the popstate consumer; this one closes the loop through the real +// state *writer*: a masked `Router.push(href, as)` produces history state via +// `updateHistory`, and that exact captured object is replayed through popstate. +// It guards against drift between the two sides — if `updateHistory` ever +// stopped writing `url` distinct from `as`, or the popstate handler stopped +// keying the fetch off `state.url`, this fails where the isolated tests would +// not. It also reproduces the PR's bug through the real write path: after a +// push the router tracker (`lastPathnameAndSearch`) equals the visible URL, so +// the masked entry hits the identical-URL case that the `isMaskedRoute` guard +// must exclude from the hash-only fast path. +describe("Pages Router masked popstate (producer→consumer integration)", () => { + it("replays a Router.push-produced masked entry and fetches state.url, not state.as", async () => { + const previousWindow = (globalThis as any).window; + const originalFetch = globalThis.fetch; + const originalCustomEvent = globalThis.CustomEvent; + const { win } = createNavWindow(); + win.location.pathname = "/"; + win.location.href = "http://localhost/"; + + const fetchMock = vi.fn<(input: RequestInfo | URL, init?: RequestInit) => Promise>( + async () => new Response(buildNavHtml("/dynamic-route", PAGE_MODULE_URL), { status: 200 }), + ); + globalThis.fetch = fetchMock; + (globalThis as any).CustomEvent = class CustomEventMock { + constructor(public type: string) {} + } as any; + + const listeners = new Map(); + win.addEventListener = vi.fn((type: string, handler: PopStateListener) => { + listeners.set(type, handler); + }) as any; + (globalThis as any).window = win; + + try { + vi.resetModules(); + const routerModule = await import("../packages/vinext/src/shims/router.js"); + const Router = routerModule.default; + const { installPagesRouterRuntime } = + await import("../packages/vinext/src/shims/pages-router-runtime.js"); + installPagesRouterRuntime(); + + const popstateHandler = listeners.get("popstate"); + expect(popstateHandler).toBeDefined(); + + // Producer: masked navigation. `href` is the route, `as` the address bar. + // `updateHistory` writes `{ url: "/dynamic-route", as: "/static", … }`. + await Router.push("/dynamic-route", "/static"); + await vi.waitFor(() => expect(win.__NEXT_DATA__.page).toBe("/dynamic-route")); + + // Capture the state the producer actually wrote — no hand-built shape. + const produced = win.history.state as MaskedHistoryState; + expect(produced.__N).toBe(true); + expect(produced.url).toBe("/dynamic-route"); + expect(produced.as).toBe("/static"); + expect(win.location.pathname).toBe("/static"); + + // Consumer: replay that exact entry. The visible URL is unchanged from + // the push (`/static`), so without the masked-route guard this would be + // mis-classified as a hash-only no-op. A single popstate suffices because + // the push already set `routerDidNavigate`, disabling the replay filter. + fetchMock.mockClear(); + win.__NEXT_DATA__ = { + ...win.__NEXT_DATA__, + page: "/static", + }; + + popstateHandler!({ state: produced }); + await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + // The fetch targets the route behind the mask, not the address bar. + expect(fetchMock.mock.calls[0]![0]).toBe("/dynamic-route"); + await vi.waitFor(() => expect(win.__NEXT_DATA__.page).toBe("/dynamic-route")); + } finally { + vi.resetModules(); + if (previousWindow === undefined) { + delete (globalThis as any).window; + } else { + (globalThis as any).window = previousWindow; + } + globalThis.fetch = originalFetch; + (globalThis as any).CustomEvent = originalCustomEvent; + } + }); +}); + // Ported from Next.js test/e2e/ignore-invalid-popstateevent — Next.js writes // `{ url, as, options, __N: true, key }` on every pushState/replaceState so // the popstate handler can detect stale or non-Next events. From ce0bec939e93a7905283f99799bf58f8e3fdc558 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Fri, 19 Jun 2026 16:40:08 +1000 Subject: [PATCH 4/5] fix(router): track full href for popstate hash detection --- packages/vinext/src/shims/router.ts | 42 ++++++++++------- tests/pages-router-i18n-sticky-locale.test.ts | 9 ++-- tests/shims.test.ts | 45 +++++++++++++------ 3 files changed, 60 insertions(+), 36 deletions(-) diff --git a/packages/vinext/src/shims/router.ts b/packages/vinext/src/shims/router.ts index d77b84c8b..03fe26743 100644 --- a/packages/vinext/src/shims/router.ts +++ b/packages/vinext/src/shims/router.ts @@ -414,6 +414,7 @@ type PagesRouterRuntimeState = { cancelPendingRenderCommit: (() => void) | null; beforePopStateCb?: BeforePopStateCallback; lastPathnameAndSearch: string; + lastBrowserUrl: string; isFirstPopStateEvent: boolean; routerDidNavigate: boolean; deprecatedEventBridgeInstalled: boolean; @@ -441,6 +442,7 @@ function createPagesRouterRuntimeState(): PagesRouterRuntimeState { cancelPendingRenderCommit: null, lastPathnameAndSearch: typeof window !== "undefined" ? window.location.pathname + window.location.search : "", + lastBrowserUrl: typeof window !== "undefined" ? window.location.href : "", isFirstPopStateEvent: true, routerDidNavigate: false, deprecatedEventBridgeInstalled: false, @@ -1697,6 +1699,7 @@ async function navigateClientData( window.history.replaceState(window.history.state ?? {}, "", redirectedUrl); routerRuntimeState.lastPathnameAndSearch = window.location.pathname + window.location.search; + routerRuntimeState.lastBrowserUrl = window.location.href; await navigateClientHtml(redirectedUrl, redirectedUrl, controller, navId, assertStillCurrent); return; } @@ -2019,6 +2022,7 @@ async function navigateClientHtml( if (pendingRedirectHistoryUrl) { window.history.replaceState(window.history.state ?? {}, "", pendingRedirectHistoryUrl); routerRuntimeState.lastPathnameAndSearch = window.location.pathname + window.location.search; + routerRuntimeState.lastBrowserUrl = window.location.href; } window.__NEXT_DATA__ = nextData; applyVinextLocaleGlobals(window, nextData); @@ -2113,6 +2117,7 @@ async function navigateClient( window.history.replaceState(window.history.state ?? {}, "", redirectedUrl); routerRuntimeState.lastPathnameAndSearch = window.location.pathname + window.location.search; + routerRuntimeState.lastBrowserUrl = window.location.href; browserUrl = redirectedUrl; htmlFetchUrl = redirectedUrl; } else if (middlewareEffect?.rewriteTarget) { @@ -2302,6 +2307,7 @@ function updateHistory( else window.history.replaceState(state, "", fullUrl); routerRuntimeState.currentHistoryKey = key; routerRuntimeState.lastPathnameAndSearch = window.location.pathname + window.location.search; + routerRuntimeState.lastBrowserUrl = window.location.href; routerRuntimeState.routerDidNavigate = true; } @@ -2819,10 +2825,10 @@ function PagesRouterProvider({ children }: { children: ReactNode }): ReactElemen : content; } -// `routerRuntimeState.lastPathnameAndSearch` tracks pathname+search for -// detecting hash-only back/forward in the popstate handler. It is updated after -// every pushState/replaceState so popstate can compare the previous value with -// the already-changed window.location. +// `routerRuntimeState.lastPathnameAndSearch` tracks pathname+search for the +// Safari-replay filter and push->replace coercion. It deliberately stays +// hash-stripped. `lastBrowserUrl` tracks the full browser href separately so +// popstate hash-only detection can compare against the previous hash-aware URL. // // `routerRuntimeState.isFirstPopStateEvent` mirrors Next.js's first-popstate // Safari replay filter (packages/next/src/shared/lib/router/router.ts around @@ -2929,16 +2935,17 @@ function handlePagesRouterPopState(e: PopStateEvent): void { state.url !== state.as ? normalizePathTrailingSlash(withBasePath(state.url, __basePath), __trailingSlash) : browserUrl; - const isMaskedRoute = stateRouteUrl !== browserUrl; - - // Detect hash-only back/forward: pathname+search unchanged, only hash differs. - // A masked entry is never hash-only even when the visible URL is identical to - // the current one: the route behind the mask still has to be fetched. This - // mirrors Next.js's `onlyAHashChange`, which returns false for an identical - // no-hash `as` (it requires a real hash delta), so the masked second popstate - // in test/e2e/ignore-invalid-popstateevent falls through to a full navigation - // instead of being short-circuited as a hash change. - const isHashOnly = !isMaskedRoute && browserUrl === routerRuntimeState.lastPathnameAndSearch; + + // Detect hash-only back/forward against the previous full browser URL. The + // pathname+search tracker is hash-stripped for other router comparisons, so + // using it here would misclassify identical no-hash URLs as hash-only. This + // mirrors Next.js's `onlyAHashChange`, which requires a real hash-bearing + // destination and therefore lets masked same-URL popstates fetch `state.url`. + const isHashOnly = isHashOnlyBrowserUrlChange( + window.location.href, + routerRuntimeState.lastBrowserUrl, + __basePath, + ); const targetKey = getRouterStateKey(state); let forcedScroll: ScrollPosition | undefined; @@ -2983,13 +2990,14 @@ function handlePagesRouterPopState(e: PopStateEvent): void { // Update trackers only after beforePopState confirms navigation proceeds. // If beforePopState cancels, the app stays on the previous history entry, - // so both must retain their previous values: `lastPathnameAndSearch` so the - // next popstate compares against the correct baseline, and `currentHistoryKey` - // so subsequent scroll bookkeeping keys off the entry the app is actually on. + // so the URL trackers must retain their previous values for the next + // popstate baseline, and `currentHistoryKey` must keep pointing at the entry + // the app is actually on for subsequent scroll bookkeeping. if (targetKey !== undefined) { routerRuntimeState.currentHistoryKey = targetKey; } routerRuntimeState.lastPathnameAndSearch = browserUrl; + routerRuntimeState.lastBrowserUrl = window.location.href; if (isHashOnly) { // Hash-only back/forward — no page fetch needed. diff --git a/tests/pages-router-i18n-sticky-locale.test.ts b/tests/pages-router-i18n-sticky-locale.test.ts index 8711ca3cc..0e72fb6c8 100644 --- a/tests/pages-router-i18n-sticky-locale.test.ts +++ b/tests/pages-router-i18n-sticky-locale.test.ts @@ -253,9 +253,8 @@ describe("Pages Router popstate stale-state filter (non-i18n parity)", () => { // stopped writing `url` distinct from `as`, or the popstate handler stopped // keying the fetch off `state.url`, this fails where the isolated tests would // not. It also reproduces the PR's bug through the real write path: after a -// push the router tracker (`lastPathnameAndSearch`) equals the visible URL, so -// the masked entry hits the identical-URL case that the `isMaskedRoute` guard -// must exclude from the hash-only fast path. +// push the visible browser URL is unchanged and has no hash delta, so the +// masked entry must not be swallowed by the hash-only fast path. describe("Pages Router masked popstate (producer→consumer integration)", () => { it("replays a Router.push-produced masked entry and fetches state.url, not state.as", async () => { const previousWindow = (globalThis as any).window; @@ -303,8 +302,8 @@ describe("Pages Router masked popstate (producer→consumer integration)", () => expect(win.location.pathname).toBe("/static"); // Consumer: replay that exact entry. The visible URL is unchanged from - // the push (`/static`), so without the masked-route guard this would be - // mis-classified as a hash-only no-op. A single popstate suffices because + // the push (`/static`) and carries no hash, so hash-only classification + // must fall through to a full fetch. A single popstate suffices because // the push already set `routerDidNavigate`, disabling the replay filter. fetchMock.mockClear(); win.__NEXT_DATA__ = { diff --git a/tests/shims.test.ts b/tests/shims.test.ts index 517b0802e..4fc0a6f21 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -15879,6 +15879,7 @@ describe("Pages Router concurrent navigation", () => { // snapshot stays in sessionStorage for a later non-hash popstate. it("hash-only popstate scrolls to the hash anchor and leaves the entry snapshot for later", async () => { const previousWindow = (globalThis as any).window; + const previousDocument = (globalThis as any).document; const originalFetch = globalThis.fetch; const originalScrollRestorationEnv = process.env.__NEXT_SCROLL_RESTORATION; const listeners = new Map void>(); @@ -15906,6 +15907,10 @@ describe("Pages Router concurrent navigation", () => { }; (globalThis as any).window = win; + (globalThis as any).document = { + getElementById: vi.fn(() => null), + getElementsByName: vi.fn(() => []), + }; globalThis.fetch = vi.fn(); process.env.__NEXT_SCROLL_RESTORATION = "true"; @@ -15928,9 +15933,11 @@ describe("Pages Router concurrent navigation", () => { win.scrollX = 5; win.scrollY = 500; - // Hash-only push mints a new history entry ("#top" avoids the - // document-dependent getElementById branch of scrollToHashTarget). - await (win as any).next.router.push("/#top"); + // Hash-only push mints a router-owned hash entry and, more importantly, + // records a real previous full browser URL for the later popstate + // comparison. Use a non-top hash so the popstate below has a real hash + // delta while still staying on the same pathname+search. + await (win as any).next.router.push("/#departed"); expect(sessionStore.get("__next_scroll_key-initial")).toBe(JSON.stringify({ x: 5, y: 500 })); const pushedKey = pushState.mock.calls.at(-1)![0].key as string; expect(typeof pushedKey).toBe("string"); @@ -15943,34 +15950,39 @@ describe("Pages Router concurrent navigation", () => { hashEvents.push("hashChangeComplete"), ); - // The live position on the hash entry, snapshotted on departure. + const hashKey = "key-hash"; + sessionStore.set(`__next_scroll_${hashKey}`, JSON.stringify({ x: 12, y: 345 })); + // The live position on the departed hash entry, snapshotted on + // traversal away from `pushedKey`. win.scrollX = 7; win.scrollY = 70; win.scrollTo.mockClear(); - // Back to the initial entry: same pathname+search, only the hash + // Forward to another hash entry: same pathname+search, only the hash // differs, so this is a hash-only popstate. - win.location.hash = ""; + win.location.hash = "#top"; win.location.pathname = "/"; - win.location.href = "http://localhost/"; + win.location.href = "http://localhost/#top"; popstateHandler!({ - state: { __N: true, url: "/", as: "/", options: {}, key: "key-initial" }, + state: { __N: true, url: "/", as: "/", options: {}, key: hashKey }, }); // Hash-only: no page fetch, hash events fired in order. expect(globalThis.fetch).not.toHaveBeenCalled(); expect(hashEvents).toEqual(["hashChangeStart", "hashChangeComplete"]); - // Only the hash anchor is honored (empty hash scrolls to top); the - // target entry's saved {x: 5, y: 500} is not applied (upstream parity). + // Only the hash anchor is honored; the target entry's saved {x: 12, y: 345} + // is not applied (upstream parity). expect(win.scrollTo).toHaveBeenCalledWith(0, 0); expect(win.scrollTo).not.toHaveBeenCalledWith(5, 500); + expect(win.scrollTo).not.toHaveBeenCalledWith(12, 345); - // The departed hash entry's live position was snapshotted under its - // key, and the unconsumed target snapshot survives for a later - // non-hash popstate to this entry. - expect(sessionStore.get(`__next_scroll_${pushedKey}`)).toBe(JSON.stringify({ x: 7, y: 70 })); + // The departed hash entry's live position was snapshotted under its key, + // and the unconsumed target snapshot survives for a later non-hash + // popstate to this entry. expect(sessionStore.get("__next_scroll_key-initial")).toBe(JSON.stringify({ x: 5, y: 500 })); + expect(sessionStore.get(`__next_scroll_${pushedKey}`)).toBe(JSON.stringify({ x: 7, y: 70 })); + expect(sessionStore.get(`__next_scroll_${hashKey}`)).toBe(JSON.stringify({ x: 12, y: 345 })); } finally { vi.resetModules(); if (previousWindow === undefined) { @@ -15978,6 +15990,11 @@ describe("Pages Router concurrent navigation", () => { } else { (globalThis as any).window = previousWindow; } + if (previousDocument === undefined) { + delete (globalThis as any).document; + } else { + (globalThis as any).document = previousDocument; + } globalThis.fetch = originalFetch; if (originalScrollRestorationEnv === undefined) { delete process.env.__NEXT_SCROLL_RESTORATION; From 80df5edc026fdc4a4c6900033e07d2e145df5e18 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Fri, 19 Jun 2026 16:46:42 +1000 Subject: [PATCH 5/5] fix(router): treat hash removal as hash-only --- packages/vinext/src/shims/router.ts | 35 ++++++++++++-------------- packages/vinext/src/shims/url-utils.ts | 6 ++++- tests/shims.test.ts | 27 ++++++++++++++++++++ 3 files changed, 48 insertions(+), 20 deletions(-) diff --git a/packages/vinext/src/shims/router.ts b/packages/vinext/src/shims/router.ts index 03fe26743..cb11b417b 100644 --- a/packages/vinext/src/shims/router.ts +++ b/packages/vinext/src/shims/router.ts @@ -474,6 +474,12 @@ function getPagesRouterRuntimeState(): PagesRouterRuntimeState { const routerRuntimeState = getPagesRouterRuntimeState(); const routerEvents = routerRuntimeState.events; +function updateBrowserUrlTrackers(pathnameAndSearch?: string): void { + routerRuntimeState.lastPathnameAndSearch = + pathnameAndSearch ?? window.location.pathname + window.location.search; + routerRuntimeState.lastBrowserUrl = window.location.href; +} + function getPagesRouterRuntimeComponents(): PagesRouterRuntimeComponents { const existing = routerRuntimeState.components; if (existing) return existing; @@ -1274,7 +1280,7 @@ function getPathnameAndQuery(): { function getCurrentHistoryAsPath(): string | null { const state = window.history?.state; - if (!isNextRouterState(state) || typeof state.as !== "string") return null; + if (!isNextRouterState(state)) return null; try { const browserUrl = new URL(window.location.href); @@ -1698,8 +1704,7 @@ async function navigateClientData( } window.history.replaceState(window.history.state ?? {}, "", redirectedUrl); - routerRuntimeState.lastPathnameAndSearch = window.location.pathname + window.location.search; - routerRuntimeState.lastBrowserUrl = window.location.href; + updateBrowserUrlTrackers(); await navigateClientHtml(redirectedUrl, redirectedUrl, controller, navId, assertStillCurrent); return; } @@ -2021,8 +2026,7 @@ async function navigateClientHtml( // Next.js's client Root callback without remounting the page tree. if (pendingRedirectHistoryUrl) { window.history.replaceState(window.history.state ?? {}, "", pendingRedirectHistoryUrl); - routerRuntimeState.lastPathnameAndSearch = window.location.pathname + window.location.search; - routerRuntimeState.lastBrowserUrl = window.location.href; + updateBrowserUrlTrackers(); } window.__NEXT_DATA__ = nextData; applyVinextLocaleGlobals(window, nextData); @@ -2115,9 +2119,7 @@ async function navigateClient( scheduleHardNavigationAndThrow(redirectLocation, "Navigation redirected externally"); } window.history.replaceState(window.history.state ?? {}, "", redirectedUrl); - routerRuntimeState.lastPathnameAndSearch = - window.location.pathname + window.location.search; - routerRuntimeState.lastBrowserUrl = window.location.href; + updateBrowserUrlTrackers(); browserUrl = redirectedUrl; htmlFetchUrl = redirectedUrl; } else if (middlewareEffect?.rewriteTarget) { @@ -2306,8 +2308,7 @@ function updateHistory( if (mode === "push") window.history.pushState(state, "", fullUrl); else window.history.replaceState(state, "", fullUrl); routerRuntimeState.currentHistoryKey = key; - routerRuntimeState.lastPathnameAndSearch = window.location.pathname + window.location.search; - routerRuntimeState.lastBrowserUrl = window.location.href; + updateBrowserUrlTrackers(); routerRuntimeState.routerDidNavigate = true; } @@ -2912,7 +2913,6 @@ function handlePagesRouterPopState(e: PopStateEvent): void { const currentLocale = window.__VINEXT_LOCALE__; if ( state.options?.locale === currentLocale && - typeof state.as === "string" && withBasePath(state.as, __basePath) === routerRuntimeState.lastPathnameAndSearch ) { return; @@ -2929,18 +2929,16 @@ function handlePagesRouterPopState(e: PopStateEvent): void { // URL. Defaults to `browserUrl` for non-masked entries, making the rest of // the handler a no-op for ordinary back/forward navigation. const stateRouteUrl = - isNextRouterState(state) && - typeof state.url === "string" && - typeof state.as === "string" && - state.url !== state.as + isNextRouterState(state) && state.url !== state.as ? normalizePathTrailingSlash(withBasePath(state.url, __basePath), __trailingSlash) : browserUrl; // Detect hash-only back/forward against the previous full browser URL. The // pathname+search tracker is hash-stripped for other router comparisons, so // using it here would misclassify identical no-hash URLs as hash-only. This - // mirrors Next.js's `onlyAHashChange`, which requires a real hash-bearing - // destination and therefore lets masked same-URL popstates fetch `state.url`. + // mirrors Next.js's `onlyAHashChange`, which requires a real hash delta (or + // a hash-bearing destination for same-hash scrolling) and therefore lets + // masked same-URL popstates fetch `state.url`. const isHashOnly = isHashOnlyBrowserUrlChange( window.location.href, routerRuntimeState.lastBrowserUrl, @@ -2996,8 +2994,7 @@ function handlePagesRouterPopState(e: PopStateEvent): void { if (targetKey !== undefined) { routerRuntimeState.currentHistoryKey = targetKey; } - routerRuntimeState.lastPathnameAndSearch = browserUrl; - routerRuntimeState.lastBrowserUrl = window.location.href; + updateBrowserUrlTrackers(browserUrl); if (isHashOnly) { // Hash-only back/forward — no page fetch needed. diff --git a/packages/vinext/src/shims/url-utils.ts b/packages/vinext/src/shims/url-utils.ts index 321c270a4..c6a89a44d 100644 --- a/packages/vinext/src/shims/url-utils.ts +++ b/packages/vinext/src/shims/url-utils.ts @@ -216,7 +216,11 @@ export function isHashOnlyBrowserUrlChange( // Keep this raw comparison distinct from the App Router planner's parsed // search-param comparison until their separate navigation lifecycles are // deliberately unified. - return currentPathname === nextPathname && current.search === next.search && next.hash !== ""; + return ( + currentPathname === nextPathname && + current.search === next.search && + (next.hash !== "" || current.hash !== next.hash) + ); } catch { return false; } diff --git a/tests/shims.test.ts b/tests/shims.test.ts index 4fc0a6f21..48feff058 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -3,6 +3,7 @@ import { readFile } from "node:fs/promises"; import path from "node:path"; import { PAGES_FIXTURE_DIR } from "./helpers.js"; import { isExternalUrl, isHashOnlyChange } from "../packages/vinext/src/shims/router.js"; +import { isHashOnlyBrowserUrlChange } from "../packages/vinext/src/shims/url-utils.js"; import { extractVinextNextDataJson } from "../packages/vinext/src/client/vinext-next-data.js"; import { isValidModulePath } from "../packages/vinext/src/client/validate-module-path.js"; import vinext from "../packages/vinext/src/index.js"; @@ -14237,6 +14238,32 @@ describe("Pages Router router helpers", () => { }); }); + describe("isHashOnlyBrowserUrlChange", () => { + it("returns false for identical no-hash URLs", () => { + expect(isHashOnlyBrowserUrlChange("http://localhost/about", "http://localhost/about")).toBe( + false, + ); + }); + + it("returns true when browser back removes a hash on the same path and search", () => { + expect( + isHashOnlyBrowserUrlChange( + "http://localhost/about?tab=1", + "http://localhost/about?tab=1#section", + ), + ).toBe(true); + }); + + it("returns true for same-hash anchor scrolling", () => { + expect( + isHashOnlyBrowserUrlChange( + "http://localhost/about#section", + "http://localhost/about#section", + ), + ).toBe(true); + }); + }); + describe("applyNavigationLocale", () => { it("does not prefix absolute https:// URLs", async () => { const { applyNavigationLocale } = await import("../packages/vinext/src/shims/router.js");