diff --git a/packages/vinext/src/shims/internal/app-prefetch-setup.ts b/packages/vinext/src/shims/internal/app-prefetch-setup.ts new file mode 100644 index 0000000000..4de04e60e8 --- /dev/null +++ b/packages/vinext/src/shims/internal/app-prefetch-setup.ts @@ -0,0 +1,78 @@ +/** + * Registry of prefetch calls whose asynchronous setup is still running. Both + * `router.prefetch()` (navigation.ts) and ``'s `prefetchUrl` (link.tsx) + * register a token before their first `await` and re-check `cancelled` after, + * so superseded setup cannot register a cache entry or start a request. + * + * Cancellation is sticky and scoped to the destination: + * - a navigation to the same href (`notifyAppNavigationStart` in + * navigation.ts) will fetch that route itself, so a late prefetch would + * duplicate the request. Navigations elsewhere leave the prefetch alone — + * nothing else is going to fetch it, and dropping it would make the + * prefetch timing-dependent. + * - invalidating the whole cache (`invalidatePrefetchCache`, reached via + * `router.refresh()`) cancels every pending setup, which would otherwise + * repopulate one route from the pre-refresh generation. + * + * Sticky matters: a navigation to `/a` followed by one to `/b` must leave a + * pending `/a` prefetch cancelled, which comparing against a "current + * destination" value would not. + * + * This lives in shims/internal/ rather than navigation.ts because link.tsx + * must register synchronously but only loads navigation.ts lazily — a static + * import of the registry must not pull the navigation runtime onto Link's + * startup path. Keep this module free of React and route-trie dependencies. + */ +import { isExternalUrl } from "../../utils/external-url.js"; +import { toBrowserNavigationHref, toSameOriginAppPath } from "../url-utils.js"; + +/** basePath from next.config.js, injected by the plugin at build time */ +const __basePath: string = process.env.__NEXT_ROUTER_BASEPATH ?? ""; + +const isServer = typeof window === "undefined"; + +export type PendingPrefetchSetup = { readonly destination: string; cancelled: boolean }; +const pendingPrefetchSetups = new Set(); + +export function beginPrefetchSetup(destination: string): PendingPrefetchSetup { + const setup: PendingPrefetchSetup = { destination, cancelled: false }; + pendingPrefetchSetups.add(setup); + return setup; +} + +/** Passing `null` cancels every pending setup regardless of destination. */ +export function cancelPendingPrefetchSetups(destination: string | null): void { + for (const setup of pendingPrefetchSetups) { + if (destination === null || setup.destination === destination) { + setup.cancelled = true; + } + } +} + +/** Unregister a token once its setup closure has settled, cancelled or not. */ +export function finishPrefetchSetup(setup: PendingPrefetchSetup): void { + pendingPrefetchSetups.delete(setup); +} + +/** + * Normalize a navigation or prefetch target to the browser href both sides + * compare on. Returns null when the target is not same-origin — no same-origin + * prefetch can be a duplicate of it — and on the server, where + * `navigateClientSide` can still be reached but there is nothing to cancel. + */ +export function toAppPrefetchDestination(href: string): string | null { + if (isServer) return null; + let localHref = href; + if (isExternalUrl(href)) { + const localPath = toSameOriginAppPath(href, __basePath); + if (localPath == null) return null; + localHref = localPath; + } + const browserHref = toBrowserNavigationHref(localHref, window.location.href, __basePath); + try { + const url = new URL(browserHref, window.location.href); + return `${url.pathname}${url.search}`; + } catch { + return browserHref.split("#", 1)[0]; + } +} diff --git a/packages/vinext/src/shims/link.tsx b/packages/vinext/src/shims/link.tsx index de37c5b349..acbfe0b93b 100644 --- a/packages/vinext/src/shims/link.tsx +++ b/packages/vinext/src/shims/link.tsx @@ -55,6 +55,12 @@ import { } from "./internal/pages-data-target.js"; import { interpolateDynamicRouteHref, resolveDynamicRouteHref } from "./internal/interpolate-as.js"; import { markAppRouteDetectedOnPrefetch } from "./internal/app-route-detection.js"; +import { + beginPrefetchSetup, + finishPrefetchSetup, + toAppPrefetchDestination, + type PendingPrefetchSetup, +} from "./internal/app-prefetch-setup.js"; import { canAutoPrefetchFullAppRoute, resolveAutoAppRoutePrefetch, @@ -175,23 +181,19 @@ export function useLinkStatus(): LinkStatusContextValue { return useContext(LinkStatusContext); } -let linkPrefetchNavigationEpoch = 0; - -function notifyLinkNavigationStartAndCancelPrefetchSetup(): void { - linkPrefetchNavigationEpoch += 1; - notifyLinkNavigationStart(); -} - // Register the link-status reset hook on the navigation runtime as soon as this // module evaluates on the client. `navigateClientSide` calls it at the start of // every App Router navigation (including router.push and shallow routing), so a // stale link's pending state is cleared even when no initiated the // navigation. The registry itself lives in internal/link-status-registry.ts so // it can be unit-tested without rendering a . +// +// Prefetch-setup cancellation does NOT ride this hook: `prefetchUrl` registers +// in the shared registry (internal/app-prefetch-setup.ts), which navigation.ts +// cancels by destination on navigation start and wholesale on cache +// invalidation — the same policy `router.prefetch()` gets. if (typeof window !== "undefined") { - registerNavigationRuntimeFunctions({ - notifyLinkNavigationStart: notifyLinkNavigationStartAndCancelPrefetchSetup, - }); + registerNavigationRuntimeFunctions({ notifyLinkNavigationStart }); } /** basePath from next.config.js, injected by the plugin at build time */ @@ -366,7 +368,6 @@ function prefetchUrl( locale?: string | false, ): void { if (typeof window === "undefined") return; - const navigationEpoch = linkPrefetchNavigationEpoch; const prefetchHref = getLinkPrefetchHref({ href, @@ -412,10 +413,28 @@ function prefetchUrl( } const runPrefetch = () => { + let setup: PendingPrefetchSetup | null = null; + // Setup work that keeps running after the outer closure returns (the + // fire-and-forget loading shell, the staged shell-first payload). The + // token must stay registered until these can no longer write to the + // prefetch cache, or a cancellation during their awaits finds no token + // and their guards never trip. + const stagedSetupWork: Promise[] = []; void (async () => { if (hasAppNavigationRuntime()) { if (isBotUserAgent(window.navigator?.userAgent ?? "")) return; + // Registered before the first await, so a navigation to this + // destination or a cache invalidation in the same task cancels the + // setup below instead of racing it. The destination is derived from + // the app-relative `prefetchHref`, matching what navigations pass to + // `toAppPrefetchDestination` — `fullHref` already carries basePath and + // would get it prefixed a second time, producing a key no navigation + // ever matches. `prefetchHref` is same-origin local (getLinkPrefetchHref + // returned non-null) and this runs on the client, so the destination + // always resolves; the fallback is unreachable. + const activeSetup = beginPrefetchSetup(toAppPrefetchDestination(prefetchHref) ?? fullHref); + setup = activeSetup; const [ navigation, { AppElementsWire }, @@ -437,7 +456,10 @@ function prefetchUrl( // A pointer-intent prefetch and its click navigation can start in the // same event turn. If navigation won the module-loading race, do not // begin a second request after it consumes an equivalent cached route. - if (navigationEpoch !== linkPrefetchNavigationEpoch) return; + // Cancellation also arrives from invalidatePrefetchCache(): a + // router.refresh() while these modules load voids the whole cache + // generation, and resuming would repopulate one route from it. + if (activeSetup.cancelled) return; const { getPrefetchInterceptionContext, getPrefetchCache, @@ -526,6 +548,10 @@ function prefetchUrl( rewrittenPrefetchHref && rewrittenPrefetchHref !== fullHref ? [await createRscRequestUrl(rewrittenPrefetchHref, headers)] : []; + // Re-checked after the RSC-URL awaits above: a cancellation that + // arrived since the module-loading check must not reach the cache + // writes below. Mirrors router.prefetch()'s guard in navigation.ts. + if (activeSetup.cancelled) return; const cacheKey = AppElementsWire.encodeCacheKey(rscUrl, interceptionContext); const prefetched = getPrefetchedUrls(); if (autoPrefetch.cacheForNavigation) { @@ -561,6 +587,11 @@ function prefetchUrl( shellHeaders.set(VINEXT_MOUNTED_SLOTS_HEADER, mountedSlotsHeader); } const shellRscUrl = await createRscRequestUrl(fullHref, shellHeaders); + // This helper outlives the outer closure (it is launched without + // being awaited), so a cancellation that lands during the await + // above must stop it here before it registers a shell entry in an + // invalidated cache generation. + if (activeSetup.cancelled) return; const shellCacheKey = AppElementsWire.encodeCacheKey(shellRscUrl, interceptionContext); const shellCache = getPrefetchCache(); let shellEntry = shellCache.get(shellCacheKey); @@ -681,7 +712,11 @@ function prefetchUrl( ); const shellCache = getPrefetchCache(); let shellEntry = shellCache.get(shellCacheKey); - if (shellEntry === undefined) { + // Guard only the side write: this closure's return value feeds + // the navigation entry registered before any cancellation, so + // it must still resolve; only a new route-tree entry must not + // be added to an invalidated cache generation. + if (shellEntry === undefined && !activeSetup.cancelled) { getPrefetchedUrls().add(shellCacheKey); prefetchRscResponse( shellRscUrl, @@ -735,6 +770,11 @@ function prefetchUrl( ); })() : fetchFullRscPayload(); + if (autoPrefetch.cacheForNavigation && (gateViaRouteTree || gateViaLoadingShell)) { + // The staged closure writes shell/route-tree entries between its + // awaits; hold the token until it settles. + stagedSetupWork.push(fetchPromise.catch(() => {})); + } if ( !__prefetchInlining && mode === "full" && @@ -743,7 +783,7 @@ function prefetchUrl( mountedSlotsHeader === null && !gateViaExplicitSearchShell ) { - void fetchLoadingShellForReuse(); + stagedSetupWork.push(fetchLoadingShellForReuse().catch(() => {})); } prefetchRscResponse( rscUrl, @@ -803,9 +843,17 @@ function prefetchUrl( document.head.appendChild(link); } } - })().catch((error) => { - console.error("[vinext] RSC prefetch setup error:", error); - }); + })() + .catch((error) => { + console.error("[vinext] RSC prefetch setup error:", error); + }) + .finally(() => { + if (setup === null) return; + const registered = setup; + // Not until the outer closure ends: staged helpers keep writing after + // it, and an unregistered token can no longer be cancelled. + void Promise.allSettled(stagedSetupWork).then(() => finishPrefetchSetup(registered)); + }); }; if (priority === "high" || hasAppNavigationRuntime()) { diff --git a/packages/vinext/src/shims/navigation.ts b/packages/vinext/src/shims/navigation.ts index 448335f24e..a3a4fbfe29 100644 --- a/packages/vinext/src/shims/navigation.ts +++ b/packages/vinext/src/shims/navigation.ts @@ -65,6 +65,12 @@ import { assertSafeNavigationUrl } from "./url-safety.js"; import { markPprFallbackShellDynamicBoundary } from "./ppr-fallback-shell.js"; import type { AppRscRenderMode } from "../server/app-rsc-render-mode.js"; import { AppRouterContext, type AppRouterInstance } from "./internal/app-router-context.js"; +import { + beginPrefetchSetup, + cancelPendingPrefetchSetups, + finishPrefetchSetup, + toAppPrefetchDestination, +} from "./internal/app-prefetch-setup.js"; import { getPagesNavigationContext as _getPagesNavigationContext } from "./internal/pages-router-accessor.js"; import { resolveDirectHybridClientRouteOwner, @@ -768,68 +774,6 @@ function evictPrefetchCacheIfNeeded(): void { } } -/** - * `router.prefetch()` calls whose asynchronous setup is still running. Each one - * registers a token before its first `await` and re-checks `cancelled` after, - * so superseded setup cannot register a cache entry or start a request. - * - * Cancellation is sticky and scoped to the destination: - * - a navigation to the same href (`notifyAppNavigationStart`) will fetch that - * route itself, so a late prefetch would duplicate the request. Navigations - * elsewhere leave the prefetch alone — nothing else is going to fetch it, - * and dropping it would make an explicit prefetch timing-dependent. - * - invalidating the whole cache (`invalidatePrefetchCache`, reached via - * `router.refresh()`) cancels every pending setup, which would otherwise - * repopulate one route from the pre-refresh generation. - * - * Sticky matters: a navigation to `/a` followed by one to `/b` must leave a - * pending `/a` prefetch cancelled, which comparing against a "current - * destination" value would not. - * - * `linkPrefetchNavigationEpoch` in link.tsx still uses a global counter for the - * navigation case; unifying the two is tracked separately. - */ -type PendingPrefetchSetup = { readonly destination: string; cancelled: boolean }; -const pendingPrefetchSetups = new Set(); - -function beginPrefetchSetup(destination: string): PendingPrefetchSetup { - const setup: PendingPrefetchSetup = { destination, cancelled: false }; - pendingPrefetchSetups.add(setup); - return setup; -} - -/** Passing `null` cancels every pending setup regardless of destination. */ -function cancelPendingPrefetchSetups(destination: string | null): void { - for (const setup of pendingPrefetchSetups) { - if (destination === null || setup.destination === destination) { - setup.cancelled = true; - } - } -} - -/** - * Normalize a navigation or prefetch target to the browser href both sides - * compare on. Returns null when the target is not same-origin — no same-origin - * prefetch can be a duplicate of it — and on the server, where - * `navigateClientSide` can still be reached but there is nothing to cancel. - */ -function toAppPrefetchDestination(href: string): string | null { - if (isServer) return null; - let localHref = href; - if (isExternalUrl(href)) { - const localPath = toSameOriginAppPath(href, __basePath); - if (localPath == null) return null; - localHref = localPath; - } - const browserHref = toBrowserNavigationHref(localHref, window.location.href, __basePath); - try { - const url = new URL(browserHref, window.location.href); - return `${url.pathname}${url.search}`; - } catch { - return browserHref.split("#", 1)[0]; - } -} - function clearPrefetchInvalidation(entry: PrefetchCacheEntry): void { if (entry.invalidationTimer !== undefined) { clearTimeout(entry.invalidationTimer); @@ -2763,7 +2707,7 @@ const _appRouter: AppRouterInstance = { console.error("[vinext] RSC prefetch setup error:", error); }) .finally(() => { - pendingPrefetchSetups.delete(setup); + finishPrefetchSetup(setup); }); }, }; diff --git a/tests/link-navigation.test.ts b/tests/link-navigation.test.ts index 428fc71696..314302fe4a 100644 --- a/tests/link-navigation.test.ts +++ b/tests/link-navigation.test.ts @@ -1768,6 +1768,170 @@ describe("Link prefetch scheduling", () => { } }); + it("does not resume a Link prefetch across a cache invalidation (#2718)", async () => { + // A hover intent rather than a viewport entry: invalidation re-pings + // *visible* links so they legitimately re-prefetch the fresh generation, + // which would mask the stale resume this test pins down. A hovered link + // gets no re-ping, so any fetch below can only come from the cancelled + // setup resuming. + const result = await renderIsolatedLink({ + href: "/intent-prefetch-target", + nodeEnv: "production", + }); + const { getPrefetchCache, invalidatePrefetchCache } = + await import("../packages/vinext/src/shims/navigation.js"); + + try { + // The intent prefetch starts synchronously, so its setup is in flight + // (registered, awaiting its module imports) when router.refresh() + // reaches invalidatePrefetchCache(). + result.capturedAnchorProps.onMouseEnter?.({ currentTarget: result.anchor }); + invalidatePrefetchCache(); + await flushPrefetchTasks(); + + // A resumed setup would repopulate a navigation-reusable entry built + // from the pre-refresh cache generation. + expect(result.fetch).not.toHaveBeenCalled(); + expect(getPrefetchCache().size).toBe(0); + } finally { + result.restoreNodeEnv(); + } + }); + + // A navigation cancels Link prefetch setup only for the route it is about + // to fetch itself. These two cases differ only in where the navigation + // goes, so together they pin the scoping: a global "any navigation cancels + // everything" rule passes the first and fails the second. + it("cancels a pending Link prefetch superseded by a navigation to the same route (#2718)", async () => { + const observer = stubIntersectionObserver(); + + const result = await renderIsolatedLink({ + href: "/viewport-prefetch-target", + nodeEnv: "production", + }); + const { navigateClientSide } = await import("../packages/vinext/src/shims/navigation.js"); + + try { + // Yield one microtask so the queued viewport prefetch has started its + // setup before the navigation begins. + observer.dispatchIntersectingEntry(result.anchor); + await Promise.resolve(); + // The navigation fetches this route itself; a late prefetch would make + // it two requests for one route. Only the synchronous navigation-start + // notification matters here — the harness runtime stubs the actual + // navigation, so any fetch below can only come from the prefetch. + void navigateClientSide("/viewport-prefetch-target", "push", false).catch(() => {}); + await flushPrefetchTasks(); + + expect(result.fetch).not.toHaveBeenCalled(); + } finally { + result.restoreNodeEnv(); + } + }); + + it("keeps a pending Link prefetch when the navigation goes elsewhere (#2718)", async () => { + const observer = stubIntersectionObserver(); + + const result = await renderIsolatedLink({ + href: "/viewport-prefetch-target", + nodeEnv: "production", + }); + const { navigateClientSide } = await import("../packages/vinext/src/shims/navigation.js"); + + try { + observer.dispatchIntersectingEntry(result.anchor); + await Promise.resolve(); + // Nothing else is going to fetch the link's route, so dropping its + // pending prefetch here would make viewport prefetching depend on + // unrelated navigation timing. + void navigateClientSide("/intent-prefetch-target", "push", false).catch(() => {}); + await waitForFetchCalls(result.fetch, 1); + + expect(result.fetch).toHaveBeenCalledTimes(1); + expectCanonicalRscFetchCall( + result.fetch.mock.calls[0], + "/viewport-prefetch-target", + expect.objectContaining({ + credentials: "include", + priority: "low", + }), + ); + } finally { + result.restoreNodeEnv(); + } + }); + + it("cancels a pending Link prefetch on same-route navigation under a basePath (#2718)", async () => { + // The cancellation key must come from the app-relative href on both sides. + // Deriving it from the basePath-prefixed browser href double-prefixes it + // (/docs + /docs/intent-prefetch-target), so the navigation's key never + // matches and the same-route prefetch survives as a duplicate request. + vi.stubEnv("__NEXT_ROUTER_BASEPATH", "/docs"); + const result = await renderIsolatedLink({ + href: "/intent-prefetch-target", + nodeEnv: "production", + windowOverrides: { + location: { + href: "https://example.com/docs/current", + origin: "https://example.com", + pathname: "/docs/current", + search: "", + }, + }, + }); + const { navigateClientSide } = await import("../packages/vinext/src/shims/navigation.js"); + + try { + result.capturedAnchorProps.onMouseEnter?.({ currentTarget: result.anchor }); + void navigateClientSide("/intent-prefetch-target", "push", false).catch(() => {}); + await flushPrefetchTasks(); + + expect(result.fetch).not.toHaveBeenCalled(); + } finally { + result.restoreNodeEnv(); + } + }); + + it("does not register a staged loading shell after a cache invalidation (#2718)", async () => { + // An explicit full prefetch launches its loading-shell warmup without + // awaiting it, and the outer setup finishes while that helper is still + // awaiting createRscRequestUrl(). Hover intent rather than a viewport + // entry, for the same reason as the invalidation test above: a visible + // link would legitimately re-prefetch after the invalidation and mask the + // staged leak. + const result = await renderIsolatedLink({ + href: "/blog/hello", + nodeEnv: "production", + props: { prefetch: true }, + }); + const { getPrefetchCache, invalidatePrefetchCache } = + await import("../packages/vinext/src/shims/navigation.js"); + + try { + // Timing, driven entirely from the stubbed fetch boundary: a + // high-priority intent prefetch issues its full-payload fetch + // synchronously inside the setup closure, in the same synchronous + // stretch that launches the shell helper. A microtask scheduled from + // that fetch call therefore runs after the closure returns but before + // the helper's createRscRequestUrl() can resolve — its SHA-256 digest + // completes through the event loop — so the invalidation lands + // deterministically inside the helper's pre-write window. + result.fetch.mockImplementationOnce(() => { + void Promise.resolve().then(() => invalidatePrefetchCache()); + return Promise.resolve(new Response("")); + }); + result.capturedAnchorProps.onMouseEnter?.({ currentTarget: result.anchor }); + await flushPrefetchTasks(); + + // A resumed helper would fetch the shell and register it in the + // invalidated cache generation. + expect(result.fetch).toHaveBeenCalledTimes(1); + expect(getPrefetchCache().size).toBe(0); + } finally { + result.restoreNodeEnv(); + } + }); + it("re-prefetches a visible Link when the exact cache entry has gone stale", async () => { const observer = stubIntersectionObserver();