From 381830a8cc2e70f60159cdcf9de6bd38b22fdbe3 Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Fri, 17 Nov 2023 12:57:23 -0800 Subject: [PATCH] disable static prefetches --- packages/next/src/export/routes/app-page.ts | 19 ++++- packages/next/src/server/base-server.ts | 45 +++++----- .../app-prefetch/app/force-dynamic/layout.js | 19 +++++ .../app/force-dynamic/search-params/page.js | 11 +++ .../app/force-dynamic/test-page/page.js | 10 +++ .../force-dynamic/test-page/sub-page/page.js | 10 +++ .../app-prefetch/app/revalidate-0/layout.js | 19 +++++ .../app/revalidate-0/search-params/page.js | 11 +++ .../app/revalidate-0/test-page/page.js | 10 +++ .../revalidate-0/test-page/sub-page/page.js | 10 +++ .../app-dir/app-prefetch/prefetching.test.ts | 82 +++++++++++++++---- .../e2e/app-dir/app-static/app-static.test.ts | 36 ++++---- test/e2e/app-dir/app/index.test.ts | 3 +- 13 files changed, 225 insertions(+), 60 deletions(-) create mode 100644 test/e2e/app-dir/app-prefetch/app/force-dynamic/layout.js create mode 100644 test/e2e/app-dir/app-prefetch/app/force-dynamic/search-params/page.js create mode 100644 test/e2e/app-dir/app-prefetch/app/force-dynamic/test-page/page.js create mode 100644 test/e2e/app-dir/app-prefetch/app/force-dynamic/test-page/sub-page/page.js create mode 100644 test/e2e/app-dir/app-prefetch/app/revalidate-0/layout.js create mode 100644 test/e2e/app-dir/app-prefetch/app/revalidate-0/search-params/page.js create mode 100644 test/e2e/app-dir/app-prefetch/app/revalidate-0/test-page/page.js create mode 100644 test/e2e/app-dir/app-prefetch/app/revalidate-0/test-page/sub-page/page.js diff --git a/packages/next/src/export/routes/app-page.ts b/packages/next/src/export/routes/app-page.ts index e0ccd312c0d9e7..775b95382823aa 100644 --- a/packages/next/src/export/routes/app-page.ts +++ b/packages/next/src/export/routes/app-page.ts @@ -37,7 +37,14 @@ async function generatePrefetchRsc( htmlFilepath: string, renderOpts: RenderOpts, fileWriter: FileWriter -) { +): Promise { + // TODO: Re-enable once this is better supported client-side + // It's currently not reliable to generate these prefetches because the client router + // depends on the RSC payload being generated with FlightRouterState. When we generate these prefetches + // without router state, it causes mismatches on client-side nav, resulting in subtle navigation bugs + // like unnecessarily re-rendering layouts. + return false + // When we're in PPR, the RSC payload is emitted as the prefetch payload, so // attempting to generate a prefetch RSC is an error. if (renderOpts.experimental.ppr) { @@ -64,13 +71,15 @@ async function generatePrefetchRsc( const prefetchRscData = await prefetchRenderResult.toUnchunkedString(true) - if ((renderOpts as any).store.staticPrefetchBailout) return + if ((renderOpts as any).store.staticPrefetchBailout) return false await fileWriter( ExportedAppPageFiles.FLIGHT, htmlFilepath.replace(/\.html$/, RSC_PREFETCH_SUFFIX), prefetchRscData ) + + return true } export async function exportAppPage( @@ -94,7 +103,7 @@ export async function exportAppPage( try { if (isAppPrefetch) { - await generatePrefetchRsc( + const generated = await generatePrefetchRsc( req, path, res, @@ -104,7 +113,9 @@ export async function exportAppPage( fileWriter ) - return { revalidate: 0 } + if (generated) { + return { revalidate: 0 } + } } const result = await lazyRenderAppPage( diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index f1e885a390663c..54849d52b92eb4 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -2315,28 +2315,29 @@ export default abstract class Server { { page: pathname, params: opts.params, query, renderOpts } ) } else if (isAppPageRouteModule(routeModule)) { - if ( - !opts.experimental.ppr && - isPrefetchRSCRequest && - process.env.NODE_ENV === 'production' && - !this.minimalMode - ) { - try { - const prefetchRsc = await this.getPrefetchRsc(resolvedUrlPathname) - if (prefetchRsc) { - res.setHeader( - 'cache-control', - 'private, no-cache, no-store, max-age=0, must-revalidate' - ) - res.setHeader('content-type', RSC_CONTENT_TYPE_HEADER) - res.body(prefetchRsc).send() - return null - } - } catch { - // We fallback to invoking the function if prefetch data is not - // available. - } - } + // TODO: Re-enable once static prefetches re-land + // if ( + // !opts.experimental.ppr && + // isPrefetchRSCRequest && + // process.env.NODE_ENV === 'production' && + // !this.minimalMode + // ) { + // try { + // const prefetchRsc = await this.getPrefetchRsc(resolvedUrlPathname) + // if (prefetchRsc) { + // res.setHeader( + // 'cache-control', + // 'private, no-cache, no-store, max-age=0, must-revalidate' + // ) + // res.setHeader('content-type', RSC_CONTENT_TYPE_HEADER) + // res.body(prefetchRsc).send() + // return null + // } + // } catch { + // // We fallback to invoking the function if prefetch data is not + // // available. + // } + // } const module = components.routeModule as AppPageRouteModule diff --git a/test/e2e/app-dir/app-prefetch/app/force-dynamic/layout.js b/test/e2e/app-dir/app-prefetch/app/force-dynamic/layout.js new file mode 100644 index 00000000000000..d48c1a709a6c3c --- /dev/null +++ b/test/e2e/app-dir/app-prefetch/app/force-dynamic/layout.js @@ -0,0 +1,19 @@ +import React from 'react' + +export const dynamic = 'force-dynamic' + +export default async function Layout({ children }) { + console.log('re-fetching in layout') + const data = await fetch( + 'https://next-data-api-endpoint.vercel.app/api/random' + ) + const randomNumber = await data.text() + + return ( +
+

{randomNumber}

+ + {children} +
+ ) +} diff --git a/test/e2e/app-dir/app-prefetch/app/force-dynamic/search-params/page.js b/test/e2e/app-dir/app-prefetch/app/force-dynamic/search-params/page.js new file mode 100644 index 00000000000000..be0b02addb54d7 --- /dev/null +++ b/test/e2e/app-dir/app-prefetch/app/force-dynamic/search-params/page.js @@ -0,0 +1,11 @@ +import Link from 'next/link' + +export default function Home({ searchParams }) { + return ( + <> +
{JSON.stringify(searchParams)}
+ Add search params + Clear Params + + ) +} diff --git a/test/e2e/app-dir/app-prefetch/app/force-dynamic/test-page/page.js b/test/e2e/app-dir/app-prefetch/app/force-dynamic/test-page/page.js new file mode 100644 index 00000000000000..6eafc9a17990e9 --- /dev/null +++ b/test/e2e/app-dir/app-prefetch/app/force-dynamic/test-page/page.js @@ -0,0 +1,10 @@ +import Link from 'next/link' + +export default function Page() { + return ( +
+ Hello from /force-dynamic/test-page{' '} + To Sub Page +
+ ) +} diff --git a/test/e2e/app-dir/app-prefetch/app/force-dynamic/test-page/sub-page/page.js b/test/e2e/app-dir/app-prefetch/app/force-dynamic/test-page/sub-page/page.js new file mode 100644 index 00000000000000..de9cb036b49a4c --- /dev/null +++ b/test/e2e/app-dir/app-prefetch/app/force-dynamic/test-page/sub-page/page.js @@ -0,0 +1,10 @@ +import Link from 'next/link' + +export default function Page() { + return ( +
+ Hello from /force-dynamic/test-page/sub-page{' '} + Back to Test Page +
+ ) +} diff --git a/test/e2e/app-dir/app-prefetch/app/revalidate-0/layout.js b/test/e2e/app-dir/app-prefetch/app/revalidate-0/layout.js new file mode 100644 index 00000000000000..95d001d2c77e5b --- /dev/null +++ b/test/e2e/app-dir/app-prefetch/app/revalidate-0/layout.js @@ -0,0 +1,19 @@ +import React from 'react' + +export const revalidate = 0 + +export default async function Layout({ children }) { + console.log('re-fetching in layout') + const data = await fetch( + 'https://next-data-api-endpoint.vercel.app/api/random' + ) + const randomNumber = await data.text() + + return ( +
+

{randomNumber}

+ + {children} +
+ ) +} diff --git a/test/e2e/app-dir/app-prefetch/app/revalidate-0/search-params/page.js b/test/e2e/app-dir/app-prefetch/app/revalidate-0/search-params/page.js new file mode 100644 index 00000000000000..6faaf156f4fdc8 --- /dev/null +++ b/test/e2e/app-dir/app-prefetch/app/revalidate-0/search-params/page.js @@ -0,0 +1,11 @@ +import Link from 'next/link' + +export default function Home({ searchParams }) { + return ( + <> +
{JSON.stringify(searchParams)}
+ Add search params + Clear Params + + ) +} diff --git a/test/e2e/app-dir/app-prefetch/app/revalidate-0/test-page/page.js b/test/e2e/app-dir/app-prefetch/app/revalidate-0/test-page/page.js new file mode 100644 index 00000000000000..a3eb60248ef2e1 --- /dev/null +++ b/test/e2e/app-dir/app-prefetch/app/revalidate-0/test-page/page.js @@ -0,0 +1,10 @@ +import Link from 'next/link' + +export default function Page() { + return ( +
+ Hello from /revalidate-0/test-page{' '} + To Sub Page +
+ ) +} diff --git a/test/e2e/app-dir/app-prefetch/app/revalidate-0/test-page/sub-page/page.js b/test/e2e/app-dir/app-prefetch/app/revalidate-0/test-page/sub-page/page.js new file mode 100644 index 00000000000000..d672b3beac96d4 --- /dev/null +++ b/test/e2e/app-dir/app-prefetch/app/revalidate-0/test-page/sub-page/page.js @@ -0,0 +1,10 @@ +import Link from 'next/link' + +export default function Page() { + return ( +
+ Hello from /revalidate-0/test-page/sub-page{' '} + Back to Test Page +
+ ) +} diff --git a/test/e2e/app-dir/app-prefetch/prefetching.test.ts b/test/e2e/app-dir/app-prefetch/prefetching.test.ts index 9f8426392ee550..cfbd8ec5c589d3 100644 --- a/test/e2e/app-dir/app-prefetch/prefetching.test.ts +++ b/test/e2e/app-dir/app-prefetch/prefetching.test.ts @@ -301,24 +301,76 @@ createNextDescribe( expect(prefetchResponse).toContain('Loading Prefetch Auto') }) - it('should not generate static prefetches for layouts that opt into dynamic rendering', async () => { - await next.stop() - const rootLoading = await next.readFile('./app/loading.js') - await next.deleteFile('./app/loading.js') - await next.start() - expect( - await next - .readFile('.next/server/app/prefetch-dynamic-usage/foo.prefetch.rsc') + describe('dynamic rendering', () => { + async function hasPrefetch(filePath: string) { + return await next + .readFile(`.next/server/app${filePath}`) .catch(() => false) - ).toBeFalsy() + } + it('should not generate a static prefetch for layouts that use cookies/headers', async () => { + expect( + await hasPrefetch('/prefetch-dynamic-usage/foo.prefetch.rsc') + ).toBeFalsy() - expect( - await next - .readFile('.next/server/app/prefetch-dynamic-usage/foo.prefetch.rsc') - .catch(() => false) - ).toBeFalsy() + expect( + await hasPrefetch('/prefetch-dynamic-usage/bar.prefetch.rsc') + ).toBeFalsy() + }) - await next.patchFile('./app/loading', rootLoading) + describe.each(['/force-dynamic', '/revalidate-0'])('%s', (basePath) => { + it('should not re-render layout when navigating between sub-pages', async () => { + const logStartIndex = next.cliOutput.length + + const browser = await next.browser(`${basePath}/test-page`) + let initialRandomNumber = await browser + .elementById('random-number') + .text() + await browser + .elementByCss(`[href="${basePath}/test-page/sub-page"]`) + .click() + + await check(() => browser.hasElementByCssSelector('#sub-page'), true) + + const newRandomNumber = await browser + .elementById('random-number') + .text() + + expect(initialRandomNumber).toBe(newRandomNumber) + + await check(() => { + const logOccurrences = + next.cliOutput.slice(logStartIndex).split('re-fetching in layout') + .length - 1 + + return logOccurrences + }, 1) + }) + + it('should update search params following a link click', async () => { + const browser = await next.browser(`${basePath}/search-params`) + await check( + () => browser.elementById('search-params-data').text(), + /{}/ + ) + await browser.elementByCss('[href="?foo=true"]').click() + await check( + () => browser.elementById('search-params-data').text(), + /{"foo":"true"}/ + ) + await browser + .elementByCss(`[href="${basePath}/search-params"]`) + .click() + await check( + () => browser.elementById('search-params-data').text(), + /{}/ + ) + await browser.elementByCss('[href="?foo=true"]').click() + await check( + () => browser.elementById('search-params-data').text(), + /{"foo":"true"}/ + ) + }) + }) }) } ) diff --git a/test/e2e/app-dir/app-static/app-static.test.ts b/test/e2e/app-dir/app-static/app-static.test.ts index 868c3a7cae165a..68b8ce979ab4c4 100644 --- a/test/e2e/app-dir/app-static/app-static.test.ts +++ b/test/e2e/app-dir/app-static/app-static.test.ts @@ -507,7 +507,7 @@ createNextDescribe( 'ssg-draft-mode.rsc', 'ssr-forced/page.js', 'articles/works.rsc', - 'custom.prefetch.rsc', + // 'custom.prefetch.rsc', 'force-cache/page.js', 'ssg-draft-mode.html', 'articles/works.html', @@ -526,7 +526,7 @@ createNextDescribe( 'force-static/first.html', 'force-static/second.rsc', 'ssg-draft-mode/test.rsc', - 'ssr-forced.prefetch.rsc', + // 'ssr-forced.prefetch.rsc', 'isr-error-handling.html', 'articles/[slug]/page.js', 'no-store/static/page.js', @@ -537,15 +537,15 @@ createNextDescribe( 'no-store/dynamic/page.js', 'blog/seb/second-post.html', 'ssg-draft-mode/test-2.rsc', - 'response-url.prefetch.rsc', + // 'response-url.prefetch.rsc', 'blog/styfle/first-post.rsc', - 'default-cache.prefetch.rsc', + // 'default-cache.prefetch.rsc', 'dynamic-error/[id]/page.js', 'ssg-draft-mode/test-2.html', 'blog/styfle/first-post.html', 'blog/styfle/second-post.rsc', - 'fetch-no-cache.prefetch.rsc', - 'force-no-store.prefetch.rsc', + // 'fetch-no-cache.prefetch.rsc', + // 'force-no-store.prefetch.rsc', 'force-static/[slug]/page.js', 'hooks/use-pathname/slug.rsc', 'hooks/use-search-params.rsc', @@ -571,12 +571,12 @@ createNextDescribe( 'react-fetch-deduping-node/page.js', 'variable-revalidate/encoding.html', 'variable-revalidate/cookie/page.js', - 'gen-params-dynamic/one.prefetch.rsc', + // 'gen-params-dynamic/one.prefetch.rsc', 'ssg-draft-mode/[[...route]]/page.js', 'variable-revalidate/post-method.rsc', 'stale-cache-serving/app-page/page.js', 'dynamic-no-gen-params/[slug]/page.js', - 'ssr-auto/cache-no-store.prefetch.rsc', + // 'ssr-auto/cache-no-store.prefetch.rsc', 'static-to-dynamic-error/[id]/page.js', 'variable-revalidate/encoding/page.js', 'variable-revalidate/no-store/page.js', @@ -590,7 +590,7 @@ createNextDescribe( 'variable-revalidate/revalidate-3.html', 'force-dynamic-prerender/[slug]/page.js', 'gen-params-dynamic-revalidate/one.html', - 'react-fetch-deduping-node.prefetch.rsc', + // 'react-fetch-deduping-node.prefetch.rsc', 'ssr-auto/fetch-revalidate-zero/page.js', 'variable-revalidate/authorization.html', '_not-found_client-reference-manifest.js', @@ -604,8 +604,8 @@ createNextDescribe( 'variable-revalidate/headers-instance.rsc', 'variable-revalidate/revalidate-3/page.js', 'stale-cache-serving-edge/app-page/page.js', - 'stale-cache-serving/app-page.prefetch.rsc', - 'force-dynamic-catch-all/slug.prefetch.rsc', + // 'stale-cache-serving/app-page.prefetch.rsc', + // 'force-dynamic-catch-all/slug.prefetch.rsc', 'hooks/use-search-params/force-static.html', 'hooks/use-search-params/with-suspense.rsc', 'route-handler/revalidate-360-isr/route.js', @@ -613,13 +613,13 @@ createNextDescribe( 'variable-revalidate-edge/no-store/page.js', 'variable-revalidate/authorization/page.js', 'variable-revalidate/headers-instance.html', - 'variable-revalidate/no-store.prefetch.rsc', + // 'variable-revalidate/no-store.prefetch.rsc', 'stale-cache-serving/route-handler/route.js', 'hooks/use-search-params/with-suspense.html', 'route-handler-edge/revalidate-360/route.js', 'variable-revalidate/revalidate-360-isr.rsc', 'variable-revalidate/revalidate-360/page.js', - 'ssr-auto/fetch-revalidate-zero.prefetch.rsc', + // 'ssr-auto/fetch-revalidate-zero.prefetch.rsc', 'static-to-dynamic-error-forced/[id]/page.js', 'variable-config-revalidate/revalidate-3.rsc', 'variable-revalidate/revalidate-360-isr.html', @@ -630,9 +630,9 @@ createNextDescribe( 'variable-config-revalidate/revalidate-3.html', 'variable-revalidate-edge/post-method/page.js', 'variable-revalidate/headers-instance/page.js', - 'variable-revalidate/status-code.prefetch.rsc', + // 'variable-revalidate/status-code.prefetch.rsc', 'force-cache/page_client-reference-manifest.js', - 'force-dynamic-no-with-revalidate.prefetch.rsc', + // 'force-dynamic-no-with-revalidate.prefetch.rsc', 'hooks/use-search-params/with-suspense/page.js', 'variable-revalidate-edge/revalidate-3/page.js', '(new)/custom/page_client-reference-manifest.js', @@ -642,10 +642,10 @@ createNextDescribe( 'stale-cache-serving-edge/route-handler/route.js', 'blog/[author]/page_client-reference-manifest.js', 'default-cache/page_client-reference-manifest.js', - 'force-dynamic-prerender/frameworks.prefetch.rsc', + // 'force-dynamic-prerender/frameworks.prefetch.rsc', 'variable-config-revalidate/revalidate-3/page.js', 'variable-revalidate/post-method-request/page.js', - 'variable-revalidate/revalidate-360.prefetch.rsc', + // 'variable-revalidate/revalidate-360.prefetch.rsc', 'fetch-no-cache/page_client-reference-manifest.js', 'force-dynamic-catch-all/[slug]/[[...id]]/page.js', 'force-no-store/page_client-reference-manifest.js', @@ -677,7 +677,7 @@ createNextDescribe( 'partial-gen-params-no-additional-lang/fr/second.html', 'partial-gen-params-no-additional-slug/en/second.html', 'partial-gen-params-no-additional-slug/fr/second.html', - 'variable-revalidate/post-method-request.prefetch.rsc', + // 'variable-revalidate/post-method-request.prefetch.rsc', 'variable-revalidate-edge/post-method-request/page.js', 'force-static/[slug]/page_client-reference-manifest.js', 'blog/[author]/[slug]/page_client-reference-manifest.js', diff --git a/test/e2e/app-dir/app/index.test.ts b/test/e2e/app-dir/app/index.test.ts index 6811a37c142f85..eaa1c69224cd56 100644 --- a/test/e2e/app-dir/app/index.test.ts +++ b/test/e2e/app-dir/app/index.test.ts @@ -37,7 +37,8 @@ createNextDescribe( ) }) - it('should use RSC prefetch data from build', async () => { + // TODO: Re-enable once static prefetches are re-landed + it.skip('should use RSC prefetch data from build', async () => { expect( await next.readFile('.next/server/app/linking.prefetch.rsc') ).toBeTruthy()