From 9e686d48800c80c768f5830ebd8265a4e18f038d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 6 Dec 2024 12:51:43 -0500 Subject: [PATCH] [Segment Cache] Skip prefetched segments on server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently if you navigate to a partially static route, the server will always start rendering at the first segment that's not present on the previous page. However, it should really start rendering at the first *dynamic* segment — if the client has already prefetched a segment, and it's fully static, there's no reason to render it again during the dynamic server render. We can do this by sending a more specific Next-Router-State-Tree request header. Rather than send a tree that represents the previous route, we sent the tree of the target route, but with a `refetch` marker added to the first dynamic segment. (Without the refetch marker, the server would send back an empty response.) This is determined by diffing againt both the previous route *and* the prefetch cache. For now, this only works up to the first dynamic segment inside the new subtree; once the server starts rendering along a path, it renders everything else along that path. We could improve this in the future to also omit static segments that appear inside a dynamic layout, though this would likely require a change to the Next-Router-State- Tree protocol. --- .../router-reducer/ppr-navigations.ts | 87 ++++++++++++++----- .../reducers/navigate-reducer.ts | 5 +- .../components/segment-cache/navigation.ts | 18 +++- .../basic/app/partially-static/page.tsx | 17 ++++ .../partially-static/target-page/layout.tsx | 12 +++ .../app/partially-static/target-page/page.tsx | 17 ++++ .../basic/segment-cache-basic.test.ts | 47 ++++++++++ 7 files changed, 177 insertions(+), 26 deletions(-) create mode 100644 test/e2e/app-dir/segment-cache/basic/app/partially-static/page.tsx create mode 100644 test/e2e/app-dir/segment-cache/basic/app/partially-static/target-page/layout.tsx create mode 100644 test/e2e/app-dir/segment-cache/basic/app/partially-static/target-page/page.tsx diff --git a/packages/next/src/client/components/router-reducer/ppr-navigations.ts b/packages/next/src/client/components/router-reducer/ppr-navigations.ts index 68c48d0ed88db6..db682d8b2ea4d0 100644 --- a/packages/next/src/client/components/router-reducer/ppr-navigations.ts +++ b/packages/next/src/client/components/router-reducer/ppr-navigations.ts @@ -28,9 +28,11 @@ export type Task = { // represents a brand new Cache Node tree, which way or may not need to be // filled with dynamic data from the server. node: CacheNode | null - // Whether anything in this tree contains dynamic holes that need to be filled - // by the server. - needsDynamicRequest: boolean + // The tree sent to the server during the dynamic request. This is the + // same as `route`, except with the `refetch` marker set on dynamic segments. + // If all the segments are static, then this will be null, and no server + // request is required. + dynamicRequestTree: FlightRouterState | null children: Map | null } @@ -111,6 +113,16 @@ export function updateCacheNodeOnNavigation( // This starts off as `false`, and is set to `true` if any of the child // routes requires a dynamic request. let needsDynamicRequest = false + // As we traverse the children, we'll construct a FlightRouterState that can + // be sent to the server to request the dynamic data. If it turns out that + // nothing in the subtree is dynamic (i.e. needsDynamicRequest is false at the + // end), then this will be discarded. + // TODO: We can probably optimize the format of this data structure to only + // include paths that are dynamic. Instead of reusing the + // FlightRouterState type. + let dynamicRequestTreeChildren: { + [parallelRouteKey: string]: FlightRouterState + } = {} for (let parallelRouteKey in newRouterStateChildren) { const newRouterStateChild: FlightRouterState = @@ -209,17 +221,24 @@ export function updateCacheNodeOnNavigation( prefetchParallelRoutes.set(parallelRouteKey, newSegmentMapChild) } - if (taskChild.needsDynamicRequest) { - needsDynamicRequest = true - } - // The child tree's route state may be different from the prefetched // route sent by the server. We need to clone it as we traverse back up // the tree. - patchedRouterStateChildren[parallelRouteKey] = taskChild.route + const taskChildRoute = taskChild.route + patchedRouterStateChildren[parallelRouteKey] = taskChildRoute + + const dynamicRequestTreeChild = taskChild.dynamicRequestTree + if (dynamicRequestTreeChild !== null) { + // Something in the child tree is dynamic. + needsDynamicRequest = true + dynamicRequestTreeChildren[parallelRouteKey] = dynamicRequestTreeChild + } else { + dynamicRequestTreeChildren[parallelRouteKey] = taskChildRoute + } } else { // The child didn't change. We can use the prefetched router state. patchedRouterStateChildren[parallelRouteKey] = newRouterStateChild + dynamicRequestTreeChildren[parallelRouteKey] = newRouterStateChild } } @@ -253,7 +272,12 @@ export function updateCacheNodeOnNavigation( patchedRouterStateChildren ), node: newCacheNode, - needsDynamicRequest, + dynamicRequestTree: needsDynamicRequest + ? patchRouterStateWithNewChildren( + newRouterState, + dynamicRequestTreeChildren + ) + : null, children: taskChildren, } } @@ -312,6 +336,9 @@ function createCacheNodeOnNavigation( const prefetchDataChildren = prefetchData[2] const taskChildren = new Map() const cacheNodeChildren = new Map() + let dynamicRequestTreeChildren: { + [parallelRouteKey: string]: FlightRouterState + } = {} let needsDynamicRequest = false for (let parallelRouteKey in routerStateChildren) { const routerStateChild: FlightRouterState = @@ -329,8 +356,13 @@ function createCacheNodeOnNavigation( isPrefetchHeadPartial ) taskChildren.set(parallelRouteKey, taskChild) - if (taskChild.needsDynamicRequest) { + const dynamicRequestTreeChild = taskChild.dynamicRequestTree + if (dynamicRequestTreeChild !== null) { + // Something in the child tree is dynamic. needsDynamicRequest = true + dynamicRequestTreeChildren[parallelRouteKey] = dynamicRequestTreeChild + } else { + dynamicRequestTreeChildren[parallelRouteKey] = routerStateChild } const newCacheNodeChild = taskChild.node if (newCacheNodeChild !== null) { @@ -343,6 +375,10 @@ function createCacheNodeOnNavigation( const rsc = prefetchData[1] const loading = prefetchData[3] return { + // Since we're inside a new route tree, unlike the + // `updateCacheNodeOnNavigation` path, the router state on the children + // tasks is always the same as the router state we pass in. So we don't need + // to clone/modify it. route: routerState, node: { lazyData: null, @@ -355,7 +391,9 @@ function createCacheNodeOnNavigation( loading, parallelRoutes: cacheNodeChildren, }, - needsDynamicRequest, + dynamicRequestTree: needsDynamicRequest + ? patchRouterStateWithNewChildren(routerState, dynamicRequestTreeChildren) + : null, children: taskChildren, } } @@ -387,6 +425,15 @@ function spawnPendingTask( isPrefetchHeadPartial: boolean ): Task { // Create a task that will later be fulfilled by data from the server. + + // Clone the prefetched route tree and the `refetch` marker to it. We'll send + // this to the server so it knows where to start rendering. + const dynamicRequestTree = patchRouterStateWithNewChildren( + routerState, + routerState[1] + ) + dynamicRequestTree[3] = 'refetch' + const newTask: Task = { route: routerState, @@ -397,9 +444,9 @@ function spawnPendingTask( prefetchHead, isPrefetchHeadPartial ), - // Set this to true to indicate that this tree is missing data. This will - // be propagated to all the parent tasks. - needsDynamicRequest: true, + // Because this is non-null, and it gets propagated up through the parent + // tasks, the root task will know that it needs to perform a server request. + dynamicRequestTree, children: null, } return newTask @@ -411,7 +458,7 @@ function spawnReusedTask(reusedRouterState: FlightRouterState): Task { return { route: reusedRouterState, node: null, - needsDynamicRequest: false, + dynamicRequestTree: null, children: null, } } @@ -533,7 +580,7 @@ function finishTaskUsingDynamicDataPayload( dynamicData: CacheNodeSeedData, dynamicHead: React.ReactNode ) { - if (!task.needsDynamicRequest) { + if (task.dynamicRequestTree === null) { // Everything in this subtree is already complete. Bail out. return } @@ -554,8 +601,8 @@ function finishTaskUsingDynamicDataPayload( dynamicData, dynamicHead ) - // Set this to false to indicate that this task is now complete. - task.needsDynamicRequest = false + // Set this to null to indicate that this task is now complete. + task.dynamicRequestTree = null } return } @@ -775,8 +822,8 @@ export function abortTask(task: Task, error: any): void { } } - // Set this to false to indicate that this task is now complete. - task.needsDynamicRequest = false + // Set this to null to indicate that this task is now complete. + task.dynamicRequestTree = null } function abortPendingCacheNode( diff --git a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts index 9921442edc6a41..bc41f5b8c075ed 100644 --- a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts +++ b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts @@ -335,7 +335,8 @@ export function navigateReducer( // version of the next page. This can be rendered instantly. mutable.cache = newCache } - if (task.needsDynamicRequest) { + const dynamicRequestTree = task.dynamicRequestTree + if (dynamicRequestTree !== null) { // The prefetched tree has dynamic holes in it. We initiate a // dynamic request to fill them in. // @@ -350,7 +351,7 @@ export function navigateReducer( // a different response than we expected. For now, we revert back // to the lazy fetching mechanism in that case.) const dynamicRequest = fetchServerResponse(url, { - flightRouterState: currentTree, + flightRouterState: dynamicRequestTree, nextUrl: state.nextUrl, }) diff --git a/packages/next/src/client/components/segment-cache/navigation.ts b/packages/next/src/client/components/segment-cache/navigation.ts index 2f00f1d1767e7b..7b3ee3417121de 100644 --- a/packages/next/src/client/components/segment-cache/navigation.ts +++ b/packages/next/src/client/components/segment-cache/navigation.ts @@ -144,9 +144,10 @@ function navigateUsingPrefetchedRouteTree( isPrefetchHeadPartial ) if (task !== null) { - if (task.needsDynamicRequest) { + const dynamicRequestTree = task.dynamicRequestTree + if (dynamicRequestTree !== null) { const promiseForDynamicServerResponse = fetchServerResponse(url, { - flightRouterState: currentFlightRouterState, + flightRouterState: dynamicRequestTree, nextUrl, }) listenForDynamicRequest(task, promiseForDynamicServerResponse) @@ -323,11 +324,20 @@ async function navigateDynamicallyWithNoPrefetch( isPrefetchHeadPartial ) if (task !== null) { - if (task.needsDynamicRequest) { + // In this case, we've already sent the dynamic request, so we don't + // actually use the request tree created by `updateCacheNodeOnNavigation`, + // except to check if it contains dynamic holes. + // + // This is almost always true, but it could be false if all the segment data + // was present in the cache, but the route tree was not. E.g. navigating + // to a URL that was not prefetched but rewrites to a different URL + // that was. + const hasDynamicHoles = task.dynamicRequestTree !== null + if (hasDynamicHoles) { listenForDynamicRequest(task, promiseForDynamicServerResponse) } else { // The prefetched tree does not contain dynamic holes — it's - // fully static. We can skip the dynamic request. + // fully static. We don't need to process the server response further. } return navigationTaskToResult(task, currentCacheNode, canonicalUrl) } diff --git a/test/e2e/app-dir/segment-cache/basic/app/partially-static/page.tsx b/test/e2e/app-dir/segment-cache/basic/app/partially-static/page.tsx new file mode 100644 index 00000000000000..f6efcc888513a2 --- /dev/null +++ b/test/e2e/app-dir/segment-cache/basic/app/partially-static/page.tsx @@ -0,0 +1,17 @@ +import Link from 'next/link' + +export default function FullyStaticStart() { + return ( + <> +

