From 847d5036ed11f8f052d3e680b7cbe8d701672abe Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 10 Dec 2024 17:05:18 -0500 Subject: [PATCH] [Segment Cache] No data on tree prefetch if no PPR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the client Segment Cache attempts to prefetch a route tree, and it the route does not have PPR enabled, the server falls through to the old implementation, which sends back a FlightRouterState. This is fine, since the client can use the FlightRouterState to recreate the non-PPR behavior. However, when loading.tsx is present in the route, the server will not just send back the FlightRouterState, but also render the page up to the first loading boundary. This is not what we want in the Segment Cache implementation — we want the route tree only, so we can determine which parts the client already has before sending a second request for the rest. So this PR disables the loading.tsx mechanism when prefetching the route tree, by checking the Next-Router-Segment-Prefetch header. --- .../next/src/server/app-render/app-render.tsx | 7 ++++++ .../walk-tree-with-flight-router-state.tsx | 11 ++++++--- .../loading.tsx | 15 ++++++++++++ .../page.tsx | 6 +++++ .../segment-cache-incremental-opt-in.test.ts | 24 +++++++++++++++++++ 5 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 test/e2e/app-dir/segment-cache/incremental-opt-in/app/ppr-disabled-with-loading-boundary/loading.tsx create mode 100644 test/e2e/app-dir/segment-cache/incremental-opt-in/app/ppr-disabled-with-loading-boundary/page.tsx diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 3036d801d4440..ca552c01bdfaf 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -53,6 +53,7 @@ import { NEXT_ROUTER_STALE_TIME_HEADER, NEXT_URL, RSC_HEADER, + NEXT_ROUTER_SEGMENT_PREFETCH_HEADER, } from '../../client/components/app-router-headers' import { createTrackedMetadataContext, @@ -234,6 +235,7 @@ interface ParsedRequestHeaders { */ readonly flightRouterState: FlightRouterState | undefined readonly isPrefetchRequest: boolean + readonly isRouteTreePrefetchRequest: boolean readonly isDevWarmupRequest: boolean readonly isHmrRefresh: boolean readonly isRSCRequest: boolean @@ -267,6 +269,10 @@ function parseRequestHeaders( ) : undefined + // Checks if this is a prefetch of the Route Tree by the Segment Cache + const isRouteTreePrefetchRequest = + headers[NEXT_ROUTER_SEGMENT_PREFETCH_HEADER.toLowerCase()] === '/_tree' + const csp = headers['content-security-policy'] || headers['content-security-policy-report-only'] @@ -277,6 +283,7 @@ function parseRequestHeaders( return { flightRouterState, isPrefetchRequest, + isRouteTreePrefetchRequest, isHmrRefresh, isRSCRequest, isDevWarmupRequest, diff --git a/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx b/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx index cef329040f321..f28fee51508b3 100644 --- a/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx +++ b/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx @@ -62,6 +62,7 @@ export async function walkTreeWithFlightRouterState({ query, isPrefetch, getDynamicParamFromSegment, + parsedRequestHeaders, } = ctx const [segment, parallelRoutes, modules] = loaderTreeToFilter @@ -115,9 +116,13 @@ export async function walkTreeWithFlightRouterState({ // somewhere in the tree, we'll recursively render the component tree up until we encounter that loading component, and then stop. const shouldSkipComponentTree = !experimental.isRoutePPREnabled && - isPrefetch && - !Boolean(modules.loading) && - !hasLoadingComponentInTree(loaderTreeToFilter) + // If PPR is disabled, and this is a request for the route tree, then we + // never render any components. Only send the router state. + (parsedRequestHeaders.isRouteTreePrefetchRequest || + // Otherwise, check for the presence of a `loading` component. + (isPrefetch && + !Boolean(modules.loading) && + !hasLoadingComponentInTree(loaderTreeToFilter))) if (!parentRendered && renderComponentsOnThisLevel) { const overriddenSegment = diff --git a/test/e2e/app-dir/segment-cache/incremental-opt-in/app/ppr-disabled-with-loading-boundary/loading.tsx b/test/e2e/app-dir/segment-cache/incremental-opt-in/app/ppr-disabled-with-loading-boundary/loading.tsx new file mode 100644 index 0000000000000..18ad01678962b --- /dev/null +++ b/test/e2e/app-dir/segment-cache/incremental-opt-in/app/ppr-disabled-with-loading-boundary/loading.tsx @@ -0,0 +1,15 @@ +import { Suspense } from 'react' +import { connection } from 'next/server' + +async function Content() { + await connection() + return 'Dynamic Content' +} + +export default function PPRDisabled() { + return ( + + + + ) +} diff --git a/test/e2e/app-dir/segment-cache/incremental-opt-in/app/ppr-disabled-with-loading-boundary/page.tsx b/test/e2e/app-dir/segment-cache/incremental-opt-in/app/ppr-disabled-with-loading-boundary/page.tsx new file mode 100644 index 0000000000000..b92fb53239b5c --- /dev/null +++ b/test/e2e/app-dir/segment-cache/incremental-opt-in/app/ppr-disabled-with-loading-boundary/page.tsx @@ -0,0 +1,6 @@ +import { connection } from 'next/server' + +export default async function PPRDisabledWithLoadingBoundary() { + await connection() + return 'Dynamic Content' +} diff --git a/test/e2e/app-dir/segment-cache/incremental-opt-in/segment-cache-incremental-opt-in.test.ts b/test/e2e/app-dir/segment-cache/incremental-opt-in/segment-cache-incremental-opt-in.test.ts index 1299e7700eedd..498174004fa76 100644 --- a/test/e2e/app-dir/segment-cache/incremental-opt-in/segment-cache-incremental-opt-in.test.ts +++ b/test/e2e/app-dir/segment-cache/incremental-opt-in/segment-cache-incremental-opt-in.test.ts @@ -40,4 +40,28 @@ describe('segment cache (incremental opt in)', () => { const result = extractPseudoJSONFromFlightResponse(flightText) expect(typeof result.b === 'string').toBe(true) }) + + // TODO: Replace with e2e test once the client part is implemented + it('route tree prefetch does not include any component data even if loading.tsx is defined', async () => { + await next.browser('/') + const response = await next.fetch('/ppr-disabled-with-loading-boundary', { + headers: { + RSC: '1', + 'Next-Router-Prefetch': '1', + 'Next-Router-Segment-Prefetch': '/_tree', + }, + }) + expect(response.status).toBe(200) + expect(response.headers.get('x-nextjs-postponed')).toBe(null) + + // Usually when PPR is disabled, a prefetch to a route that has a + // loading.tsx boundary will include component data in the response, up to + // the first loading boundary. But since this is specifically a prefetch + // of the route tree, it should skip all the component data and only return + // the router state. + const flightText = await response.text() + // Confirm that the response does not include any component data by checking + // for the absence of the loading component. + expect(flightText).not.toContain('Loading...') + }) })