From 448098b4f8bda0e149babbaa2ffb59e9eee8e2c5 Mon Sep 17 00:00:00 2001 From: Zack Tanner <1939140+ztanner@users.noreply.github.com> Date: Sat, 18 May 2024 11:06:29 -0700 Subject: [PATCH] fix redundant fetching of inactive default segments --- .../app-render/create-component-tree.tsx | 5 ++ .../app/[letter]/@grid/default.tsx | 3 + .../app/[letter]/@grid/loading.tsx | 3 + .../app/[letter]/@grid/page.tsx | 33 +++++++++++ .../app/[letter]/@grid2/default.tsx | 3 + .../app/[letter]/@grid3/default.tsx | 3 + .../app/[letter]/@grid4/default.tsx | 3 + .../app/[letter]/@grid5/default.tsx | 3 + .../app/[letter]/default.tsx | 3 + .../app/[letter]/layout.tsx | 26 +++++++++ .../app/[letter]/loading.tsx | 3 + .../app/[letter]/page.tsx | 8 +++ .../app/layout.tsx | 7 +++ .../parallel-routes-prefetching/app/page.tsx | 5 ++ .../next.config.js | 6 ++ .../parallel-routes-prefetching.test.ts | 58 +++++++++++++++++++ 16 files changed, 172 insertions(+) create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid/default.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid/loading.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid/page.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid2/default.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid3/default.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid4/default.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid5/default.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/[letter]/default.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/[letter]/layout.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/[letter]/loading.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/[letter]/page.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/layout.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/app/page.tsx create mode 100644 test/production/app-dir/parallel-routes-prefetching/next.config.js create mode 100644 test/production/app-dir/parallel-routes-prefetching/parallel-routes-prefetching.test.ts diff --git a/packages/next/src/server/app-render/create-component-tree.tsx b/packages/next/src/server/app-render/create-component-tree.tsx index acee5b460119a..8c5fb49f539ce 100644 --- a/packages/next/src/server/app-render/create-component-tree.tsx +++ b/packages/next/src/server/app-render/create-component-tree.tsx @@ -15,6 +15,7 @@ import { getTracer } from '../lib/trace/tracer' import { NextNodeServerSpan } from '../lib/trace/constants' import { StaticGenBailoutError } from '../../client/components/static-generation-bailout' import type { LoadingModuleData } from '../../shared/lib/app-router-context.shared-runtime' +import { DEFAULT_SEGMENT_KEY } from '../../shared/lib/segment' type ComponentTree = { seedData: CacheNodeSeedData @@ -395,6 +396,10 @@ async function createComponentTreeInternal({ // This behavior predates PPR and is only relevant if the // PPR flag is not enabled. isPrefetch && + // If we'd be rendering the default parallel route component, we include it in the tree. + // Otherwise, the prefetch would cause any parallel routes rendering the default state + // to be refetched on navigation, because the server will have cleared them. + parallelRoute[0] !== DEFAULT_SEGMENT_KEY && (Loading || !hasLoadingComponentInTree(parallelRoute)) && // The approach with PPR is different — loading.tsx behaves like a // regular Suspense boundary and has no special behavior. diff --git a/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid/default.tsx b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid/default.tsx new file mode 100644 index 0000000000000..86b9e9a388129 --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid/default.tsx @@ -0,0 +1,3 @@ +export default function Default() { + return null +} diff --git a/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid/loading.tsx b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid/loading.tsx new file mode 100644 index 0000000000000..15a2da4ac28e0 --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid/loading.tsx @@ -0,0 +1,3 @@ +export default function Loading() { + return

Grid Loading

+} diff --git a/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid/page.tsx b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid/page.tsx new file mode 100644 index 0000000000000..5eb8af3a85ab3 --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid/page.tsx @@ -0,0 +1,33 @@ +import Link from 'next/link' + +const alphabet = Array.from('abcdefghijklmnopqrstuvwxyz') + +function getShuffledAlphabet() { + return alphabet + .map((value) => ({ value, sort: Math.random() })) + .sort((a, b) => a.sort - b.sort) + .map(({ value }) => value) +} + +export default async function Page({ + params: { letter }, +}: { + params: { letter: string } +}) { + await new Promise((r) => setTimeout(r, 250)) + + return ( + <> + {letter} + + + ) +} diff --git a/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid2/default.tsx b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid2/default.tsx new file mode 100644 index 0000000000000..86b9e9a388129 --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid2/default.tsx @@ -0,0 +1,3 @@ +export default function Default() { + return null +} diff --git a/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid3/default.tsx b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid3/default.tsx new file mode 100644 index 0000000000000..86b9e9a388129 --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid3/default.tsx @@ -0,0 +1,3 @@ +export default function Default() { + return null +} diff --git a/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid4/default.tsx b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid4/default.tsx new file mode 100644 index 0000000000000..86b9e9a388129 --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid4/default.tsx @@ -0,0 +1,3 @@ +export default function Default() { + return null +} diff --git a/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid5/default.tsx b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid5/default.tsx new file mode 100644 index 0000000000000..86b9e9a388129 --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/@grid5/default.tsx @@ -0,0 +1,3 @@ +export default function Default() { + return null +} diff --git a/test/production/app-dir/parallel-routes-prefetching/app/[letter]/default.tsx b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/default.tsx new file mode 100644 index 0000000000000..86b9e9a388129 --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/default.tsx @@ -0,0 +1,3 @@ +export default function Default() { + return null +} diff --git a/test/production/app-dir/parallel-routes-prefetching/app/[letter]/layout.tsx b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/layout.tsx new file mode 100644 index 0000000000000..aaba6bb8ab5b8 --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/layout.tsx @@ -0,0 +1,26 @@ +import { ReactNode } from 'react' + +type LetterLayoutProps = { + params: { letter: string } + children: ReactNode + grid: ReactNode + grid2: ReactNode + grid3: ReactNode + grid4: ReactNode + grid5: ReactNode +} + +export default function LetterLayout(props: LetterLayoutProps) { + return ( +
+ {props.children} +
+ {props.grid} + {props.grid2} + {props.grid3} + {props.grid4} + {props.grid5} +
+
+ ) +} diff --git a/test/production/app-dir/parallel-routes-prefetching/app/[letter]/loading.tsx b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/loading.tsx new file mode 100644 index 0000000000000..127f6a1255ee7 --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/loading.tsx @@ -0,0 +1,3 @@ +export default function Loading() { + return

Loading

+} diff --git a/test/production/app-dir/parallel-routes-prefetching/app/[letter]/page.tsx b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/page.tsx new file mode 100644 index 0000000000000..eee824bf3f46d --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/[letter]/page.tsx @@ -0,0 +1,8 @@ +export default async function LetterPage({ + params: { letter }, +}: { + params: { letter: string } +}) { + await new Promise((r) => setTimeout(r, 250)) + return

{letter}

+} diff --git a/test/production/app-dir/parallel-routes-prefetching/app/layout.tsx b/test/production/app-dir/parallel-routes-prefetching/app/layout.tsx new file mode 100644 index 0000000000000..e7077399c03ce --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/layout.tsx @@ -0,0 +1,7 @@ +export default function Root({ children }: { children: React.ReactNode }) { + return ( + + {children} + + ) +} diff --git a/test/production/app-dir/parallel-routes-prefetching/app/page.tsx b/test/production/app-dir/parallel-routes-prefetching/app/page.tsx new file mode 100644 index 0000000000000..aebfb306b399e --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/app/page.tsx @@ -0,0 +1,5 @@ +import Link from 'next/link' + +export default function Page() { + return Go to a letter +} diff --git a/test/production/app-dir/parallel-routes-prefetching/next.config.js b/test/production/app-dir/parallel-routes-prefetching/next.config.js new file mode 100644 index 0000000000000..807126e4cf0bf --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/next.config.js @@ -0,0 +1,6 @@ +/** + * @type {import('next').NextConfig} + */ +const nextConfig = {} + +module.exports = nextConfig diff --git a/test/production/app-dir/parallel-routes-prefetching/parallel-routes-prefetching.test.ts b/test/production/app-dir/parallel-routes-prefetching/parallel-routes-prefetching.test.ts new file mode 100644 index 0000000000000..803034396da9e --- /dev/null +++ b/test/production/app-dir/parallel-routes-prefetching/parallel-routes-prefetching.test.ts @@ -0,0 +1,58 @@ +import { nextTestSetup } from 'e2e-utils' +import { retry } from 'next-test-utils' + +describe('parallel-routes-prefetching', () => { + const { next } = nextTestSetup({ + files: __dirname, + }) + + it('should not re-fetch data for the __DEFAULT__ segments when navigating within a dynamic segment', async () => { + let prefetchRequests = [] + let rscRequests = [] + const browser = await next.browser('/a', { + beforePageLoad(page) { + page.on('request', (request) => { + return request.allHeaders().then((headers) => { + if (headers['Next-Router-Prefetch'.toLowerCase()] === '1') { + prefetchRequests.push(request.url()) + } else if (headers['RSC'.toLowerCase()] === '1') { + rscRequests.push(request.url()) + } + }) + }) + }, + }) + + expect(await browser.elementById('letter-page').text()).toBe('a') + expect(await browser.elementById('letter-parallel').text()).toBe('a') + + await retry(() => { + // we should see 25 prefetch requests for each letter of the alphabet (excluding the current) + expect(prefetchRequests).toHaveLength(25) + // and 0 RSC requests since we're not navigating to a different dynamic segment + expect(rscRequests).toHaveLength(0) + }) + + prefetchRequests = [] + + await browser.elementByCss('[href="/b"]').click() + + await retry(() => { + // Navigating to a new dynamic segment should trigger any additional prefetch requests + expect(prefetchRequests).toHaveLength(0) + + // but we except 2 RSC requests for the new page: + // - one for the `children` slot + // - one for the active parallel slot + // TODO: It would be nice to achieve this in a single request, but it currently isn't possible, + // because the router will see missing data for both slots simultaneously and kick off two separate requests. + // PPR doesn't have this issue because it doesn't perform the "lazy fetch" in layout-router. + expect(rscRequests).toHaveLength( + process.env.__NEXT_EXPERIMENTAL_PPR ? 1 : 2 + ) + }) + + expect(await browser.elementById('letter-page').text()).toBe('b') + expect(await browser.elementById('letter-parallel').text()).toBe('b') + }) +})