+ Demonstrates that when navigating to a partially static route, the + server does not render static layouts that were already prefetched. +

+ + + ) +} diff --git a/test/e2e/app-dir/segment-cache/basic/app/partially-static/target-page/layout.tsx b/test/e2e/app-dir/segment-cache/basic/app/partially-static/target-page/layout.tsx new file mode 100644 index 00000000000000..f50ba6d94832fe --- /dev/null +++ b/test/e2e/app-dir/segment-cache/basic/app/partially-static/target-page/layout.tsx @@ -0,0 +1,12 @@ +export default function StaticLayout({ + children, +}: { + children: React.ReactNode +}) { + return ( + <> +
Static layout
+
{children}
+ + ) +} diff --git a/test/e2e/app-dir/segment-cache/basic/app/partially-static/target-page/page.tsx b/test/e2e/app-dir/segment-cache/basic/app/partially-static/target-page/page.tsx new file mode 100644 index 00000000000000..9ef20e76811e47 --- /dev/null +++ b/test/e2e/app-dir/segment-cache/basic/app/partially-static/target-page/page.tsx @@ -0,0 +1,17 @@ +import { Suspense } from 'react' +import { connection } from 'next/server' + +async function Content() { + await connection() + return 'Dynamic page' +} + +export default function DynamicPage() { + return ( +
+ + + +
+ ) +} diff --git a/test/e2e/app-dir/segment-cache/basic/segment-cache-basic.test.ts b/test/e2e/app-dir/segment-cache/basic/segment-cache-basic.test.ts index 623a6155f3b64a..0adc43905d7589 100644 --- a/test/e2e/app-dir/segment-cache/basic/segment-cache-basic.test.ts +++ b/test/e2e/app-dir/segment-cache/basic/segment-cache-basic.test.ts @@ -131,6 +131,53 @@ describe('segment cache (basic tests)', () => { const numberOfNavigationRequests = (await navigationsLock.release()).size expect(numberOfNavigationRequests).toBe(0) }) + + it('skips static layouts during partially static navigation', async () => { + const interceptor = createRequestInterceptor() + let page: Page + const browser = await next.browser('/partially-static', { + beforePageLoad(p: Page) { + page = p + page.route('**/*', async (route: Route) => { + await interceptor.interceptRoute(route) + }) + }, + }) + + // Rendering the link triggers a prefetch of the test page. + const link = await browser.elementByCss( + 'a[href="/partially-static/target-page"]' + ) + const navigationsLock = interceptor.lockNavigations() + await link.click() + + // The static layout and the loading state of the dynamic page should render + // immediately because they were prefetched. + const layoutMarkerId = 'static-layout' + const layoutMarker = await browser.elementById(layoutMarkerId) + const layoutMarkerContent = await layoutMarker.innerHTML() + expect(layoutMarkerContent).toBe('Static layout') + const dynamicDiv = await browser.elementById('dynamic-page') + expect(await dynamicDiv.innerHTML()).toBe('Loading...') + + // Unblock the navigation request to allow the dynamic content to stream in. + const navigationRoutes = await navigationsLock.release() + + // Check that the navigation response does not include the static layout, + // since it was already prefetched. Because this is not observable in the + // UI, we check the navigation response body. + const numberOfNavigationRequests = navigationRoutes.size + expect(numberOfNavigationRequests).toBe(1) + for (const route of navigationRoutes) { + const response = await page.request.fetch(route.request()) + const responseText = await response.text() + expect(responseText).not.toContain(layoutMarkerId) + expect(responseText).not.toContain(layoutMarkerContent) + } + + // The dynamic content has streamed in. + expect(await dynamicDiv.innerHTML()).toBe('Dynamic page') + }) }) function createRequestInterceptor() {