From 93710ddb42a17c359d727aff91983ac1827a23b5 Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Mon, 22 Jan 2024 14:31:16 -0800 Subject: [PATCH] disable static generation on interception routes --- packages/next/src/build/index.ts | 51 +++++++++++-------- packages/next/src/build/utils.ts | 9 +++- .../interception-route-groups.test.ts | 29 +++-------- .../app/baz/@modal/(.)modal/page.tsx | 3 ++ .../app/baz/@modal/default.tsx | 3 ++ .../app/baz/layout.tsx | 8 +++ .../app/baz/modal/page.tsx | 3 ++ .../app/baz/page.tsx | 9 ++++ .../interception-route-prefetch-cache.test.ts | 39 +++++++++++++- 9 files changed, 108 insertions(+), 46 deletions(-) create mode 100644 test/e2e/app-dir/interception-route-prefetch-cache/app/baz/@modal/(.)modal/page.tsx create mode 100644 test/e2e/app-dir/interception-route-prefetch-cache/app/baz/@modal/default.tsx create mode 100644 test/e2e/app-dir/interception-route-prefetch-cache/app/baz/layout.tsx create mode 100644 test/e2e/app-dir/interception-route-prefetch-cache/app/baz/modal/page.tsx create mode 100644 test/e2e/app-dir/interception-route-prefetch-cache/app/baz/page.tsx diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts index 6e177b864ce..cb5c624a21f 100644 --- a/packages/next/src/build/index.ts +++ b/packages/next/src/build/index.ts @@ -162,6 +162,7 @@ import { hasCustomExportOutput } from '../export/utils' import { interopDefault } from '../lib/interop-default' import { formatDynamicImportPath } from '../lib/format-dynamic-import-path' import { isDefaultRoute } from '../lib/is-default-route' +import { isInterceptionRouteAppPath } from '../server/future/helpers/interception-routes' interface ExperimentalBypassForInfo { experimentalBypassFor?: RouteHas[] @@ -1855,7 +1856,7 @@ export default async function build( ) } else { // If this route can be partially pre-rendered, then - // mark it as such and mark it that it can be + // mark it as such and mark that it can be // generated server-side. if (workerResult.isPPR) { isPPR = workerResult.isPPR @@ -1883,6 +1884,8 @@ export default async function build( } const appConfig = workerResult.appConfig || {} + const isInterceptionRoute = + isInterceptionRouteAppPath(page) if (appConfig.revalidate !== 0) { const isDynamic = isDynamicRoute(page) const hasGenerateStaticParams = @@ -1898,26 +1901,29 @@ export default async function build( ) } - if ( - // Mark the app as static if: - // - It has no dynamic param - // - It doesn't have generateStaticParams but `dynamic` is set to - // `error` or `force-static` - !isDynamic - ) { - appStaticPaths.set(originalAppPath, [page]) - appStaticPathsEncoded.set(originalAppPath, [page]) - isStatic = true - } else if ( - isDynamic && - !hasGenerateStaticParams && - (appConfig.dynamic === 'error' || - appConfig.dynamic === 'force-static') - ) { - appStaticPaths.set(originalAppPath, []) - appStaticPathsEncoded.set(originalAppPath, []) - isStatic = true - isPPR = false + // Mark the app as static if: + // - It's not an interception route (these currently depend on request headers and cannot be computed at build) + // - It has no dynamic param + // - It doesn't have generateStaticParams but `dynamic` is set to + // `error` or `force-static` + if (!isInterceptionRoute) { + if (!isDynamic) { + appStaticPaths.set(originalAppPath, [page]) + appStaticPathsEncoded.set(originalAppPath, [ + page, + ]) + isStatic = true + } else if ( + isDynamic && + !hasGenerateStaticParams && + (appConfig.dynamic === 'error' || + appConfig.dynamic === 'force-static') + ) { + appStaticPaths.set(originalAppPath, []) + appStaticPathsEncoded.set(originalAppPath, []) + isStatic = true + isPPR = false + } } } @@ -1934,7 +1940,8 @@ export default async function build( !isStatic && !isAppRouteRoute(originalAppPath) && !isDynamicRoute(originalAppPath) && - !isPPR + !isPPR && + !isInterceptionRoute ) { appPrefetchPaths.set(originalAppPath, page) } diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index 91818fd3d71..63815f59d35 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -83,6 +83,7 @@ import { isAppRouteRouteModule } from '../server/future/route-modules/checks' import { interopDefault } from '../lib/interop-default' import type { PageExtensions } from './page-extensions-type' import { formatDynamicImportPath } from '../lib/format-dynamic-import-path' +import { isInterceptionRouteAppPath } from '../server/future/helpers/interception-routes' export type ROUTER_TYPE = 'pages' | 'app' @@ -1744,7 +1745,7 @@ export async function isPageStatic({ isStatic = true } - // When PPR is enabled, any route may contain or be completely static, so + // When PPR is enabled, any route may be completely static, so // mark this route as static. let isPPR = false if (ppr && routeModule.definition.kind === RouteKind.APP_PAGE) { @@ -1752,6 +1753,12 @@ export async function isPageStatic({ isStatic = true } + // interception routes depend on `Next-URL` and `Next-Router-State-Tree` request headers and thus cannot be prerendered + if (isInterceptionRouteAppPath(page)) { + isStatic = false + isPPR = false + } + return { isStatic, isPPR, diff --git a/test/e2e/app-dir/interception-route-groups/interception-route-groups.test.ts b/test/e2e/app-dir/interception-route-groups/interception-route-groups.test.ts index 562d8b5bb13..548af8faa8f 100644 --- a/test/e2e/app-dir/interception-route-groups/interception-route-groups.test.ts +++ b/test/e2e/app-dir/interception-route-groups/interception-route-groups.test.ts @@ -127,27 +127,12 @@ createNextDescribe( app: new FileRef(path.join(__dirname, 'app')), }, }, - ({ next, isNextStart }) => { - if (process.env.__NEXT_EXPERIMENTAL_PPR === 'true' && isNextStart) { - // The PPR prefetch will 404 since it'll request the full page (which won't exist, since the intercepted route - // has no default). The default router behavior if a prefetch fails is to trigger an MPA navigation - it('should render the non-intercepted page on navigation', async () => { - const browser = await next.browser('/') - - await browser.elementByCss('[href="/photos/1"]').click() - await check(() => browser.elementById('slot').text(), /@slot default/) - await check( - () => browser.elementById('children').text(), - /Photo Page \(non-intercepted\) 1/ - ) - }) - } else { - it('should use the default fallback (a 404) if there is no custom default page', async () => { - const browser = await next.browser('/') - - await browser.elementByCss('[href="/photos/1"]').click() - await check(() => browser.elementByCss('body').text(), /404/) - }) - } + ({ next }) => { + it('should use the default fallback (a 404) if there is no custom default page', async () => { + const browser = await next.browser('/') + + await browser.elementByCss('[href="/photos/1"]').click() + await check(() => browser.elementByCss('body').text(), /404/) + }) } ) diff --git a/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/@modal/(.)modal/page.tsx b/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/@modal/(.)modal/page.tsx new file mode 100644 index 00000000000..c7dca9d0375 --- /dev/null +++ b/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/@modal/(.)modal/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return
Interception Modal
+} diff --git a/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/@modal/default.tsx b/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/@modal/default.tsx new file mode 100644 index 00000000000..c17431379f9 --- /dev/null +++ b/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/@modal/default.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return null +} diff --git a/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/layout.tsx b/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/layout.tsx new file mode 100644 index 00000000000..1314a763344 --- /dev/null +++ b/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/layout.tsx @@ -0,0 +1,8 @@ +export default function Layout(props) { + return ( + <> +
{props.children}
+
{props.modal}
+ + ) +} diff --git a/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/modal/page.tsx b/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/modal/page.tsx new file mode 100644 index 00000000000..353cfd3dab0 --- /dev/null +++ b/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/modal/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return
Modal Page
+} diff --git a/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/page.tsx b/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/page.tsx new file mode 100644 index 00000000000..2d80f0dfba4 --- /dev/null +++ b/test/e2e/app-dir/interception-route-prefetch-cache/app/baz/page.tsx @@ -0,0 +1,9 @@ +import Link from 'next/link' + +export default function Page() { + return ( +
+ Open Interception Modal +
+ ) +} diff --git a/test/e2e/app-dir/interception-route-prefetch-cache/interception-route-prefetch-cache.test.ts b/test/e2e/app-dir/interception-route-prefetch-cache/interception-route-prefetch-cache.test.ts index a2c59ac600b..b830f64f4a9 100644 --- a/test/e2e/app-dir/interception-route-prefetch-cache/interception-route-prefetch-cache.test.ts +++ b/test/e2e/app-dir/interception-route-prefetch-cache/interception-route-prefetch-cache.test.ts @@ -1,12 +1,13 @@ import { createNextDescribe } from 'e2e-utils' import { check } from 'next-test-utils' +import { Response } from 'playwright-chromium' createNextDescribe( 'interception-route-prefetch-cache', { files: __dirname, }, - ({ next }) => { + ({ next, isNextStart }) => { it('should render the correct interception when two distinct layouts share the same path structure', async () => { const browser = await next.browser('/') @@ -41,5 +42,41 @@ createNextDescribe( /Intercepted on Bar Page/ ) }) + + if (isNextStart) { + it('should not be a cache HIT when prefetching an interception route', async () => { + const responses: { cacheStatus: string; pathname: string }[] = [] + const browser = await next.browser('/baz', { + beforePageLoad(page) { + page.on('response', (response: Response) => { + const url = new URL(response.url()) + const request = response.request() + const responseHeaders = response.headers() + const requestHeaders = request.headers() + if (requestHeaders['next-router-prefetch']) { + responses.push({ + cacheStatus: responseHeaders['x-nextjs-cache'], + pathname: url.pathname, + }) + } + }) + }, + }) + + expect(await browser.elementByCss('body').text()).toContain( + 'Open Interception Modal' + ) + + const interceptionPrefetchResponse = responses.find( + (response) => response.pathname === '/baz/modal' + ) + const homePrefetchResponse = responses.find( + (response) => response.pathname === '/' + ) + + expect(homePrefetchResponse.cacheStatus).toBe('HIT') // sanity check to ensure we're seeing cache statuses + expect(interceptionPrefetchResponse.cacheStatus).toBeUndefined() + }) + } } )