From 16e5f9e685188751f1f4fb085f6e9e729f409950 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 25 Apr 2026 12:51:06 -0700 Subject: [PATCH] Suspend dynamic route params in dev instant shell (#93108) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In dev with cacheComponents, the instant-nav prefetch for a dynamic route resolved the slug into the cached shell instead of returning the generic fallback, so `params` never suspended under a locked instant scope. Existing e2e coverage missed this because the fixture's dynamic-params page had `generateStaticParams` defined. That populates the dev prerender manifest and masks the bug — the broken path is the one where no `generateStaticParams` is defined. The regression test uses a new `ungenerated-params/[slug]` fixture that leaves it out. --- packages/next/src/build/templates/app-page.ts | 54 +++++++++++++------ .../segment-cache/navigation-testing-lock.ts | 53 ++++++++++++++---- .../instant-navs-devtools.test.ts | 26 ++++----- .../fixtures/default/app/page.tsx | 9 ++++ .../[slug]/page.tsx | 41 ++++++++++++++ .../app/ungenerated-params/[slug]/page.tsx | 28 ++++++++++ .../instant-navigation-testing-api.test.ts | 46 ++++++++++++++++ 7 files changed, 218 insertions(+), 39 deletions(-) create mode 100644 test/e2e/app-dir/instant-navigation-testing-api/fixtures/default/app/ungenerated-params-runtime/[slug]/page.tsx create mode 100644 test/e2e/app-dir/instant-navigation-testing-api/fixtures/default/app/ungenerated-params/[slug]/page.tsx diff --git a/packages/next/src/build/templates/app-page.ts b/packages/next/src/build/templates/app-page.ts index 88aa969fed27..a333aa84acc1 100644 --- a/packages/next/src/build/templates/app-page.ts +++ b/packages/next/src/build/templates/app-page.ts @@ -1105,22 +1105,44 @@ export async function handler( ? prerenderInfo.fallback : normalizedSrcPage - const fallbackRouteParams = - // In production or when debugging the static shell (e.g. instant - // navigation testing), use the prerender manifest's fallback - // route params which correctly identifies which params are - // unknown. Note: in dev, this block is only entered for - // non-prerendered URLs (guarded by the outer condition). - (isProduction || isDebugStaticShell) && - prerenderInfo?.fallbackRouteParams - ? createOpaqueFallbackRouteParams( - prerenderInfo.fallbackRouteParams - ) - : // When debugging the fallback shell, treat all params as - // fallback (simulating the worst-case shell). - isDebugFallbackShell - ? getFallbackRouteParams(normalizedSrcPage, routeModule) - : null + let fallbackRouteParams: OpaqueFallbackRouteParams | null + if (isProduction) { + // In production, rely on the prerender manifest's fallback + // entry — the authoritative set computed at build time by + // `buildAppStaticPaths`. + if (prerenderInfo?.fallbackRouteParams) { + fallbackRouteParams = createOpaqueFallbackRouteParams( + prerenderInfo.fallbackRouteParams + ) + } else if (isDebugFallbackShell) { + fallbackRouteParams = getFallbackRouteParams( + normalizedSrcPage, + routeModule + ) + } else { + fallbackRouteParams = null + } + } else { + // In dev, the prerender manifest isn't populated for ad-hoc + // prefetches. The outer `!isPrerendered` guard means every URL + // reaching this block has params not covered by + // `generateStaticParams`, so the worst-case fallback set — + // every dynamic segment from the loader tree — matches what a + // static prerender would use. This keeps the prefetch response + // from baking resolved param values into the shell. + // + // `isDebugStaticShell` covers the `?__nextppronly=1` query and + // the Instant Navigation testing cookie; `isDebugFallbackShell` + // is the explicit fallback-shell debug flow. + if (isDebugStaticShell || isDebugFallbackShell) { + fallbackRouteParams = getFallbackRouteParams( + normalizedSrcPage, + routeModule + ) + } else { + fallbackRouteParams = null + } + } // When rendering a debug static shell, override the fallback // params on the request so that the staged rendering correctly diff --git a/packages/next/src/client/components/segment-cache/navigation-testing-lock.ts b/packages/next/src/client/components/segment-cache/navigation-testing-lock.ts index 8ddd52744a7f..a39e5566dceb 100644 --- a/packages/next/src/client/components/segment-cache/navigation-testing-lock.ts +++ b/packages/next/src/client/components/segment-cache/navigation-testing-lock.ts @@ -15,7 +15,7 @@ import type { FlightRouterState } from '../../../shared/lib/app-router-types' import { NEXT_INSTANT_TEST_COOKIE } from '../app-router-headers' import { refreshOnInstantNavigationUnlock } from '../use-action-queue' -type InstantNavCookieState = 'pending' | 'mpa' | 'spa' +type InstantNavCookieState = 'empty' | 'pending' | 'mpa' | 'spa' type InstantCookie = // pending (waiting to capture) @@ -30,6 +30,9 @@ type InstantCookie = ] function parseCookieValue(raw: string): InstantNavCookieState { + if (raw === '') { + return 'empty' + } try { const parsed = JSON.parse(raw) if (Array.isArray(parsed) && parsed.length >= 3) { @@ -187,16 +190,14 @@ export function startListeningForInstantNavigationCookie(): void { if (cookie.name === NEXT_INSTANT_TEST_COOKIE) { const state = parseCookieValue(cookie.value ?? '') - if (state !== 'pending') { - // Captured value — our own transition. Ignore. - return - } - - // Pending value — external actor starting a new lock scope. - if (lockState !== null) { - releaseLock() + if (state === 'pending') { + // External actor starting a new lock scope. + if (lockState !== null) { + releaseLock() + } + acquireLock() } - acquireLock() + // Captured value (our own transition) or empty. Ignore. return } } @@ -246,7 +247,37 @@ export function updateCapturedSPAToTree( */ export function isNavigationLocked(): boolean { if (process.env.__NEXT_EXPOSE_TESTING_API) { - return lockState !== null + if (lockState !== null) { + return true + } + + // If `lockState` is null, fall back to reading the test cookie + // synchronously from `document.cookie`. This accounts for a small race + // between `cookieStore.set(...)` and its corresponding `change` event. + // During that gap `lockState` is still null even though the cookie + // indicates a new lock scope is starting. + if (typeof document === 'undefined') { + return false + } + const allCookies = document.cookie + if (!allCookies.includes(NEXT_INSTANT_TEST_COOKIE)) { + // Fast bail-out: in almost every navigation the test cookie is not + // set at all. + return false + } + const target = NEXT_INSTANT_TEST_COOKIE + '=' + for (const segment of allCookies.split(';')) { + const trimmed = segment.trim() + if ( + trimmed.startsWith(target) && + parseCookieValue(trimmed.slice(target.length)) === 'pending' + ) { + // The cookie was set by an external actor but the change event was not + // yet dispatched. Acquire the lock synchronously. + acquireLock() + return true + } + } } return false } diff --git a/test/development/app-dir/instant-navs-devtools/instant-navs-devtools.test.ts b/test/development/app-dir/instant-navs-devtools/instant-navs-devtools.test.ts index a1cd26e5bb7a..e54de28d213e 100644 --- a/test/development/app-dir/instant-navs-devtools/instant-navs-devtools.test.ts +++ b/test/development/app-dir/instant-navs-devtools/instant-navs-devtools.test.ts @@ -136,7 +136,7 @@ describe('instant-nav-panel', () => { await clearInstantModeCookie(browser) }) - it('should show loading skeleton during SPA navigation after clicking Start', async () => { + it('should show loading skeletons during SPA navigation after clicking Start', async () => { const targetPage = '/target-page/my-post?search=foo' if (isNextDev && !isTurbopack) { // warmup target page compilation before clicking Start, to avoid extra flakiness. @@ -156,18 +156,20 @@ describe('instant-nav-panel', () => { document.querySelector(`[href="${page}"]`)!.click() }, targetPage) - // The data fetching skeleton should be visible (dynamic content is locked). + // Every runtime-dependent segment should be suspended under the lock: + // data-fetching (dynamic content), `await params`, and `await searchParams`. // Use a longer timeout because dev mode needs to compile the target page. - await retry( - async () => { - const skeleton = await browser.hasElementByCss( - '[data-testid="dynamic-skeleton"]' - ) - expect(skeleton).toBe(true) - }, - 30000, - 500 - ) + await browser + .locator('[data-testid="dynamic-skeleton"]') + .waitFor({ state: 'visible', timeout: 30000 }) + await browser + .locator('[data-testid="param-skeleton"]') + .waitFor({ state: 'visible' }) + await browser + .locator('[data-testid="search-param-skeleton"]') + .waitFor({ state: 'visible' }) + // The resolved param value must not have leaked through the lock. + expect(await browser.locator('[data-testid="param-value"]').count()).toBe(0) // Clean up await clearInstantModeCookie(browser) diff --git a/test/e2e/app-dir/instant-navigation-testing-api/fixtures/default/app/page.tsx b/test/e2e/app-dir/instant-navigation-testing-api/fixtures/default/app/page.tsx index 65b758fae366..dbc7e65c46f5 100644 --- a/test/e2e/app-dir/instant-navigation-testing-api/fixtures/default/app/page.tsx +++ b/test/e2e/app-dir/instant-navigation-testing-api/fixtures/default/app/page.tsx @@ -29,6 +29,15 @@ export default function HomePage() { Go to static dynamic params page + + Go to ungenerated params page + + + Go to ungenerated params runtime page + Go to search params page diff --git a/test/e2e/app-dir/instant-navigation-testing-api/fixtures/default/app/ungenerated-params-runtime/[slug]/page.tsx b/test/e2e/app-dir/instant-navigation-testing-api/fixtures/default/app/ungenerated-params-runtime/[slug]/page.tsx new file mode 100644 index 000000000000..df3aff76c38e --- /dev/null +++ b/test/e2e/app-dir/instant-navigation-testing-api/fixtures/default/app/ungenerated-params-runtime/[slug]/page.tsx @@ -0,0 +1,41 @@ +import { Suspense } from 'react' + +// Same shape as `ungenerated-params/[slug]` (no `generateStaticParams`), but +// this route opts into runtime prefetching. The prefetch therefore includes +// the resolved `slug`, so the param should be visible inside the instant +// scope instead of suspending. +export const unstable_instant: { + samples: Array<{ params: { slug: string } }> +} = { + samples: [{ params: { slug: 'anything' } }], +} +export const unstable_prefetch = 'force-runtime' + +export default function UngeneratedParamsRuntimePage({ + params, +}: { + params: Promise<{ slug: string }> +}) { + return ( +
+

+ Ungenerated Params Runtime Page +

+ + Loading params... +
+ } + > + + + + ) +} + +async function ParamContent({ params }: { params: Promise<{ slug: string }> }) { + const { slug } = await params + + return
slug: {slug}
+} diff --git a/test/e2e/app-dir/instant-navigation-testing-api/fixtures/default/app/ungenerated-params/[slug]/page.tsx b/test/e2e/app-dir/instant-navigation-testing-api/fixtures/default/app/ungenerated-params/[slug]/page.tsx new file mode 100644 index 000000000000..91f85f0055a0 --- /dev/null +++ b/test/e2e/app-dir/instant-navigation-testing-api/fixtures/default/app/ungenerated-params/[slug]/page.tsx @@ -0,0 +1,28 @@ +import { Suspense } from 'react' + +// Intentionally no generateStaticParams — this exercises the fallback-shell +// path for a dynamic route whose URL is not covered by static params. +export default function UngeneratedParamsPage({ + params, +}: { + params: Promise<{ slug: string }> +}) { + return ( +
+

