From 030d111e7cf3bc474487902794ed15987425c525 Mon Sep 17 00:00:00 2001 From: Divanshu Chauhan Date: Wed, 25 Mar 2026 17:59:01 -0700 Subject: [PATCH 1/5] fix: cross-route client navigation hangs in Firefox (#652) Replace flushSync-based RSC tree rendering with a two-phase navigation commit that uses startTransition for same-route navigations and synchronous updates for cross-route navigations. This fixes the Firefox hang where startTransition never commits when the entire component tree is replaced, and also resolves the Suspense double-flash from #639. Key changes: - navigation.ts: ClientNavigationState on Symbol.for global for module instance safety, render snapshot context for hook consistency during transitions, RSC response snapshot/restore for visited cache, unified navigateClientSide() entry point, history suppression helpers - app-browser-entry.ts: BrowserRoot component with useState-managed tree, NavigationCommitSignal that defers URL commit to useLayoutEffect, visited response cache with LRU eviction, navigation ID counter for stale navigation bailout, manual scroll restoration - link.tsx: remove duplicated helpers, delegate to navigateClientSide - form.tsx: delegate App Router GET navigation to navigateClientSide Closes #652 --- packages/vinext/src/global.d.ts | 16 +- .../vinext/src/server/app-browser-entry.ts | 446 ++++++++++++-- packages/vinext/src/shims/form.tsx | 13 +- packages/vinext/src/shims/link.tsx | 117 +--- packages/vinext/src/shims/navigation.ts | 550 ++++++++++++++---- .../app-router/navigation-regressions.spec.ts | 188 ++++++ .../app/nav-flash/link-sync/FilterLinks.tsx | 29 + .../app/nav-flash/link-sync/page.tsx | 37 ++ .../app-basic/app/nav-flash/list/page.tsx | 21 + .../param-sync/[filter]/FilterControls.tsx | 22 + .../nav-flash/param-sync/[filter]/page.tsx | 20 + .../app/nav-flash/provider/[id]/page.tsx | 19 + .../nav-flash/query-sync/FilterControls.tsx | 26 + .../app/nav-flash/query-sync/page.tsx | 27 + tests/fixtures/app-basic/app/page.tsx | 6 + tests/form.test.ts | 44 +- 16 files changed, 1288 insertions(+), 293 deletions(-) create mode 100644 tests/e2e/app-router/navigation-regressions.spec.ts create mode 100644 tests/fixtures/app-basic/app/nav-flash/link-sync/FilterLinks.tsx create mode 100644 tests/fixtures/app-basic/app/nav-flash/link-sync/page.tsx create mode 100644 tests/fixtures/app-basic/app/nav-flash/list/page.tsx create mode 100644 tests/fixtures/app-basic/app/nav-flash/param-sync/[filter]/FilterControls.tsx create mode 100644 tests/fixtures/app-basic/app/nav-flash/param-sync/[filter]/page.tsx create mode 100644 tests/fixtures/app-basic/app/nav-flash/provider/[id]/page.tsx create mode 100644 tests/fixtures/app-basic/app/nav-flash/query-sync/FilterControls.tsx create mode 100644 tests/fixtures/app-basic/app/nav-flash/query-sync/page.tsx diff --git a/packages/vinext/src/global.d.ts b/packages/vinext/src/global.d.ts index ff360f2cc..9d714e0d7 100644 --- a/packages/vinext/src/global.d.ts +++ b/packages/vinext/src/global.d.ts @@ -16,6 +16,7 @@ import type { Root } from "react-dom/client"; import type { OnRequestErrorHandler } from "./server/instrumentation"; +import type { CachedRscResponse, PrefetchCacheEntry } from "./shims/navigation"; // --------------------------------------------------------------------------- // Window globals — browser-side state shared across module boundaries @@ -75,8 +76,17 @@ declare global { * * @param href - The destination URL (may be absolute or relative). * @param redirectDepth - Internal parameter used to detect redirect loops. + * @param navigationKind - Internal hint for traversal vs regular navigation. + * @param historyUpdateMode - Internal hint for when history should publish. */ - __VINEXT_RSC_NAVIGATE__: ((href: string, redirectDepth?: number) => Promise) | undefined; + __VINEXT_RSC_NAVIGATE__: + | (( + href: string, + redirectDepth?: number, + navigationKind?: "navigate" | "traverse" | "refresh", + historyUpdateMode?: "push" | "replace", + ) => Promise) + | undefined; /** * A Promise that resolves when the current in-flight popstate RSC navigation @@ -93,9 +103,7 @@ declare global { * Lazily initialised on `window` by `shims/navigation.ts` so the same Map * instance is shared between the navigation shim and the Link component. */ - __VINEXT_RSC_PREFETCH_CACHE__: - | Map - | undefined; + __VINEXT_RSC_PREFETCH_CACHE__: Map | undefined; /** * Set of RSC URLs that have already been prefetched (or are in-flight). diff --git a/packages/vinext/src/server/app-browser-entry.ts b/packages/vinext/src/server/app-browser-entry.ts index 3713228b1..df34d11be 100644 --- a/packages/vinext/src/server/app-browser-entry.ts +++ b/packages/vinext/src/server/app-browser-entry.ts @@ -1,7 +1,14 @@ /// -import type { ReactNode } from "react"; -import type { Root } from "react-dom/client"; +import { + createElement, + startTransition, + useLayoutEffect, + useState, + type Dispatch, + type ReactNode, + type SetStateAction, +} from "react"; import { createFromFetch, createFromReadableStream, @@ -9,15 +16,25 @@ import { encodeReply, setServerCallback, } from "@vitejs/plugin-rsc/browser"; -import { flushSync } from "react-dom"; import { hydrateRoot } from "react-dom/client"; import { - PREFETCH_CACHE_TTL, + activateNavigationSnapshot, + commitClientNavigationState, + consumePrefetchResponse, + createClientNavigationRenderSnapshot, + getClientNavigationRenderContext, getPrefetchCache, getPrefetchedUrls, + pushHistoryStateWithoutNotify, + replaceClientParamsWithoutNotify, + replaceHistoryStateWithoutNotify, + restoreRscResponse, setClientParams, + snapshotRscResponse, setNavigationContext, toRscUrl, + type CachedRscResponse, + type ClientNavigationRenderSnapshot, } from "../shims/navigation.js"; import { chunksToReadableStream, @@ -35,19 +52,265 @@ interface ServerActionResult { }; } -let reactRoot: Root | null = null; - -function getReactRoot(): Root { - if (!reactRoot) { - throw new Error("[vinext] React root is not initialized"); - } - return reactRoot; +type BrowserTreeState = { + renderId: number; + node: ReactNode; + navigationSnapshot: ClientNavigationRenderSnapshot; +}; +type NavigationKind = "navigate" | "traverse" | "refresh"; +type HistoryUpdateMode = "push" | "replace"; +interface VisitedResponseCacheEntry { + params: Record; + expiresAt: number; + response: CachedRscResponse; } +const MAX_VISITED_RESPONSE_CACHE_SIZE = 50; +const VISITED_RESPONSE_CACHE_TTL = 5 * 60_000; + +let nextNavigationRenderId = 0; +let activeNavigationId = 0; +const pendingNavigationCommits = new Map void>(); +const pendingNavigationPrePaintEffects = new Map void>(); +let setBrowserTreeState: Dispatch> | null = null; +let latestClientParams: Record = {}; +const visitedResponseCache = new Map(); + function isServerActionResult(value: unknown): value is ServerActionResult { return !!value && typeof value === "object" && "root" in value; } +function getBrowserTreeStateSetter(): Dispatch> { + if (!setBrowserTreeState) { + throw new Error("[vinext] Browser tree state is not initialized"); + } + return setBrowserTreeState; +} + +function applyClientParams(params: Record): void { + latestClientParams = params; + setClientParams(params); +} + +function stageClientParams(params: Record): void { + latestClientParams = params; + replaceClientParamsWithoutNotify(params); +} + +function clearVisitedResponseCache(): void { + visitedResponseCache.clear(); +} + +function clearPrefetchState(): void { + getPrefetchCache().clear(); + getPrefetchedUrls().clear(); +} + +function clearClientNavigationCaches(): void { + clearVisitedResponseCache(); + clearPrefetchState(); +} + +function queuePrePaintNavigationEffect(renderId: number, effect: (() => void) | null): void { + if (!effect) { + return; + } + pendingNavigationPrePaintEffects.set(renderId, effect); +} + +/** + * Run all queued pre-paint effects for renderIds up to and including the + * given renderId. When React supersedes a startTransition update (rapid + * clicks on same-route links), the superseded NavigationCommitSignal never + * mounts, so its pre-paint effect never fires. By draining all effects + * <= the committed renderId here, the winning transition cleans up after + * any superseded ones, keeping the counter balanced. + */ +function drainPrePaintEffects(upToRenderId: number): void { + for (const [id, effect] of pendingNavigationPrePaintEffects) { + if (id <= upToRenderId) { + pendingNavigationPrePaintEffects.delete(id); + if (id === upToRenderId) { + effect(); + } else { + commitClientNavigationState(); + } + } + } +} + +function createNavigationCommitEffect( + href: string, + historyUpdateMode: HistoryUpdateMode | undefined, +): () => void { + return () => { + if (historyUpdateMode === "replace") { + replaceHistoryStateWithoutNotify(null, "", href); + } else if (historyUpdateMode === "push") { + pushHistoryStateWithoutNotify(null, "", href); + } + + commitClientNavigationState(); + }; +} + +function evictVisitedResponseCacheIfNeeded(): void { + while (visitedResponseCache.size >= MAX_VISITED_RESPONSE_CACHE_SIZE) { + const oldest = visitedResponseCache.keys().next().value; + if (oldest === undefined) { + return; + } + visitedResponseCache.delete(oldest); + } +} + +function getVisitedResponse( + rscUrl: string, + navigationKind: NavigationKind, +): VisitedResponseCacheEntry | null { + const cached = visitedResponseCache.get(rscUrl); + if (!cached) { + return null; + } + + if (navigationKind === "refresh") { + return null; + } + + if (navigationKind === "traverse") { + return cached; + } + + if (cached.expiresAt > Date.now()) { + return cached; + } + + visitedResponseCache.delete(rscUrl); + return null; +} + +function storeVisitedResponseSnapshot( + rscUrl: string, + snapshot: CachedRscResponse, + params: Record, +): void { + visitedResponseCache.delete(rscUrl); + evictVisitedResponseCacheIfNeeded(); + const now = Date.now(); + visitedResponseCache.set(rscUrl, { + params, + expiresAt: now + VISITED_RESPONSE_CACHE_TTL, + response: snapshot, + }); +} + +function resolveCommittedNavigations(renderId: number): void { + for (const [pendingId, resolve] of pendingNavigationCommits) { + if (pendingId <= renderId) { + pendingNavigationCommits.delete(pendingId); + resolve(); + } + } +} + +function NavigationCommitSignal({ children, renderId }: { children: ReactNode; renderId: number }) { + useLayoutEffect(() => { + drainPrePaintEffects(renderId); + + const frame = requestAnimationFrame(() => { + resolveCommittedNavigations(renderId); + }); + + return () => { + cancelAnimationFrame(frame); + }; + }, [renderId]); + + return children; +} + +function BrowserRoot({ + initialNode, + initialNavigationSnapshot, +}: { + initialNode: ReactNode; + initialNavigationSnapshot: ClientNavigationRenderSnapshot; +}) { + const [treeState, setTreeState] = useState({ + renderId: 0, + node: initialNode, + navigationSnapshot: initialNavigationSnapshot, + }); + + setBrowserTreeState = setTreeState; + + const committedTree = createElement(NavigationCommitSignal, { + renderId: treeState.renderId, + children: treeState.node, + }); + + const ClientNavigationRenderContext = getClientNavigationRenderContext(); + if (!ClientNavigationRenderContext) { + return committedTree; + } + + return createElement( + ClientNavigationRenderContext.Provider, + { value: treeState.navigationSnapshot }, + committedTree, + ); +} + +function updateBrowserTree( + node: ReactNode | Promise, + navigationSnapshot: ClientNavigationRenderSnapshot, + renderId: number, + useTransitionMode: boolean, +): void { + const setter = getBrowserTreeStateSetter(); + + const resolvedThenSet = (resolvedNode: ReactNode) => { + setter({ renderId, node: resolvedNode, navigationSnapshot }); + }; + + if (node instanceof Promise) { + if (useTransitionMode) { + void node.then((resolved) => { + startTransition(() => resolvedThenSet(resolved)); + }); + } else { + void node.then(resolvedThenSet); + } + return; + } + + if (useTransitionMode) { + startTransition(() => resolvedThenSet(node)); + return; + } + + resolvedThenSet(node); +} + +function renderNavigationPayload( + payload: Promise | ReactNode, + navigationSnapshot: ClientNavigationRenderSnapshot, + prePaintEffect: (() => void) | null = null, + useTransition = true, +): Promise { + const renderId = ++nextNavigationRenderId; + queuePrePaintNavigationEffect(renderId, prePaintEffect); + + const committed = new Promise((resolve) => { + pendingNavigationCommits.set(renderId, resolve); + }); + + activateNavigationSnapshot(); + updateBrowserTree(payload, navigationSnapshot, renderId, useTransition); + + return committed; +} + function restoreHydrationNavigationContext( pathname: string, searchParams: SearchParamInput, @@ -60,6 +323,19 @@ function restoreHydrationNavigationContext( }); } +function restorePopstateScrollPosition(state: unknown): void { + if (!(state && typeof state === "object" && "__vinext_scrollY" in state)) { + return; + } + + const y = Number(state.__vinext_scrollY); + const x = "__vinext_scrollX" in state ? Number(state.__vinext_scrollX) : 0; + + requestAnimationFrame(() => { + window.scrollTo(x, y); + }); +} + async function readInitialRscStream(): Promise> { const vinext = getVinextBrowserGlobal(); @@ -70,7 +346,7 @@ async function readInitialRscStream(): Promise> { const params = embedData.params ?? {}; if (embedData.params) { - setClientParams(embedData.params); + applyClientParams(embedData.params); } if (embedData.nav) { restoreHydrationNavigationContext( @@ -85,7 +361,7 @@ async function readInitialRscStream(): Promise> { const params = vinext.__VINEXT_RSC_PARAMS__ ?? {}; if (vinext.__VINEXT_RSC_PARAMS__) { - setClientParams(vinext.__VINEXT_RSC_PARAMS__); + applyClientParams(vinext.__VINEXT_RSC_PARAMS__); } if (vinext.__VINEXT_RSC_NAV__) { restoreHydrationNavigationContext( @@ -105,7 +381,7 @@ async function readInitialRscStream(): Promise> { if (paramsHeader) { try { params = JSON.parse(decodeURIComponent(paramsHeader)) as Record; - setClientParams(params); + applyClientParams(params); } catch { // Ignore malformed param headers and continue with hydration. } @@ -157,12 +433,20 @@ function registerServerActionCallback(): void { return undefined; } - const result = await createFromFetch(Promise.resolve(fetchResponse), { - temporaryReferences, - }); + clearClientNavigationCaches(); + + const result = await createFromFetch( + Promise.resolve(fetchResponse), + { temporaryReferences }, + ); if (isServerActionResult(result)) { - getReactRoot().render(result.root); + updateBrowserTree( + result.root, + createClientNavigationRenderSnapshot(window.location.href, latestClientParams), + ++nextNavigationRenderId, + false, + ); if (result.returnValue) { if (!result.returnValue.ok) throw result.returnValue.data; return result.returnValue.data; @@ -170,7 +454,12 @@ function registerServerActionCallback(): void { return undefined; } - getReactRoot().render(result as ReactNode); + updateBrowserTree( + result, + createClientNavigationRenderSnapshot(window.location.href, latestClientParams), + ++nextNavigationRenderId, + false, + ); return result; }); } @@ -179,19 +468,26 @@ async function main(): Promise { registerServerActionCallback(); const rscStream = await readInitialRscStream(); - const root = await createFromReadableStream(rscStream); + const root = await createFromReadableStream(rscStream); + const initialNavigationSnapshot = createClientNavigationRenderSnapshot( + window.location.href, + latestClientParams, + ); - reactRoot = hydrateRoot( + window.__VINEXT_RSC_ROOT__ = hydrateRoot( document, - root as ReactNode, + createElement(BrowserRoot, { + initialNode: root, + initialNavigationSnapshot, + }), import.meta.env.DEV ? { onCaughtError() {} } : undefined, ); - window.__VINEXT_RSC_ROOT__ = reactRoot; - window.__VINEXT_RSC_NAVIGATE__ = async function navigateRsc( href: string, redirectDepth = 0, + navigationKind: NavigationKind = "navigate", + historyUpdateMode?: HistoryUpdateMode, ): Promise { if (redirectDepth > 10) { console.error( @@ -204,18 +500,41 @@ async function main(): Promise { try { const url = new URL(href, window.location.origin); const rscUrl = toRscUrl(url.pathname + url.search); + const navId = ++activeNavigationId; + // Use startTransition for same-route navigations (searchParam changes) + // so React keeps the old UI visible during the transition. For cross-route + // navigations (different pathname), use synchronous updates — React's + // startTransition hangs in Firefox when replacing the entire tree. + const isSameRoute = url.pathname === window.location.pathname; + const cachedRoute = getVisitedResponse(rscUrl, navigationKind); + const navigationCommitEffect = createNavigationCommitEffect(href, historyUpdateMode); + + if (cachedRoute) { + stageClientParams(cachedRoute.params); + const cachedNavigationSnapshot = createClientNavigationRenderSnapshot( + href, + cachedRoute.params, + ); + const cachedPayload = createFromFetch( + Promise.resolve(restoreRscResponse(cachedRoute.response)), + ); + await renderNavigationPayload( + cachedPayload, + cachedNavigationSnapshot, + navigationCommitEffect, + isSameRoute, + ); + return; + } let navResponse: Response | undefined; - const prefetchCache = getPrefetchCache(); - const cached = prefetchCache.get(rscUrl); - - if (cached && Date.now() - cached.timestamp < PREFETCH_CACHE_TTL) { - navResponse = cached.response; - prefetchCache.delete(rscUrl); - getPrefetchedUrls().delete(rscUrl); - } else if (cached) { - prefetchCache.delete(rscUrl); - getPrefetchedUrls().delete(rscUrl); + let navResponseUrl: string | null = null; + if (navigationKind !== "refresh") { + const prefetchedResponse = consumePrefetchResponse(rscUrl); + if (prefetchedResponse) { + navResponse = restoreRscResponse(prefetchedResponse); + navResponseUrl = prefetchedResponse.url; + } } if (!navResponse) { @@ -225,11 +544,13 @@ async function main(): Promise { }); } - const finalUrl = new URL(navResponse.url); + if (navId !== activeNavigationId) return; + + const finalUrl = new URL(navResponseUrl ?? navResponse.url, window.location.origin); const requestedUrl = new URL(rscUrl, window.location.origin); + if (finalUrl.pathname !== requestedUrl.pathname) { const destinationPath = finalUrl.pathname.replace(/\.rsc$/, "") + finalUrl.search; - window.history.replaceState(null, "", destinationPath); const navigate = window.__VINEXT_RSC_NAVIGATE__; if (!navigate) { @@ -237,35 +558,58 @@ async function main(): Promise { return; } - return navigate(destinationPath, redirectDepth + 1); + return navigate(destinationPath, redirectDepth + 1, navigationKind, historyUpdateMode); } + let navParams: Record = {}; const paramsHeader = navResponse.headers.get("X-Vinext-Params"); if (paramsHeader) { try { - setClientParams(JSON.parse(decodeURIComponent(paramsHeader))); + navParams = JSON.parse(decodeURIComponent(paramsHeader)) as Record< + string, + string | string[] + >; + stageClientParams(navParams); } catch { - setClientParams({}); + stageClientParams({}); } } else { - setClientParams({}); + stageClientParams({}); } + const navigationSnapshot = createClientNavigationRenderSnapshot(href, latestClientParams); - const rscPayload = await createFromFetch(Promise.resolve(navResponse)); - flushSync(() => { - getReactRoot().render(rscPayload as ReactNode); - }); + const responseSnapshot = await snapshotRscResponse(navResponse); + + if (navId !== activeNavigationId) return; + + storeVisitedResponseSnapshot(rscUrl, responseSnapshot, navParams); + const rscPayload = createFromFetch( + Promise.resolve(restoreRscResponse(responseSnapshot)), + ); + + await renderNavigationPayload( + rscPayload, + navigationSnapshot, + navigationCommitEffect, + isSameRoute, + ); } catch (error) { + commitClientNavigationState(); console.error("[vinext] RSC navigation error:", error); window.location.href = href; } }; - window.addEventListener("popstate", () => { + if ("scrollRestoration" in history) { + history.scrollRestoration = "manual"; + } + + window.addEventListener("popstate", (event) => { const pendingNavigation = - window.__VINEXT_RSC_NAVIGATE__?.(window.location.href) ?? Promise.resolve(); + window.__VINEXT_RSC_NAVIGATE__?.(window.location.href, 0, "traverse") ?? Promise.resolve(); window.__VINEXT_RSC_PENDING__ = pendingNavigation; void pendingNavigation.finally(() => { + restorePopstateScrollPosition(event.state); if (window.__VINEXT_RSC_PENDING__ === pendingNavigation) { window.__VINEXT_RSC_PENDING__ = null; } @@ -275,10 +619,16 @@ async function main(): Promise { if (import.meta.hot) { import.meta.hot.on("rsc:update", async () => { try { - const rscPayload = await createFromFetch( + clearClientNavigationCaches(); + const rscPayload = await createFromFetch( fetch(toRscUrl(window.location.pathname + window.location.search)), ); - getReactRoot().render(rscPayload as ReactNode); + updateBrowserTree( + rscPayload, + createClientNavigationRenderSnapshot(window.location.href, latestClientParams), + ++nextNavigationRenderId, + false, + ); } catch (error) { console.error("[vinext] RSC HMR error:", error); } diff --git a/packages/vinext/src/shims/form.tsx b/packages/vinext/src/shims/form.tsx index f78d23e60..6e60d5a22 100644 --- a/packages/vinext/src/shims/form.tsx +++ b/packages/vinext/src/shims/form.tsx @@ -19,6 +19,7 @@ */ import { forwardRef, useActionState, type FormHTMLAttributes, type ForwardedRef } from "react"; +import { navigateClientSide } from "./navigation.js"; import { isDangerousScheme } from "./url-safety.js"; import { toSameOriginPath } from "./url-utils.js"; @@ -223,13 +224,9 @@ const Form = forwardRef(function Form(props: FormProps, ref: ForwardedRef { if (typeof window.__VINEXT_RSC_NAVIGATE__ === "function") { - // App Router: prefetch the RSC payload and store in cache - fetch(rscUrl, { - headers: { Accept: "text/x-component" }, - credentials: "include", - priority: "low" as any, - // @ts-expect-error — purpose is a valid fetch option in some browsers - purpose: "prefetch", - }) - .then((response) => { - if (response.ok) { - storePrefetchResponse(rscUrl, response); - } else { - // Non-ok response: allow retry on next viewport intersection - prefetched.delete(rscUrl); - } - }) - .catch(() => { - // Network error: allow retry on next viewport intersection - prefetched.delete(rscUrl); - }); + prefetchRscResponse( + rscUrl, + fetch(rscUrl, { + headers: { Accept: "text/x-component" }, + credentials: "include", + priority: "low" as any, + // @ts-expect-error — purpose is a valid fetch option in some browsers + purpose: "prefetch", + }), + ); } else if ((window.__NEXT_DATA__ as VinextNextData | undefined)?.__vinext?.pageModuleUrl) { // Pages Router: inject a prefetch link for the target page module // We can't easily resolve the target page's module URL from the Link, @@ -436,48 +401,12 @@ const Link = forwardRef(function Link( } } - // Save scroll position for back/forward restoration - if (!replace) { - const state = window.history.state ?? {}; - window.history.replaceState( - { ...state, __vinext_scrollX: window.scrollX, __vinext_scrollY: window.scrollY }, - "", - ); - } - - // Hash-only change: update URL and scroll to target, skip RSC fetch - if (typeof window !== "undefined" && isHashOnlyChange(absoluteFullHref)) { - const hash = absoluteFullHref.includes("#") - ? absoluteFullHref.slice(absoluteFullHref.indexOf("#")) - : ""; - if (replace) { - window.history.replaceState(null, "", absoluteFullHref); - } else { - window.history.pushState(null, "", absoluteFullHref); - } - if (scroll) { - scrollToHash(hash); - } - return; - } - - // Extract hash for scroll-after-navigation - const hashIdx = absoluteFullHref.indexOf("#"); - const hash = hashIdx !== -1 ? absoluteFullHref.slice(hashIdx) : ""; - - // Try RSC navigation first (App Router), then Pages Router + // App Router: delegate to navigateClientSide which handles scroll save, + // hash-only changes, RSC fetch, and two-phase URL commit. if (typeof window.__VINEXT_RSC_NAVIGATE__ === "function") { - // App Router: push/replace history state, then fetch RSC stream. - // Await the RSC navigate so scroll-to-top happens after the new - // content is committed to the DOM (prevents flash of old page at top). - if (replace) { - window.history.replaceState(null, "", absoluteFullHref); - } else { - window.history.pushState(null, "", absoluteFullHref); - } setPending(true); try { - await window.__VINEXT_RSC_NAVIGATE__(absoluteFullHref); + await navigateClientSide(navigateHref, replace ? "replace" : "push", scroll); } finally { if (mountedRef.current) setPending(false); } @@ -502,14 +431,6 @@ const Link = forwardRef(function Link( window.dispatchEvent(new PopStateEvent("popstate")); } } - - if (scroll) { - if (hash) { - scrollToHash(hash); - } else { - window.scrollTo(0, 0); - } - } }; // Remove props that shouldn't be on diff --git a/packages/vinext/src/shims/navigation.ts b/packages/vinext/src/shims/navigation.ts index 2e23b5b8c..5002b9790 100644 --- a/packages/vinext/src/shims/navigation.ts +++ b/packages/vinext/src/shims/navigation.ts @@ -189,8 +189,17 @@ export const MAX_PREFETCH_CACHE_SIZE = 50; /** TTL for prefetch cache entries in ms (matches Next.js static prefetch TTL). */ export const PREFETCH_CACHE_TTL = 30_000; +/** A buffered RSC response stored as an ArrayBuffer for replay. */ +export interface CachedRscResponse { + buffer: ArrayBuffer; + contentType: string; + url: string; +} + export interface PrefetchCacheEntry { - response: Response; + response?: Response; + snapshot?: CachedRscResponse; + pending?: Promise; timestamp: number; } @@ -263,37 +272,188 @@ export function storePrefetchResponse(rscUrl: string, response: Response): void cache.set(rscUrl, { response, timestamp: now }); } -// Client navigation listeners +/** + * Snapshot an RSC response to an ArrayBuffer for caching and replay. + * Consumes the response body and stores it with content-type and URL metadata. + */ +export async function snapshotRscResponse(response: Response): Promise { + const buffer = await response.arrayBuffer(); + return { + buffer, + contentType: response.headers.get("content-type") ?? "text/x-component", + url: response.url, + }; +} + +/** + * Reconstruct a Response from a cached RSC snapshot. + * Creates a new Response with the original ArrayBuffer so createFromFetch + * can consume the stream from scratch. + */ +export function restoreRscResponse(cached: CachedRscResponse): Response { + return new Response(cached.buffer.slice(0), { + status: 200, + headers: { "content-type": cached.contentType }, + }); +} + +/** + * Prefetch an RSC response and snapshot it for later consumption. + * Stores the in-flight promise so immediate clicks can await it instead + * of firing a duplicate fetch. + */ +export function prefetchRscResponse(rscUrl: string, fetchPromise: Promise): void { + const cache = getPrefetchCache(); + const prefetched = getPrefetchedUrls(); + const now = Date.now(); + + const entry: PrefetchCacheEntry = { timestamp: now }; + + entry.pending = fetchPromise + .then(async (response) => { + if (response.ok) { + entry.snapshot = await snapshotRscResponse(response); + } else { + prefetched.delete(rscUrl); + cache.delete(rscUrl); + } + }) + .catch(() => { + prefetched.delete(rscUrl); + cache.delete(rscUrl); + }) + .finally(() => { + entry.pending = undefined; + }); + + cache.set(rscUrl, entry); +} + +/** + * Consume a prefetched response for a given rscUrl. + * Only returns settled (non-pending) snapshots synchronously. + * Returns null if the entry is still in flight or doesn't exist. + */ +export function consumePrefetchResponse(rscUrl: string): CachedRscResponse | null { + const cache = getPrefetchCache(); + const entry = cache.get(rscUrl); + if (!entry) return null; + + // Don't consume pending entries — let the navigation fetch independently. + if (entry.pending) return null; + + cache.delete(rscUrl); + getPrefetchedUrls().delete(rscUrl); + + if (entry.snapshot) return entry.snapshot; + + // Legacy: raw Response entries (from storePrefetchResponse) + // These can't be consumed synchronously as snapshots — skip them. + // The navigation code will re-fetch. + return null; +} + +// --------------------------------------------------------------------------- +// Client navigation state — stored on a Symbol.for global to survive +// multiple Vite module instances loading this file through different IDs. +// --------------------------------------------------------------------------- + type NavigationListener = () => void; -const _listeners: Set = new Set(); +const _CLIENT_NAV_STATE_KEY = Symbol.for("vinext.clientNavigationState"); + +type ClientNavigationState = { + listeners: Set; + cachedSearch: string; + cachedReadonlySearchParams: ReadonlyURLSearchParams; + cachedPathname: string; + clientParams: Record; + clientParamsJson: string; + pendingClientParams: Record | null; + pendingClientParamsJson: string | null; + originalPushState: typeof window.history.pushState; + originalReplaceState: typeof window.history.replaceState; + patchInstalled: boolean; + hasPendingNavigationUpdate: boolean; + suppressUrlNotifyCount: number; + navigationSnapshotActiveCount: number; +}; + +type ClientNavigationGlobal = typeof globalThis & { + [_CLIENT_NAV_STATE_KEY]?: ClientNavigationState; +}; + +function getClientNavigationState(): ClientNavigationState | null { + if (isServer) return null; + + const globalState = window as ClientNavigationGlobal; + if (!globalState[_CLIENT_NAV_STATE_KEY]) { + globalState[_CLIENT_NAV_STATE_KEY] = { + listeners: new Set(), + cachedSearch: window.location.search, + cachedReadonlySearchParams: new ReadonlyURLSearchParams(window.location.search), + cachedPathname: stripBasePath(window.location.pathname, __basePath), + clientParams: {}, + clientParamsJson: "{}", + pendingClientParams: null, + pendingClientParamsJson: null, + originalPushState: window.history.pushState.bind(window.history), + originalReplaceState: window.history.replaceState.bind(window.history), + patchInstalled: false, + hasPendingNavigationUpdate: false, + suppressUrlNotifyCount: 0, + navigationSnapshotActiveCount: 0, + }; + } + + return globalState[_CLIENT_NAV_STATE_KEY]!; +} -function notifyListeners(): void { - for (const fn of _listeners) fn(); +function notifyNavigationListeners(): void { + const state = getClientNavigationState(); + if (!state) return; + for (const fn of state.listeners) fn(); } // Cached URLSearchParams, pathname, etc. for referential stability // useSyncExternalStore compares snapshots with Object.is — avoid creating // new instances on every render (infinite re-renders). -let _cachedSearch = !isServer ? window.location.search : ""; -let _cachedReadonlySearchParams = new ReadonlyURLSearchParams(_cachedSearch); let _cachedEmptyServerSearchParams: ReadonlyURLSearchParams | null = null; -let _cachedPathname = !isServer ? stripBasePath(window.location.pathname, __basePath) : "/"; function getPathnameSnapshot(): string { - const current = stripBasePath(window.location.pathname, __basePath); - if (current !== _cachedPathname) { - _cachedPathname = current; - } - return _cachedPathname; + return getClientNavigationState()?.cachedPathname ?? "/"; } +let _cachedEmptyClientSearchParams: ReadonlyURLSearchParams | null = null; + function getSearchParamsSnapshot(): ReadonlyURLSearchParams { - const current = window.location.search; - if (current !== _cachedSearch) { - _cachedSearch = current; - _cachedReadonlySearchParams = new ReadonlyURLSearchParams(current); + const cached = getClientNavigationState()?.cachedReadonlySearchParams; + if (cached) return cached; + if (_cachedEmptyClientSearchParams === null) { + _cachedEmptyClientSearchParams = new ReadonlyURLSearchParams(); } - return _cachedReadonlySearchParams; + return _cachedEmptyClientSearchParams; +} + +function syncCommittedUrlStateFromLocation(): boolean { + const state = getClientNavigationState(); + if (!state) return false; + + let changed = false; + + const pathname = stripBasePath(window.location.pathname, __basePath); + if (pathname !== state.cachedPathname) { + state.cachedPathname = pathname; + changed = true; + } + + const search = window.location.search; + if (search !== state.cachedSearch) { + state.cachedSearch = search; + state.cachedReadonlySearchParams = new ReadonlyURLSearchParams(search); + changed = true; + } + + return changed; } function getServerSearchParamsSnapshot(): ReadonlyURLSearchParams { @@ -312,30 +472,129 @@ function getServerSearchParamsSnapshot(): ReadonlyURLSearchParams { return _cachedEmptyServerSearchParams; } +// --------------------------------------------------------------------------- +// Navigation snapshot activation flag +// +// The render snapshot context provides pending URL values during transitions. +// After the transition commits, the snapshot becomes stale and must NOT shadow +// subsequent external URL changes (user pushState/replaceState). This flag +// tracks whether a navigation transition is in progress — hooks only prefer +// the snapshot while it's active. +// --------------------------------------------------------------------------- + +/** + * Mark a navigation snapshot as active. Called before startTransition + * in renderNavigationPayload. While active, hooks prefer the snapshot + * context value over useSyncExternalStore. Uses a counter (not boolean) + * to handle overlapping navigations — rapid clicks can interleave + * activate/deactivate if multiple transitions are in flight. + */ +export function activateNavigationSnapshot(): void { + const state = getClientNavigationState(); + if (state) state.navigationSnapshotActiveCount++; +} + // Track client-side params (set during RSC hydration/navigation) // We cache the params object for referential stability — only create a new // object when the params actually change (shallow key/value comparison). const _EMPTY_PARAMS: Record = {}; -let _clientParams: Record = _EMPTY_PARAMS; -let _clientParamsJson = "{}"; + +// --------------------------------------------------------------------------- +// Client navigation render snapshot — provides pending URL values to hooks +// during a startTransition so they see the destination, not the stale URL. +// --------------------------------------------------------------------------- + +export interface ClientNavigationRenderSnapshot { + pathname: string; + searchParams: ReadonlyURLSearchParams; + params: Record; +} + +const _CLIENT_NAV_RENDER_CTX_KEY = Symbol.for("vinext.clientNavigationRenderContext"); +type _ClientNavRenderGlobal = typeof globalThis & { + [_CLIENT_NAV_RENDER_CTX_KEY]?: React.Context | null; +}; + +export function getClientNavigationRenderContext(): React.Context | null { + if (typeof React.createContext !== "function") return null; + + const globalState = globalThis as _ClientNavRenderGlobal; + if (!globalState[_CLIENT_NAV_RENDER_CTX_KEY]) { + globalState[_CLIENT_NAV_RENDER_CTX_KEY] = + React.createContext(null); + } + + return globalState[_CLIENT_NAV_RENDER_CTX_KEY] ?? null; +} + +function useClientNavigationRenderSnapshot(): ClientNavigationRenderSnapshot | null { + const ctx = getClientNavigationRenderContext(); + if (!ctx || typeof React.useContext !== "function") return null; + try { + return React.useContext(ctx); + } catch { + return null; + } +} + +export function createClientNavigationRenderSnapshot( + href: string, + params: Record, +): ClientNavigationRenderSnapshot { + const origin = typeof window !== "undefined" ? window.location.origin : "http://localhost"; + const url = new URL(href, origin); + + return { + pathname: stripBasePath(url.pathname, __basePath), + searchParams: new ReadonlyURLSearchParams(url.search), + params, + }; +} + +// Module-level fallback for environments without window (tests, SSR). +let _fallbackClientParams: Record = _EMPTY_PARAMS; +let _fallbackClientParamsJson = "{}"; export function setClientParams(params: Record): void { + const state = getClientNavigationState(); + if (!state) { + const json = JSON.stringify(params); + if (json !== _fallbackClientParamsJson) { + _fallbackClientParams = params; + _fallbackClientParamsJson = json; + } + return; + } + const json = JSON.stringify(params); - if (json !== _clientParamsJson) { - _clientParams = params; - _clientParamsJson = json; - // Notify useSyncExternalStore subscribers so useParams() re-renders. - notifyListeners(); + if (json !== state.clientParamsJson) { + state.clientParams = params; + state.clientParamsJson = json; + state.pendingClientParams = null; + state.pendingClientParamsJson = null; + notifyNavigationListeners(); + } +} + +export function replaceClientParamsWithoutNotify(params: Record): void { + const state = getClientNavigationState(); + if (!state) return; + + const json = JSON.stringify(params); + if (json !== state.clientParamsJson && json !== state.pendingClientParamsJson) { + state.pendingClientParams = params; + state.pendingClientParamsJson = json; + state.hasPendingNavigationUpdate = true; } } /** Get the current client params (for testing referential stability). */ export function getClientParams(): Record { - return _clientParams; + return getClientNavigationState()?.clientParams ?? _fallbackClientParams; } function getClientParamsSnapshot(): Record { - return _clientParams; + return getClientNavigationState()?.clientParams ?? _EMPTY_PARAMS; } function getServerParamsSnapshot(): Record { @@ -343,9 +602,12 @@ function getServerParamsSnapshot(): Record { } function subscribeToNavigation(cb: () => void): () => void { - _listeners.add(cb); + const state = getClientNavigationState(); + if (!state) return () => {}; + + state.listeners.add(cb); return () => { - _listeners.delete(cb); + state.listeners.delete(cb); }; } @@ -363,12 +625,21 @@ export function usePathname(): string { // Return a safe fallback — the client will hydrate with the real value. return _getServerContext()?.pathname ?? "/"; } + const renderSnapshot = useClientNavigationRenderSnapshot(); // Client-side: use the hook system for reactivity - return React.useSyncExternalStore( + const pathname = React.useSyncExternalStore( subscribeToNavigation, getPathnameSnapshot, () => _getServerContext()?.pathname ?? "/", ); + // Prefer the render snapshot during an active navigation transition so + // hooks return the pending URL, not the stale committed one. After commit, + // fall through to useSyncExternalStore so user pushState/replaceState + // calls are immediately reflected. + if (renderSnapshot && (getClientNavigationState()?.navigationSnapshotActiveCount ?? 0) > 0) { + return renderSnapshot.pathname; + } + return pathname; } /** @@ -380,11 +651,16 @@ export function useSearchParams(): ReadonlyURLSearchParams { // Return a safe fallback — the client will hydrate with the real value. return getServerSearchParamsSnapshot(); } - return React.useSyncExternalStore( + const renderSnapshot = useClientNavigationRenderSnapshot(); + const searchParams = React.useSyncExternalStore( subscribeToNavigation, getSearchParamsSnapshot, getServerSearchParamsSnapshot, ); + if (renderSnapshot && (getClientNavigationState()?.navigationSnapshotActiveCount ?? 0) > 0) { + return renderSnapshot.searchParams; + } + return searchParams; } /** @@ -397,11 +673,16 @@ export function useParams< // During SSR of "use client" components, the navigation context may not be set. return (_getServerContext()?.params ?? _EMPTY_PARAMS) as T; } - return React.useSyncExternalStore( + const renderSnapshot = useClientNavigationRenderSnapshot(); + const params = React.useSyncExternalStore( subscribeToNavigation, getClientParamsSnapshot as () => T, getServerParamsSnapshot as () => T, ); + if (renderSnapshot && (getClientNavigationState()?.navigationSnapshotActiveCount ?? 0) > 0) { + return renderSnapshot.params as T; + } + return params; } /** @@ -441,28 +722,78 @@ function scrollToHash(hash: string): void { } } -/** - * Reference to the native history.replaceState before patching. - * Used internally to avoid triggering the interception for internal operations - * (e.g. saving scroll position shouldn't cause re-renders). - * Captured before the history method patching at the bottom of this module. - */ -const _nativeReplaceState: typeof window.history.replaceState | null = !isServer - ? window.history.replaceState.bind(window.history) - : null; +// --------------------------------------------------------------------------- +// History method wrappers — suppress notifications for internal updates +// --------------------------------------------------------------------------- + +function withSuppressedUrlNotifications(fn: () => T): T { + const state = getClientNavigationState(); + if (!state) { + return fn(); + } + + state.suppressUrlNotifyCount += 1; + try { + return fn(); + } finally { + state.suppressUrlNotifyCount -= 1; + } +} + +export function commitClientNavigationState(): void { + if (isServer) return; + const state = getClientNavigationState(); + if (!state) return; + + state.navigationSnapshotActiveCount = Math.max(0, state.navigationSnapshotActiveCount - 1); + + const urlChanged = syncCommittedUrlStateFromLocation(); + if (state.pendingClientParams !== null && state.pendingClientParamsJson !== null) { + state.clientParams = state.pendingClientParams; + state.clientParamsJson = state.pendingClientParamsJson; + state.pendingClientParams = null; + state.pendingClientParamsJson = null; + } + const shouldNotify = urlChanged || state.hasPendingNavigationUpdate; + state.hasPendingNavigationUpdate = false; + + if (shouldNotify) { + notifyNavigationListeners(); + } +} + +export function pushHistoryStateWithoutNotify( + data: unknown, + unused: string, + url?: string | URL | null, +): void { + withSuppressedUrlNotifications(() => { + const state = getClientNavigationState(); + state?.originalPushState.call(window.history, data, unused, url); + }); +} + +export function replaceHistoryStateWithoutNotify( + data: unknown, + unused: string, + url?: string | URL | null, +): void { + withSuppressedUrlNotifications(() => { + const state = getClientNavigationState(); + state?.originalReplaceState.call(window.history, data, unused, url); + }); +} /** * Save the current scroll position into the current history state. * Called before every navigation to enable scroll restoration on back/forward. * - * Uses _nativeReplaceState to avoid triggering the history.replaceState - * interception (which would cause spurious re-renders from notifyListeners). + * Uses replaceHistoryStateWithoutNotify to avoid triggering the patched + * history.replaceState interception (which would cause spurious re-renders). */ function saveScrollPosition(): void { - if (!_nativeReplaceState) return; const state = window.history.state ?? {}; - _nativeReplaceState.call( - window.history, + replaceHistoryStateWithoutNotify( { ...state, __vinext_scrollX: window.scrollX, __vinext_scrollY: window.scrollY }, "", ); @@ -515,7 +846,7 @@ function restoreScrollPosition(state: unknown): void { /** * Navigate to a URL, handling external URLs, hash-only changes, and RSC navigation. */ -async function navigateImpl( +export async function navigateClientSide( href: string, mode: "push" | "replace", scroll: boolean, @@ -547,11 +878,11 @@ async function navigateImpl( if (isHashOnlyChange(fullHref)) { const hash = fullHref.includes("#") ? fullHref.slice(fullHref.indexOf("#")) : ""; if (mode === "replace") { - window.history.replaceState(null, "", fullHref); + replaceHistoryStateWithoutNotify(null, "", fullHref); } else { - window.history.pushState(null, "", fullHref); + pushHistoryStateWithoutNotify(null, "", fullHref); } - notifyListeners(); + commitClientNavigationState(); if (scroll) { scrollToHash(hash); } @@ -562,18 +893,18 @@ async function navigateImpl( const hashIdx = fullHref.indexOf("#"); const hash = hashIdx !== -1 ? fullHref.slice(hashIdx) : ""; - if (mode === "replace") { - window.history.replaceState(null, "", fullHref); - } else { - window.history.pushState(null, "", fullHref); - } - notifyListeners(); - // Trigger RSC re-fetch if available, and wait for the new content to render // before scrolling. This prevents the old page from visibly jumping to the // top before the new content paints. if (typeof window.__VINEXT_RSC_NAVIGATE__ === "function") { - await window.__VINEXT_RSC_NAVIGATE__(fullHref); + await window.__VINEXT_RSC_NAVIGATE__(fullHref, 0, "navigate", mode); + } else { + if (mode === "replace") { + replaceHistoryStateWithoutNotify(null, "", fullHref); + } else { + pushHistoryStateWithoutNotify(null, "", fullHref); + } + commitClientNavigationState(); } if (scroll) { @@ -588,7 +919,7 @@ async function navigateImpl( // --------------------------------------------------------------------------- // App Router router singleton // -// All methods close over module-level state (navigateImpl, withBasePath, etc.) +// All methods close over module-level state (navigateClientSide, withBasePath, etc.) // and carry no per-render data, so the object can be created once and reused. // Next.js returns the same router reference on every call to useRouter(), which // matters for components that rely on referential equality (e.g. useMemo / @@ -598,11 +929,11 @@ async function navigateImpl( const _appRouter = { push(href: string, options?: { scroll?: boolean }): void { if (isServer) return; - void navigateImpl(href, "push", options?.scroll !== false); + void navigateClientSide(href, "push", options?.scroll !== false); }, replace(href: string, options?: { scroll?: boolean }): void { if (isServer) return; - void navigateImpl(href, "replace", options?.scroll !== false); + void navigateClientSide(href, "replace", options?.scroll !== false); }, back(): void { if (isServer) return; @@ -616,7 +947,7 @@ const _appRouter = { if (isServer) return; // Re-fetch the current page's RSC stream if (typeof window.__VINEXT_RSC_NAVIGATE__ === "function") { - void window.__VINEXT_RSC_NAVIGATE__(window.location.href); + void window.__VINEXT_RSC_NAVIGATE__(window.location.href, 0, "refresh"); } }, prefetch(href: string): void { @@ -627,23 +958,14 @@ const _appRouter = { const prefetched = getPrefetchedUrls(); if (prefetched.has(rscUrl)) return; prefetched.add(rscUrl); - fetch(rscUrl, { - headers: { Accept: "text/x-component" }, - credentials: "include", - priority: "low" as RequestInit["priority"], - }) - .then((response) => { - if (response.ok) { - storePrefetchResponse(rscUrl, response); - } else { - // Non-ok response: allow retry on next prefetch() call - prefetched.delete(rscUrl); - } - }) - .catch(() => { - // Network error: allow retry on next prefetch() call - prefetched.delete(rscUrl); - }); + prefetchRscResponse( + rscUrl, + fetch(rscUrl, { + headers: { Accept: "text/x-component" }, + credentials: "include", + priority: "low" as RequestInit["priority"], + }), + ); }, }; @@ -862,47 +1184,39 @@ export function unauthorized(): never { // Helpers // --------------------------------------------------------------------------- -// React hooks are imported at the top level via ESM. - // Listen for popstate on the client if (!isServer) { - window.addEventListener("popstate", (event) => { - notifyListeners(); - // Restore scroll position for back/forward navigation - restoreScrollPosition(event.state); - }); + const state = getClientNavigationState(); + if (state && !state.patchInstalled) { + state.patchInstalled = true; + + window.addEventListener("popstate", (event) => { + if (typeof window.__VINEXT_RSC_NAVIGATE__ !== "function") { + commitClientNavigationState(); + restoreScrollPosition(event.state); + } + }); - // --------------------------------------------------------------------------- - // history.pushState / replaceState interception (shallow routing) - // - // Next.js intercepts these native methods so that when user code calls - // `window.history.pushState(null, '', '/new-path?filter=abc')` directly, - // React hooks like usePathname() and useSearchParams() re-render with - // the new URL. This is the foundation for shallow routing patterns - // (filter UIs, tabs, URL search param state, etc.). - // - // We wrap the original methods, call through to the native implementation, - // then notify our listener system so useSyncExternalStore picks up the - // URL change. - // --------------------------------------------------------------------------- - const originalPushState = window.history.pushState.bind(window.history); - const originalReplaceState = window.history.replaceState.bind(window.history); - - window.history.pushState = function patchedPushState( - data: unknown, - unused: string, - url?: string | URL | null, - ): void { - originalPushState(data, unused, url); - notifyListeners(); - }; + window.history.pushState = function patchedPushState( + data: unknown, + unused: string, + url?: string | URL | null, + ): void { + state.originalPushState.call(window.history, data, unused, url); + if (state.suppressUrlNotifyCount === 0) { + commitClientNavigationState(); + } + }; - window.history.replaceState = function patchedReplaceState( - data: unknown, - unused: string, - url?: string | URL | null, - ): void { - originalReplaceState(data, unused, url); - notifyListeners(); - }; + window.history.replaceState = function patchedReplaceState( + data: unknown, + unused: string, + url?: string | URL | null, + ): void { + state.originalReplaceState.call(window.history, data, unused, url); + if (state.suppressUrlNotifyCount === 0) { + commitClientNavigationState(); + } + }; + } } diff --git a/tests/e2e/app-router/navigation-regressions.spec.ts b/tests/e2e/app-router/navigation-regressions.spec.ts new file mode 100644 index 000000000..15eb1ab9a --- /dev/null +++ b/tests/e2e/app-router/navigation-regressions.spec.ts @@ -0,0 +1,188 @@ +import { test, expect } from "@playwright/test"; + +const BASE = "http://localhost:4174"; + +async function waitForHydration(page: import("@playwright/test").Page) { + await expect(async () => { + const ready = await page.evaluate(() => !!(window as any).__VINEXT_RSC_ROOT__); + expect(ready).toBe(true); + }).toPass({ timeout: 10_000 }); +} + +test.describe("Navigation regression tests (#652 Firefox hang fix)", () => { + test("cross-route navigation completes without hanging", async ({ page }) => { + await page.goto(`${BASE}/nav-flash/link-sync`); + await waitForHydration(page); + await expect(page.locator("#page-title")).toHaveText("All Items"); + + // Set a marker to verify no full page reload + await page.evaluate(() => { + (window as any).__NAV_MARKER__ = true; + }); + + // Navigate to a different route (cross-route) + await page.click("#link-list"); + await expect(page.locator("#list-title")).toHaveText("Nav Flash List", { timeout: 10_000 }); + + // Verify URL changed + expect(page.url()).toBe(`${BASE}/nav-flash/list`); + + // Verify no full page reload + const marker = await page.evaluate(() => (window as any).__NAV_MARKER__); + expect(marker).toBe(true); + }); + + test("same-route navigation (search param change) works", async ({ page }) => { + await page.goto(`${BASE}/nav-flash/link-sync`); + await waitForHydration(page); + await expect(page.locator("#page-title")).toHaveText("All Items"); + + await page.evaluate(() => { + (window as any).__NAV_MARKER__ = true; + }); + + // Navigate within the same route (search param change) + await page.click("#link-active"); + await expect(page.locator("#page-title")).toHaveText("Filtered: active", { timeout: 10_000 }); + expect(page.url()).toContain("filter=active"); + + // Hook values should be in sync + await expect(page.locator("#hook-filter")).toHaveText("filter: active"); + + // No full page reload + const marker = await page.evaluate(() => (window as any).__NAV_MARKER__); + expect(marker).toBe(true); + }); + + test("back/forward navigation works after cross-route nav", async ({ page }) => { + await page.goto(`${BASE}/nav-flash/link-sync`); + await waitForHydration(page); + await expect(page.locator("#page-title")).toHaveText("All Items"); + + // Navigate to list page + await page.click("#link-list"); + await expect(page.locator("#list-title")).toHaveText("Nav Flash List", { timeout: 10_000 }); + + // Go back + await page.goBack(); + await expect(page.locator("#page-title")).toHaveText("All Items", { timeout: 10_000 }); + expect(page.url()).toBe(`${BASE}/nav-flash/link-sync`); + + // Go forward + await page.goForward(); + await expect(page.locator("#list-title")).toHaveText("Nav Flash List", { timeout: 10_000 }); + expect(page.url()).toBe(`${BASE}/nav-flash/list`); + }); + + test("rapid same-route navigation settles correctly", async ({ page }) => { + await page.goto(`${BASE}/nav-flash/link-sync`); + await waitForHydration(page); + await expect(page.locator("#page-title")).toHaveText("All Items"); + + // Rapidly click between filters + await page.click("#link-active"); + await page.click("#link-completed"); + + // The final state should reflect the last click + await expect(page.locator("#page-title")).toHaveText("Filtered: completed", { + timeout: 10_000, + }); + expect(page.url()).toContain("filter=completed"); + await expect(page.locator("#hook-filter")).toHaveText("filter: completed"); + }); + + test("cross-route then same-route navigation works", async ({ page }) => { + await page.goto(`${BASE}/nav-flash/list`); + await waitForHydration(page); + await expect(page.locator("#list-title")).toHaveText("Nav Flash List"); + + // Cross-route: list -> query-sync + await page.click("#to-query-sync"); + await expect(page.locator("#query-title")).toHaveText("Search", { timeout: 10_000 }); + + // Same-route: change query param + await page.click("#link-react"); + await expect(page.locator("#query-title")).toHaveText("Search: react", { timeout: 10_000 }); + await expect(page.locator("#hook-query")).toHaveText("q: react"); + expect(page.url()).toContain("q=react"); + }); + + test("usePathname reflects correct value during cross-route navigation", async ({ page }) => { + await page.goto(`${BASE}/nav-flash/link-sync`); + await waitForHydration(page); + + await expect(page.locator("#hook-pathname")).toHaveText("pathname: /nav-flash/link-sync"); + + // Navigate to query-sync page via list + await page.click("#link-list"); + await expect(page.locator("#list-title")).toHaveText("Nav Flash List", { timeout: 10_000 }); + + // Navigate to query-sync + await page.click("#to-query-sync"); + await expect(page.locator("#hook-pathname")).toHaveText("pathname: /nav-flash/query-sync", { + timeout: 10_000, + }); + }); + + test("useParams reflects correct value after param change", async ({ page }) => { + await page.goto(`${BASE}/nav-flash/param-sync/active`); + await waitForHydration(page); + + await expect(page.locator("#param-title")).toHaveText("Filter: active"); + await expect(page.locator("#hook-params")).toHaveText("params.filter: active"); + + // Navigate to different param value + await page.click("#link-completed"); + await expect(page.locator("#param-title")).toHaveText("Filter: completed", { timeout: 10_000 }); + await expect(page.locator("#hook-params")).toHaveText("params.filter: completed"); + }); + + test("navigation from home page to nav-flash routes", async ({ page }) => { + await page.goto(`${BASE}/`); + await waitForHydration(page); + await expect(page.locator("h1")).toHaveText("Welcome to App Router"); + + // Navigate to nav-flash test + await page.click('[data-testid="nav-flash-link"]'); + await expect(page.locator("#page-title")).toHaveText("All Items", { timeout: 10_000 }); + + // Verify the page rendered fully + await expect(page.locator("#filter-links")).toBeVisible(); + }); + + test("cross-route round trip preserves SPA state", async ({ page }) => { + await page.goto(`${BASE}/nav-flash/link-sync`); + await waitForHydration(page); + + await page.evaluate(() => { + (window as any).__ROUND_TRIP_MARKER__ = "alive"; + }); + + // Go to list + await page.click("#link-list"); + await expect(page.locator("#list-title")).toHaveText("Nav Flash List", { timeout: 10_000 }); + + // Come back + await page.click("#back-to-sync"); + await expect(page.locator("#page-title")).toHaveText("All Items", { timeout: 10_000 }); + + // Marker should survive (no full reload) + const marker = await page.evaluate(() => (window as any).__ROUND_TRIP_MARKER__); + expect(marker).toBe("alive"); + }); + + test("provider page cross-route navigation between dynamic params", async ({ page }) => { + await page.goto(`${BASE}/nav-flash/provider/1`); + await waitForHydration(page); + await expect(page.locator("#provider-title")).toHaveText("Provider 1"); + + // Navigate to provider 2 (cross-route: different dynamic param) + await page.click("#link-p2"); + await expect(page.locator("#provider-title")).toHaveText("Provider 2", { timeout: 10_000 }); + expect(page.url()).toBe(`${BASE}/nav-flash/provider/2`); + + // Navigate to list (different route entirely) + await page.click("#link-list"); + await expect(page.locator("#list-title")).toHaveText("Nav Flash List", { timeout: 10_000 }); + }); +}); diff --git a/tests/fixtures/app-basic/app/nav-flash/link-sync/FilterLinks.tsx b/tests/fixtures/app-basic/app/nav-flash/link-sync/FilterLinks.tsx new file mode 100644 index 000000000..4e6de8f96 --- /dev/null +++ b/tests/fixtures/app-basic/app/nav-flash/link-sync/FilterLinks.tsx @@ -0,0 +1,29 @@ +"use client"; + +import Link from "next/link"; +import { usePathname, useSearchParams } from "next/navigation"; + +export function FilterLinks() { + const pathname = usePathname(); + const searchParams = useSearchParams(); + const current = searchParams.get("filter") ?? ""; + + return ( + + ); +} diff --git a/tests/fixtures/app-basic/app/nav-flash/link-sync/page.tsx b/tests/fixtures/app-basic/app/nav-flash/link-sync/page.tsx new file mode 100644 index 000000000..a43f4b376 --- /dev/null +++ b/tests/fixtures/app-basic/app/nav-flash/link-sync/page.tsx @@ -0,0 +1,37 @@ +import { Suspense } from "react"; +import { FilterLinks } from "./FilterLinks"; + +async function SlowContent({ filter }: { filter: string }) { + await new Promise((r) => setTimeout(r, 200)); + const items = filter + ? [{ id: 1, name: `Filtered: ${filter}` }] + : [ + { id: 1, name: "Item A" }, + { id: 2, name: "Item B" }, + { id: 3, name: "Item C" }, + ]; + return ( +
    + {items.map((i) => ( +
  • {i.name}
    • + ))} +
+ ); +} + +export default async function LinkSyncPage({ + searchParams, +}: { + searchParams: Promise<{ filter?: string }>; +}) { + const { filter = "" } = await searchParams; + return ( +
+

{filter ? `Filtered: ${filter}` : "All Items"}

+ + Loading...
}> + + + + ); +} diff --git a/tests/fixtures/app-basic/app/nav-flash/list/page.tsx b/tests/fixtures/app-basic/app/nav-flash/list/page.tsx new file mode 100644 index 000000000..291a8e59b --- /dev/null +++ b/tests/fixtures/app-basic/app/nav-flash/list/page.tsx @@ -0,0 +1,21 @@ +import Link from "next/link"; + +export default function ListPage() { + return ( +
+

Nav Flash List

+ +
+ ); +} diff --git a/tests/fixtures/app-basic/app/nav-flash/param-sync/[filter]/FilterControls.tsx b/tests/fixtures/app-basic/app/nav-flash/param-sync/[filter]/FilterControls.tsx new file mode 100644 index 000000000..c41ffd6bc --- /dev/null +++ b/tests/fixtures/app-basic/app/nav-flash/param-sync/[filter]/FilterControls.tsx @@ -0,0 +1,22 @@ +"use client"; + +import Link from "next/link"; +import { useParams, usePathname } from "next/navigation"; + +export function FilterControls() { + const params = useParams(); + const pathname = usePathname(); + + return ( +
+

pathname: {pathname}

+

params.filter: {String(params.filter ?? "")}

+ + Active + + + Completed + +
+ ); +} diff --git a/tests/fixtures/app-basic/app/nav-flash/param-sync/[filter]/page.tsx b/tests/fixtures/app-basic/app/nav-flash/param-sync/[filter]/page.tsx new file mode 100644 index 000000000..145e4aaa7 --- /dev/null +++ b/tests/fixtures/app-basic/app/nav-flash/param-sync/[filter]/page.tsx @@ -0,0 +1,20 @@ +import { Suspense } from "react"; +import { FilterControls } from "./FilterControls"; + +async function FilteredContent({ filter }: { filter: string }) { + await new Promise((r) => setTimeout(r, 150)); + return

Showing: {filter}

; +} + +export default async function ParamSyncPage({ params }: { params: Promise<{ filter: string }> }) { + const { filter } = await params; + return ( +
+

Filter: {filter}

+ + Loading filter...
}> + + + + ); +} diff --git a/tests/fixtures/app-basic/app/nav-flash/provider/[id]/page.tsx b/tests/fixtures/app-basic/app/nav-flash/provider/[id]/page.tsx new file mode 100644 index 000000000..ecb776330 --- /dev/null +++ b/tests/fixtures/app-basic/app/nav-flash/provider/[id]/page.tsx @@ -0,0 +1,19 @@ +import Link from "next/link"; + +export default async function ProviderPage({ params }: { params: Promise<{ id: string }> }) { + const { id } = await params; + return ( +
+

Provider {id}

+ + Provider 1 + + + Provider 2 + + + Back to List + +
+ ); +} diff --git a/tests/fixtures/app-basic/app/nav-flash/query-sync/FilterControls.tsx b/tests/fixtures/app-basic/app/nav-flash/query-sync/FilterControls.tsx new file mode 100644 index 000000000..5fbef4706 --- /dev/null +++ b/tests/fixtures/app-basic/app/nav-flash/query-sync/FilterControls.tsx @@ -0,0 +1,26 @@ +"use client"; + +import Link from "next/link"; +import { usePathname, useSearchParams } from "next/navigation"; + +export function FilterControls() { + const pathname = usePathname(); + const searchParams = useSearchParams(); + const filter = searchParams.get("q") ?? ""; + + return ( +
+

pathname: {pathname}

+

q: {filter}

+ + React + + + Vue + + + Clear + +
+ ); +} diff --git a/tests/fixtures/app-basic/app/nav-flash/query-sync/page.tsx b/tests/fixtures/app-basic/app/nav-flash/query-sync/page.tsx new file mode 100644 index 000000000..479dd4660 --- /dev/null +++ b/tests/fixtures/app-basic/app/nav-flash/query-sync/page.tsx @@ -0,0 +1,27 @@ +import { Suspense } from "react"; +import { FilterControls } from "./FilterControls"; + +async function SearchResults({ query }: { query: string }) { + await new Promise((r) => setTimeout(r, 200)); + if (!query) { + return

Enter a search query

; + } + return

Results for: {query}

; +} + +export default async function QuerySyncPage({ + searchParams, +}: { + searchParams: Promise<{ q?: string }>; +}) { + const { q = "" } = await searchParams; + return ( +
+

{q ? `Search: ${q}` : "Search"}

+ + Searching...
}> + + + + ); +} diff --git a/tests/fixtures/app-basic/app/page.tsx b/tests/fixtures/app-basic/app/page.tsx index f0d4978f3..a67457c09 100644 --- a/tests/fixtures/app-basic/app/page.tsx +++ b/tests/fixtures/app-basic/app/page.tsx @@ -15,6 +15,12 @@ export default function HomePage() { Go to Redirect Test + + Nav Flash Test + + + Nav Flash List + ); diff --git a/tests/form.test.ts b/tests/form.test.ts index 034feebe2..e3995c330 100644 --- a/tests/form.test.ts +++ b/tests/form.test.ts @@ -109,12 +109,21 @@ function createWindowStub() { history: { pushState, replaceState, + state: null, }, location: { origin: "http://localhost:3000", href: "http://localhost:3000/current", + pathname: "/current", + search: "", + hash: "", + hostname: "localhost", }, scrollTo, + scrollX: 0, + scrollY: 0, + addEventListener: () => {}, + dispatchEvent: () => {}, }, }; } @@ -240,7 +249,7 @@ describe("Form useActionState", () => { describe("Form client GET interception", () => { it("strips existing query params from the action URL and warns in development", async () => { - const { navigate, pushState, scrollTo } = installClientGlobals({ supportsSubmitter: true }); + const { navigate, scrollTo } = installClientGlobals({ supportsSubmitter: true }); const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); const { onSubmit } = renderClientForm({ action: "/search?lang=en" }); const event = createSubmitEvent({ @@ -253,13 +262,13 @@ describe("Form client GET interception", () => { '
received an `action` that contains search params: "/search?lang=en". This is not supported, and they will be ignored. If you need to pass in additional search params, use an `` instead.', ); expect(event.preventDefault).toHaveBeenCalledOnce(); - expect(pushState).toHaveBeenCalledWith(null, "", "/search?q=react"); - expect(navigate).toHaveBeenCalledWith("/search?q=react"); + // navigateClientSide delegates URL push to __VINEXT_RSC_NAVIGATE__ (two-phase commit) + expect(navigate).toHaveBeenCalledWith("/search?q=react", 0, "navigate", "push"); expect(scrollTo).toHaveBeenCalledWith(0, 0); }); it("honors submitter formAction, formMethod, and submitter name/value", async () => { - const { navigate, pushState } = installClientGlobals({ supportsSubmitter: true }); + const { navigate } = installClientGlobals({ supportsSubmitter: true }); const { onSubmit } = renderClientForm({ action: "/search", method: "POST" }); const submitter = new FakeButtonElement({ attributes: { @@ -280,12 +289,12 @@ describe("Form client GET interception", () => { await onSubmit(event); expect(event.preventDefault).toHaveBeenCalledOnce(); - expect(pushState).toHaveBeenCalledWith( - null, - "", + expect(navigate).toHaveBeenCalledWith( "/search-alt?q=button&lang=fr&source=submitter-action", + 0, + "navigate", + "push", ); - expect(navigate).toHaveBeenCalledWith("/search-alt?q=button&lang=fr&source=submitter-action"); }); it("falls back to appending submitter name/value when FormData submitter overload is unavailable", async () => { @@ -310,11 +319,14 @@ describe("Form client GET interception", () => { expect(navigate).toHaveBeenCalledWith( "/search-alt?q=fallback&lang=de&source=fallback-submitter", + 0, + "navigate", + "push", ); }); it("does not intercept POST submissions without a submitter GET override", async () => { - const { navigate, pushState } = installClientGlobals({ supportsSubmitter: true }); + const { navigate } = installClientGlobals({ supportsSubmitter: true }); const { onSubmit } = renderClientForm({ action: "/search", method: "POST" }); const event = createSubmitEvent({ entries: [["q", "server-action"]], @@ -323,12 +335,11 @@ describe("Form client GET interception", () => { await onSubmit(event); expect(event.preventDefault).not.toHaveBeenCalled(); - expect(pushState).not.toHaveBeenCalled(); expect(navigate).not.toHaveBeenCalled(); }); it("strips submitter formAction query params and warns in development", async () => { - const { navigate, pushState } = installClientGlobals({ supportsSubmitter: true }); + const { navigate } = installClientGlobals({ supportsSubmitter: true }); const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); const { onSubmit } = renderClientForm({ action: "/search" }); const submitter = new FakeButtonElement({ @@ -348,16 +359,16 @@ describe("Form client GET interception", () => { expect(warn).toHaveBeenCalledWith( ' received a `formAction` that contains search params: "/search-alt?lang=fr". This is not supported, and they will be ignored. If you need to pass in additional search params, use an `` instead.', ); - expect(pushState).toHaveBeenCalledWith( - null, - "", + expect(navigate).toHaveBeenCalledWith( "/search-alt?q=button&source=submitter-action", + 0, + "navigate", + "push", ); - expect(navigate).toHaveBeenCalledWith("/search-alt?q=button&source=submitter-action"); }); it("does not intercept submitters with unsupported formTarget overrides", async () => { - const { navigate, pushState } = installClientGlobals({ supportsSubmitter: true }); + const { navigate } = installClientGlobals({ supportsSubmitter: true }); const error = vi.spyOn(console, "error").mockImplementation(() => {}); const { onSubmit } = renderClientForm({ action: "/search" }); const submitter = new FakeButtonElement({ @@ -376,7 +387,6 @@ describe("Form client GET interception", () => { `'s \`target\` was set to an unsupported value via \`formTarget="_blank"\`. This will disable 's navigation functionality. If you need this, use a native element instead.`, ); expect(event.preventDefault).not.toHaveBeenCalled(); - expect(pushState).not.toHaveBeenCalled(); expect(navigate).not.toHaveBeenCalled(); }); }); From bdb689d307274a6a8dcb38ec57c2fbfe9070af48 Mon Sep 17 00:00:00 2001 From: Divanshu Chauhan Date: Wed, 25 Mar 2026 18:28:51 -0700 Subject: [PATCH 2/5] fix: detect React Flight Thenables in updateBrowserTree and apply params eagerly createFromFetch() returns a Thenable (not instanceof Promise), so the async branch was never taken. The raw unresolved Thenable was set as React children, causing suspension without a Suspense boundary and empty content after cross-route navigations. - Duck-type the .then() check to handle both Promises and Thenables - Replace stageClientParams with applyClientParams so useSyncExternalStore subscribers see correct params immediately - Remove dead stageClientParams function and replaceClientParamsWithoutNotify import Fixes useParams E2E failures in CI (hooks.spec.ts:161, hooks.spec.ts:180). --- .../vinext/src/server/app-browser-entry.ts | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/packages/vinext/src/server/app-browser-entry.ts b/packages/vinext/src/server/app-browser-entry.ts index df34d11be..7a90e0955 100644 --- a/packages/vinext/src/server/app-browser-entry.ts +++ b/packages/vinext/src/server/app-browser-entry.ts @@ -26,7 +26,6 @@ import { getPrefetchCache, getPrefetchedUrls, pushHistoryStateWithoutNotify, - replaceClientParamsWithoutNotify, replaceHistoryStateWithoutNotify, restoreRscResponse, setClientParams, @@ -92,11 +91,6 @@ function applyClientParams(params: Record): void { setClientParams(params); } -function stageClientParams(params: Record): void { - latestClientParams = params; - replaceClientParamsWithoutNotify(params); -} - function clearVisitedResponseCache(): void { visitedResponseCache.clear(); } @@ -273,23 +267,25 @@ function updateBrowserTree( setter({ renderId, node: resolvedNode, navigationSnapshot }); }; - if (node instanceof Promise) { + if (node != null && typeof (node as PromiseLike).then === "function") { + const thenable = node as PromiseLike; if (useTransitionMode) { - void node.then((resolved) => { + void thenable.then((resolved) => { startTransition(() => resolvedThenSet(resolved)); }); } else { - void node.then(resolvedThenSet); + void thenable.then(resolvedThenSet); } return; } + const syncNode = node as ReactNode; if (useTransitionMode) { - startTransition(() => resolvedThenSet(node)); + startTransition(() => resolvedThenSet(syncNode)); return; } - resolvedThenSet(node); + resolvedThenSet(syncNode); } function renderNavigationPayload( @@ -510,7 +506,7 @@ async function main(): Promise { const navigationCommitEffect = createNavigationCommitEffect(href, historyUpdateMode); if (cachedRoute) { - stageClientParams(cachedRoute.params); + applyClientParams(cachedRoute.params); const cachedNavigationSnapshot = createClientNavigationRenderSnapshot( href, cachedRoute.params, @@ -569,12 +565,12 @@ async function main(): Promise { string, string | string[] >; - stageClientParams(navParams); + applyClientParams(navParams); } catch { - stageClientParams({}); + applyClientParams({}); } } else { - stageClientParams({}); + applyClientParams({}); } const navigationSnapshot = createClientNavigationRenderSnapshot(href, latestClientParams); From c19abc70c2aad251c3475b003be6da80a427e6d8 Mon Sep 17 00:00:00 2001 From: Divanshu Chauhan Date: Wed, 25 Mar 2026 18:58:42 -0700 Subject: [PATCH 3/5] fix: await createFromFetch before passing RSC payload to tree The Flight Thenable from createFromFetch was passed unresolved to renderNavigationPayload/updateBrowserTree. The .then() callback fired unreliably across consecutive navigations, causing empty content. Await createFromFetch in both navigation paths (cached and fresh) so renderNavigationPayload always receives a resolved ReactNode. Add stale navigation checks after the new await points. The HMR handler and server action handler already awaited correctly. --- packages/vinext/src/server/app-browser-entry.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/vinext/src/server/app-browser-entry.ts b/packages/vinext/src/server/app-browser-entry.ts index 7a90e0955..7ad11b29f 100644 --- a/packages/vinext/src/server/app-browser-entry.ts +++ b/packages/vinext/src/server/app-browser-entry.ts @@ -511,9 +511,10 @@ async function main(): Promise { href, cachedRoute.params, ); - const cachedPayload = createFromFetch( + const cachedPayload = await createFromFetch( Promise.resolve(restoreRscResponse(cachedRoute.response)), ); + if (navId !== activeNavigationId) return; await renderNavigationPayload( cachedPayload, cachedNavigationSnapshot, @@ -579,10 +580,12 @@ async function main(): Promise { if (navId !== activeNavigationId) return; storeVisitedResponseSnapshot(rscUrl, responseSnapshot, navParams); - const rscPayload = createFromFetch( + const rscPayload = await createFromFetch( Promise.resolve(restoreRscResponse(responseSnapshot)), ); + if (navId !== activeNavigationId) return; + await renderNavigationPayload( rscPayload, navigationSnapshot, From e6978c030336a6b79c23c0c0e346f3c6a5cd1270 Mon Sep 17 00:00:00 2001 From: Divanshu Chauhan Date: Wed, 25 Mar 2026 19:44:29 -0700 Subject: [PATCH 4/5] fix: preserve app router params across cached navigation Preserve the X-Vinext-Params header when snapshotting prefetched and visited RSC responses so cached navigations keep dynamic params intact. Stage client params until the URL commit lands so useParams, history state, and redirect handling stay aligned during App Router transitions. --- .../vinext/src/server/app-browser-entry.ts | 23 +++++++++++++------ packages/vinext/src/shims/navigation.ts | 14 ++++++++++- tests/prefetch-cache.test.ts | 20 ++++++++++++++++ 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/packages/vinext/src/server/app-browser-entry.ts b/packages/vinext/src/server/app-browser-entry.ts index 7ad11b29f..e471d177b 100644 --- a/packages/vinext/src/server/app-browser-entry.ts +++ b/packages/vinext/src/server/app-browser-entry.ts @@ -26,6 +26,7 @@ import { getPrefetchCache, getPrefetchedUrls, pushHistoryStateWithoutNotify, + replaceClientParamsWithoutNotify, replaceHistoryStateWithoutNotify, restoreRscResponse, setClientParams, @@ -91,6 +92,11 @@ function applyClientParams(params: Record): void { setClientParams(params); } +function stageClientParams(params: Record): void { + latestClientParams = params; + replaceClientParamsWithoutNotify(params); +} + function clearVisitedResponseCache(): void { visitedResponseCache.clear(); } @@ -138,9 +144,11 @@ function createNavigationCommitEffect( historyUpdateMode: HistoryUpdateMode | undefined, ): () => void { return () => { - if (historyUpdateMode === "replace") { + const targetHref = new URL(href, window.location.origin).href; + + if (historyUpdateMode === "replace" && window.location.href !== targetHref) { replaceHistoryStateWithoutNotify(null, "", href); - } else if (historyUpdateMode === "push") { + } else if (historyUpdateMode === "push" && window.location.href !== targetHref) { pushHistoryStateWithoutNotify(null, "", href); } @@ -506,7 +514,7 @@ async function main(): Promise { const navigationCommitEffect = createNavigationCommitEffect(href, historyUpdateMode); if (cachedRoute) { - applyClientParams(cachedRoute.params); + stageClientParams(cachedRoute.params); const cachedNavigationSnapshot = createClientNavigationRenderSnapshot( href, cachedRoute.params, @@ -548,6 +556,7 @@ async function main(): Promise { if (finalUrl.pathname !== requestedUrl.pathname) { const destinationPath = finalUrl.pathname.replace(/\.rsc$/, "") + finalUrl.search; + replaceHistoryStateWithoutNotify(null, "", destinationPath); const navigate = window.__VINEXT_RSC_NAVIGATE__; if (!navigate) { @@ -555,7 +564,7 @@ async function main(): Promise { return; } - return navigate(destinationPath, redirectDepth + 1, navigationKind, historyUpdateMode); + return navigate(destinationPath, redirectDepth + 1, navigationKind); } let navParams: Record = {}; @@ -566,12 +575,12 @@ async function main(): Promise { string, string | string[] >; - applyClientParams(navParams); + stageClientParams(navParams); } catch { - applyClientParams({}); + stageClientParams({}); } } else { - applyClientParams({}); + stageClientParams({}); } const navigationSnapshot = createClientNavigationRenderSnapshot(href, latestClientParams); diff --git a/packages/vinext/src/shims/navigation.ts b/packages/vinext/src/shims/navigation.ts index 5002b9790..5fb5e4931 100644 --- a/packages/vinext/src/shims/navigation.ts +++ b/packages/vinext/src/shims/navigation.ts @@ -193,6 +193,7 @@ export const PREFETCH_CACHE_TTL = 30_000; export interface CachedRscResponse { buffer: ArrayBuffer; contentType: string; + paramsHeader: string | null; url: string; } @@ -281,6 +282,7 @@ export async function snapshotRscResponse(response: Response): Promise { // Set window BEFORE importing so isServer evaluates to false @@ -35,6 +37,8 @@ beforeEach(async () => { getPrefetchedUrls = nav.getPrefetchedUrls; MAX_PREFETCH_CACHE_SIZE = nav.MAX_PREFETCH_CACHE_SIZE; PREFETCH_CACHE_TTL = nav.PREFETCH_CACHE_TTL; + snapshotRscResponse = nav.snapshotRscResponse; + restoreRscResponse = nav.restoreRscResponse; }); afterEach(() => { @@ -54,6 +58,22 @@ function fillCache(count: number, timestamp: number, keyPrefix = "/page-"): void } describe("prefetch cache eviction", () => { + it("preserves X-Vinext-Params when replaying cached RSC responses", async () => { + const response = new Response("flight", { + headers: { + "content-type": "text/x-component; charset=utf-8", + "x-vinext-params": encodeURIComponent('{"id":"2"}'), + }, + }); + + const snapshot = await snapshotRscResponse(response); + const restored = restoreRscResponse(snapshot); + + expect(restored.headers.get("content-type")).toBe("text/x-component; charset=utf-8"); + expect(restored.headers.get("x-vinext-params")).toBe(encodeURIComponent('{"id":"2"}')); + await expect(restored.text()).resolves.toBe("flight"); + }); + it("sweeps all expired entries before FIFO", () => { // Use fixed arbitrary values to avoid any dependency on the real wall clock const now = 1_000_000; From f062859b715dbd54bf28ea6f7cd20851bb54e924 Mon Sep 17 00:00:00 2001 From: Divanshu Chauhan Date: Thu, 26 Mar 2026 20:13:32 -0700 Subject: [PATCH 5/5] fix: address review feedback on navigation caching and history management - Remove double history push for RSC navigations: let the commit effect in navigateRsc own push/replace exclusively. Also fixes a hidden bug where the early push broke isSameRoute detection (always true), negating the Firefox cross-route hang fix. - Enforce prefetch cache size limits in prefetchRscResponse() by extracting shared eviction logic from storePrefetchResponse(). - Add TTL check in consumePrefetchResponse() so expired snapshots are discarded instead of served as stale UI. - Prevent navigationSnapshotActiveCount leak when async payloads reject in updateBrowserTree by adding error handlers that balance the activate/commit pairing. - Move BrowserRoot setBrowserTreeState assignment from render to useLayoutEffect to avoid side effects under React Strict Mode. - Make visited response cache true LRU (delete+set on hit) and add a 30-minute upper bound for traversal cache hits to prevent arbitrarily stale back/forward content. --- .../vinext/src/server/app-browser-entry.ts | 41 ++++++++++++-- packages/vinext/src/shims/navigation.ts | 54 +++++++++++-------- 2 files changed, 69 insertions(+), 26 deletions(-) diff --git a/packages/vinext/src/server/app-browser-entry.ts b/packages/vinext/src/server/app-browser-entry.ts index e471d177b..fe9128d64 100644 --- a/packages/vinext/src/server/app-browser-entry.ts +++ b/packages/vinext/src/server/app-browser-entry.ts @@ -67,6 +67,7 @@ interface VisitedResponseCacheEntry { const MAX_VISITED_RESPONSE_CACHE_SIZE = 50; const VISITED_RESPONSE_CACHE_TTL = 5 * 60_000; +const MAX_TRAVERSAL_CACHE_TTL = 30 * 60_000; let nextNavigationRenderId = 0; let activeNavigationId = 0; @@ -180,10 +181,21 @@ function getVisitedResponse( } if (navigationKind === "traverse") { + const createdAt = cached.expiresAt - VISITED_RESPONSE_CACHE_TTL; + if (Date.now() - createdAt >= MAX_TRAVERSAL_CACHE_TTL) { + visitedResponseCache.delete(rscUrl); + return null; + } + // LRU: promote to most-recently-used (delete + re-insert moves to end of Map) + visitedResponseCache.delete(rscUrl); + visitedResponseCache.set(rscUrl, cached); return cached; } if (cached.expiresAt > Date.now()) { + // LRU: promote to most-recently-used + visitedResponseCache.delete(rscUrl); + visitedResponseCache.set(rscUrl, cached); return cached; } @@ -244,7 +256,15 @@ function BrowserRoot({ navigationSnapshot: initialNavigationSnapshot, }); - setBrowserTreeState = setTreeState; + // Assign the module-level setter via useLayoutEffect instead of during render + // to avoid side effects that React Strict Mode / concurrent features may + // call multiple times. useLayoutEffect fires synchronously during commit, + // before hydrateRoot returns to main(), so setBrowserTreeState is available + // before __VINEXT_RSC_NAVIGATE__ is assigned. setTreeState is referentially + // stable so the effect only runs on mount. + useLayoutEffect(() => { + setBrowserTreeState = setTreeState; + }, []); // eslint-disable-line react-hooks/exhaustive-deps -- setTreeState is referentially stable const committedTree = createElement(NavigationCommitSignal, { renderId: treeState.renderId, @@ -275,14 +295,25 @@ function updateBrowserTree( setter({ renderId, node: resolvedNode, navigationSnapshot }); }; + // Balance the activate/commit pairing if the async payload rejects + // after activateNavigationSnapshot() was called in renderNavigationPayload. + const handleAsyncError = () => { + pendingNavigationPrePaintEffects.delete(renderId); + const resolve = pendingNavigationCommits.get(renderId); + pendingNavigationCommits.delete(renderId); + commitClientNavigationState(); + resolve?.(); + }; + if (node != null && typeof (node as PromiseLike).then === "function") { const thenable = node as PromiseLike; if (useTransitionMode) { - void thenable.then((resolved) => { - startTransition(() => resolvedThenSet(resolved)); - }); + void thenable.then( + (resolved) => startTransition(() => resolvedThenSet(resolved)), + handleAsyncError, + ); } else { - void thenable.then(resolvedThenSet); + void thenable.then(resolvedThenSet, handleAsyncError); } return; } diff --git a/packages/vinext/src/shims/navigation.ts b/packages/vinext/src/shims/navigation.ts index 5fb5e4931..d1ecb0581 100644 --- a/packages/vinext/src/shims/navigation.ts +++ b/packages/vinext/src/shims/navigation.ts @@ -242,35 +242,41 @@ export function getPrefetchedUrls(): Set { } /** - * Store a prefetched RSC response in the cache. - * Enforces a maximum cache size to prevent unbounded memory growth on - * link-heavy pages. + * Evict prefetch cache entries if at capacity. + * First sweeps expired entries, then falls back to FIFO eviction. + * Shared by storePrefetchResponse() and prefetchRscResponse(). */ -export function storePrefetchResponse(rscUrl: string, response: Response): void { +function evictPrefetchCacheIfNeeded(): void { const cache = getPrefetchCache(); + if (cache.size < MAX_PREFETCH_CACHE_SIZE) return; + const now = Date.now(); + const prefetched = getPrefetchedUrls(); - // Sweep expired entries before resorting to FIFO eviction - if (cache.size >= MAX_PREFETCH_CACHE_SIZE) { - const prefetched = getPrefetchedUrls(); - for (const [key, entry] of cache) { - if (now - entry.timestamp >= PREFETCH_CACHE_TTL) { - cache.delete(key); - prefetched.delete(key); - } + for (const [key, entry] of cache) { + if (now - entry.timestamp >= PREFETCH_CACHE_TTL) { + cache.delete(key); + prefetched.delete(key); } } - // FIFO fallback if still at capacity after sweep if (cache.size >= MAX_PREFETCH_CACHE_SIZE) { const oldest = cache.keys().next().value; if (oldest !== undefined) { cache.delete(oldest); - getPrefetchedUrls().delete(oldest); + prefetched.delete(oldest); } } +} - cache.set(rscUrl, { response, timestamp: now }); +/** + * Store a prefetched RSC response in the cache. + * Enforces a maximum cache size to prevent unbounded memory growth on + * link-heavy pages. + */ +export function storePrefetchResponse(rscUrl: string, response: Response): void { + evictPrefetchCacheIfNeeded(); + getPrefetchCache().set(rscUrl, { response, timestamp: Date.now() }); } /** @@ -333,6 +339,7 @@ export function prefetchRscResponse(rscUrl: string, fetchPromise: Promise= PREFETCH_CACHE_TTL) { + return null; + } + return entry.snapshot; + } // Legacy: raw Response entries (from storePrefetchResponse) // These can't be consumed synchronously as snapshots — skip them. @@ -903,12 +915,12 @@ export async function navigateClientSide( // Trigger RSC re-fetch if available, and wait for the new content to render // before scrolling. This prevents the old page from visibly jumping to the // top before the new content paints. + // + // History is NOT pushed here for RSC navigations — the commit effect inside + // navigateRsc owns the push/replace exclusively. This avoids a fragile + // double-push and ensures window.location still reflects the *current* URL + // when navigateRsc computes isSameRoute (cross-route vs same-route). if (typeof window.__VINEXT_RSC_NAVIGATE__ === "function") { - if (mode === "replace") { - replaceHistoryStateWithoutNotify(null, "", fullHref); - } else { - pushHistoryStateWithoutNotify(null, "", fullHref); - } await window.__VINEXT_RSC_NAVIGATE__(fullHref, 0, "navigate", mode); } else { if (mode === "replace") {