Ungenerated Params Page

+ Loading params...
+ } + > + + + + ) +} + +async function ParamContent({ params }: { params: Promise<{ slug: string }> }) { + const { slug } = await params + + return
slug: {slug}
+} diff --git a/test/e2e/app-dir/instant-navigation-testing-api/instant-navigation-testing-api.test.ts b/test/e2e/app-dir/instant-navigation-testing-api/instant-navigation-testing-api.test.ts index e98915da382f..9ab0306faa87 100644 --- a/test/e2e/app-dir/instant-navigation-testing-api/instant-navigation-testing-api.test.ts +++ b/test/e2e/app-dir/instant-navigation-testing-api/instant-navigation-testing-api.test.ts @@ -512,6 +512,52 @@ describe('instant-navigation-testing-api', () => { }) }) + it('does not bake dynamic route params into the instant shell when no generateStaticParams is defined', async () => { + const page = await openPage(next, '/') + + await instant(page, async () => { + await page.click('#link-to-ungenerated-params') + + // Suspense fallback is visible in the instant shell + const fallback = page.locator( + '[data-testid="ungenerated-params-fallback"]' + ) + await fallback.waitFor({ state: 'visible' }) + + // The resolved param value must not be present in the shell + const paramValue = page.locator('[data-testid="ungenerated-param-value"]') + expect(await paramValue.count()).toBe(0) + }) + + // After the instant scope exits, the param value streams in normally + const paramValue = page.locator('[data-testid="ungenerated-param-value"]') + await paramValue.waitFor({ state: 'visible' }) + expect(await paramValue.textContent()).toContain('slug: anything') + }) + + it('does include dynamic route params in the instant shell when runtime prefetching is enabled', async () => { + const page = await openPage(next, '/') + + await instant(page, async () => { + await page.click('#link-to-ungenerated-params-runtime') + + // The param value IS in the shell because the route opts into runtime + // prefetching, so the prefetch resolves `slug` rather than returning + // the generic fallback. + const paramValue = page.locator( + '[data-testid="ungenerated-param-runtime-value"]' + ) + await paramValue.waitFor({ state: 'visible' }) + expect(await paramValue.textContent()).toContain('slug: anything') + + // Suspense fallback is NOT visible + const fallback = page.locator( + '[data-testid="ungenerated-params-runtime-fallback"]' + ) + expect(await fallback.count()).toBe(0) + }) + }) + // In dev mode, hover/intent-based prefetches should not send requests // that produce stale segment data. If a hover prefetch caches the route // with resolved runtime data before the instant lock is acquired, params