From 1861f1fca4af87aabf44b3a51ddc43c806a9bb17 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 4 Dec 2024 22:26:13 -0500 Subject: [PATCH] [Segment Cache] Skip dynamic request if possible During a navigation, if all the data has been prefetched, and the target route does not contain any dynamic data, then we should skip a request to the server. This uses the `isPartial` field I added in the previous PRs to track whether the prefetched data is complete or not. --- .../router-reducer/ppr-navigations.ts | 168 +++++++++++++----- .../reducers/navigate-reducer.ts | 25 +-- .../components/segment-cache/navigation.ts | 90 ++++++---- .../basic/app/fully-static/page.tsx | 18 ++ .../app/fully-static/target-page/page.tsx | 3 + .../basic/segment-cache-basic.test.ts | 28 +++ 6 files changed, 247 insertions(+), 85 deletions(-) create mode 100644 test/e2e/app-dir/segment-cache/basic/app/fully-static/page.tsx create mode 100644 test/e2e/app-dir/segment-cache/basic/app/fully-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 52ec6b86536985..5de3afca58908b 100644 --- a/packages/next/src/client/components/router-reducer/ppr-navigations.ts +++ b/packages/next/src/client/components/router-reducer/ppr-navigations.ts @@ -20,13 +20,14 @@ import type { FetchServerResponseResult } from './fetch-server-response' // request. We can't use the Cache Node tree or Route State tree directly // because those include reused nodes, too. This tree is discarded as soon as // the navigation response is received. -type Task = { +export type Task = { // The router state that corresponds to the tree that this Task represents. route: FlightRouterState - // This is usually non-null. It represents a brand new Cache Node tree whose - // data is still pending. If it's null, it means there's no pending data but - // the client patched the router state. + // Represents a brand new Cache Node tree. It may or may not contain dynamic + // holes, depending on the value of `needsDynamicRequest`. If + // `needsDynamicRequest` is false, then the tree is complete. node: CacheNode | null + needsDynamicRequest: boolean children: Map | null } @@ -64,7 +65,8 @@ export function updateCacheNodeOnNavigation( oldRouterState: FlightRouterState, newRouterState: FlightRouterState, prefetchData: CacheNodeSeedData | null, - prefetchHead: React.ReactNode | null + prefetchHead: React.ReactNode | null, + isPrefetchHeadPartial: boolean ): Task | null { // Diff the old and new trees to reuse the shared layouts. const oldRouterStateChildren = oldRouterState[1] @@ -96,14 +98,15 @@ export function updateCacheNodeOnNavigation( } = {} let taskChildren = null - // For most navigations, we need to issue a "dynamic" request to fetch the - // full RSC data from the server since during rendering, we'll only serve - // the prefetch shell. For some navigations, we re-use the existing cache node - // (via `spawnReusedTask`), and don't actually need fresh data from the server. - // In those cases, we use this `needsDynamicRequest` flag to return a `null` - // cache node, which signals to the caller that we don't need to issue a - // dynamic request. We start off with a `false` value, and then for each parallel - // route, we set it to `true` if we encounter a segment that needs a dynamic request. + // Most navigations require a request to fetch additional data from the + // server, either because the data was not already prefetched, or because the + // target route contains dynamic data that cannot be prefetched. + // + // However, if the target route is fully static, and it's already completely + // loaded into the segment cache, then we can skip the server request. + // + // This starts off as `false`, and is set to `true` if any of the child + // routes requires a dynamic request. let needsDynamicRequest = false for (let parallelRouteKey in newRouterStateChildren) { @@ -141,13 +144,17 @@ export function updateCacheNodeOnNavigation( // Reuse the existing Router State for this segment. We spawn a "task" // just to keep track of the updated router state; unlike most, it's // already fulfilled and won't be affected by the dynamic response. - taskChild = spawnReusedTask(oldRouterStateChild) + taskChild = spawnReusedTask( + oldRouterStateChild, + oldCacheNodeChild !== undefined ? oldCacheNodeChild : null + ) } else { // There's no currently active segment. Switch to the "create" path. taskChild = spawnPendingTask( newRouterStateChild, prefetchDataChild !== undefined ? prefetchDataChild : null, - prefetchHead + prefetchHead, + isPrefetchHeadPartial ) } } else if ( @@ -165,7 +172,8 @@ export function updateCacheNodeOnNavigation( oldRouterStateChild, newRouterStateChild, prefetchDataChild, - prefetchHead + prefetchHead, + isPrefetchHeadPartial ) } else { // Either there's no existing Cache Node for this segment, or this @@ -174,7 +182,8 @@ export function updateCacheNodeOnNavigation( taskChild = spawnPendingTask( newRouterStateChild, prefetchDataChild !== undefined ? prefetchDataChild : null, - prefetchHead + prefetchHead, + isPrefetchHeadPartial ) } } else { @@ -182,7 +191,8 @@ export function updateCacheNodeOnNavigation( taskChild = spawnPendingTask( newRouterStateChild, prefetchDataChild !== undefined ? prefetchDataChild : null, - prefetchHead + prefetchHead, + isPrefetchHeadPartial ) } @@ -197,8 +207,9 @@ export function updateCacheNodeOnNavigation( const newSegmentMapChild: ChildSegmentMap = new Map(oldSegmentMapChild) newSegmentMapChild.set(newSegmentKeyChild, newCacheNodeChild) prefetchParallelRoutes.set(parallelRouteKey, newSegmentMapChild) - // a non-null taskChild.node means we're waiting for a dynamic request to - // fill in the missing data + } + + if (taskChild.needsDynamicRequest) { needsDynamicRequest = true } @@ -241,9 +252,8 @@ export function updateCacheNodeOnNavigation( newRouterState, patchedRouterStateChildren ), - // Only return the new cache node if there are pending tasks that need to be resolved - // by the dynamic data from the server. If they don't, we don't need to trigger a dynamic request. - node: needsDynamicRequest ? newCacheNode : null, + node: newCacheNode, + needsDynamicRequest, children: taskChildren, } } @@ -271,27 +281,39 @@ function patchRouterStateWithNewChildren( function spawnPendingTask( routerState: FlightRouterState, prefetchData: CacheNodeSeedData | null, - prefetchHead: React.ReactNode | null + prefetchHead: React.ReactNode | null, + isPrefetchHeadPartial: boolean ): Task { // Create a task that will later be fulfilled by data from the server. + const task: Task = { + route: routerState, + node: null, + // This will be set to true by `createPendingCacheNode` if any of the + // segments are partial (i.e. contain dynamic holes). + needsDynamicRequest: false, + children: null, + } const pendingCacheNode = createPendingCacheNode( + task, routerState, prefetchData, - prefetchHead + prefetchHead, + isPrefetchHeadPartial ) - return { - route: routerState, - node: pendingCacheNode, - children: null, - } + task.node = pendingCacheNode + return task } -function spawnReusedTask(reusedRouterState: FlightRouterState): Task { +function spawnReusedTask( + reusedRouterState: FlightRouterState, + reusedCacheNode: CacheNode | null +): Task { // Create a task that reuses an existing segment, e.g. when reusing // the current active segment in place of a default route. return { route: reusedRouterState, - node: null, + node: reusedCacheNode, + needsDynamicRequest: false, children: null, } } @@ -413,6 +435,11 @@ function finishTaskUsingDynamicDataPayload( dynamicData: CacheNodeSeedData, dynamicHead: React.ReactNode ) { + if (!task.needsDynamicRequest) { + // Everything in this subtree is already complete. Bail out. + return + } + // dynamicData may represent a larger subtree than the task. Before we can // finish the task, we need to line them up. const taskChildren = task.children @@ -470,9 +497,11 @@ function finishTaskUsingDynamicDataPayload( } function createPendingCacheNode( + task: Task, routerState: FlightRouterState, prefetchData: CacheNodeSeedData | null, - prefetchHead: React.ReactNode | null + possiblyPartialPrefetchHead: React.ReactNode | null, + isPrefetchHeadPartial: boolean ): ReadyCacheNode { const routerStateChildren = routerState[1] const prefetchDataChildren = prefetchData !== null ? prefetchData[2] : null @@ -490,9 +519,11 @@ function createPendingCacheNode( const segmentKeyChild = createRouterCacheKey(segmentChild) const newCacheNodeChild = createPendingCacheNode( + task, routerStateChild, prefetchDataChild === undefined ? null : prefetchDataChild, - prefetchHead + possiblyPartialPrefetchHead, + isPrefetchHeadPartial ) const newSegmentMapChild: ChildSegmentMap = new Map() @@ -504,20 +535,71 @@ function createPendingCacheNode( // on corresponding logic in fill-lazy-items-till-leaf-with-head.ts const isLeafSegment = parallelRoutes.size === 0 - const maybePrefetchRsc = prefetchData !== null ? prefetchData[1] : null - const maybePrefetchLoading = prefetchData !== null ? prefetchData[3] : null + // Populate the `prefetchRsc` and `rsc` fields, depending on whether we have + // prefetch data for this segment, and also whether the prefetch is partial + // or complete (as in the case of a fully static segment). + let prefetchRsc + let rsc + let loading + if (prefetchData !== null) { + const possiblyPartialRsc = prefetchData[1] + const isPrefetchRscPartial = prefetchData[4] + if (isPrefetchRscPartial) { + // This is a partial prefetch. + prefetchRsc = possiblyPartialRsc + // Create a deferred promise. This will be fulfilled once the dynamic + // response is received from the server. + rsc = createDeferredRsc() as React.ReactNode + // Mark the task as needing a dynamic request. + task.needsDynamicRequest = true + } else { + // This is not a partial prefetch, so we can bypass the `prefetchRsc` + // field and go straight to the full `rsc` field. + prefetchRsc = null + rsc = possiblyPartialRsc + } + + // TODO: Technically, a loading boundary could contain dynamic data. We must + // have separate `loading` and `prefetchLoading` fields to handle this, like + // we do for the segment data and head. + loading = prefetchData[3] + } else { + prefetchRsc = null + rsc = createDeferredRsc() as React.ReactNode + task.needsDynamicRequest = true + + loading = null + } + + // The head is stored separately. Since it, too, may contain dynamic holes, + // we need to perform the same check that we did for the segment data. + let head + let prefetchHead + if (isLeafSegment) { + prefetchHead = null + head = null + } else { + if (isPrefetchHeadPartial) { + prefetchHead = possiblyPartialPrefetchHead + head = createDeferredRsc() as React.ReactNode + task.needsDynamicRequest = true + } else { + prefetchHead = null + head = possiblyPartialPrefetchHead + } + } + return { lazyData: null, parallelRoutes: parallelRoutes, - prefetchRsc: maybePrefetchRsc !== undefined ? maybePrefetchRsc : null, - prefetchHead: isLeafSegment ? prefetchHead : null, - loading: maybePrefetchLoading !== undefined ? maybePrefetchLoading : null, + prefetchRsc, + rsc, + + prefetchHead, + head, - // Create a deferred promise. This will be fulfilled once the dynamic - // response is received from the server. - rsc: createDeferredRsc() as React.ReactNode, - head: isLeafSegment ? (createDeferredRsc() as React.ReactNode) : null, + loading, } } 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 0ee5246873cabb..9921442edc6a41 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 @@ -266,6 +266,7 @@ export function navigateReducer( pathToSegment: flightSegmentPath, seedData, head, + isHeadPartial, isRootRender, } = normalizedFlightData let treePatch = normalizedFlightData.tree @@ -316,13 +317,11 @@ export function navigateReducer( currentTree, treePatch, seedData, - head + head, + isHeadPartial ) if (task !== null) { - // We've created a new Cache Node tree that contains a prefetched - // version of the next page. This can be rendered instantly. - // Use the tree computed by updateCacheNodeOnNavigation instead // of the one computed by applyRouterStatePatchToTree. // TODO: We should remove applyRouterStatePatchToTree @@ -330,12 +329,13 @@ export function navigateReducer( const patchedRouterState: FlightRouterState = task.route newTree = patchedRouterState - // It's possible that `updateCacheNodeOnNavigation` only spawned tasks to reuse the existing cache, - // in which case `task.node` will be null, signaling we don't need to wait for a dynamic request - // and can simply apply the patched `FlightRouterState`. - if (task.node !== null) { - const newCache = task.node - + const newCache = task.node + if (newCache !== null) { + // We've created a new Cache Node tree that contains a prefetched + // version of the next page. This can be rendered instantly. + mutable.cache = newCache + } + if (task.needsDynamicRequest) { // The prefetched tree has dynamic holes in it. We initiate a // dynamic request to fill them in. // @@ -359,8 +359,9 @@ export function navigateReducer( // because we're not going to await the dynamic request here. Since we're not blocking // on the dynamic request, `layout-router` will // task.node.lazyData = dynamicRequest - - mutable.cache = newCache + } else { + // The prefetched tree does not contain dynamic holes — it's + // fully static. We can skip the dynamic request. } } else { // Nothing changed, so reuse the old cache. diff --git a/packages/next/src/client/components/segment-cache/navigation.ts b/packages/next/src/client/components/segment-cache/navigation.ts index 39c2b2021df706..2f00f1d1767e7b 100644 --- a/packages/next/src/client/components/segment-cache/navigation.ts +++ b/packages/next/src/client/components/segment-cache/navigation.ts @@ -8,13 +8,11 @@ import type { LoadingModuleData, } from '../../../shared/lib/app-router-context.shared-runtime' import type { NormalizedFlightData } from '../../flight-data-helpers' -import { - fetchServerResponse, - type FetchServerResponseResult, -} from '../router-reducer/fetch-server-response' +import { fetchServerResponse } from '../router-reducer/fetch-server-response' import { updateCacheNodeOnNavigation, listenForDynamicRequest, + type Task as PPRNavigationTask, } from '../router-reducer/ppr-navigations' import { createHrefFromUrl as createCanonicalUrl } from '../router-reducer/create-href-from-url' import { @@ -94,19 +92,18 @@ export function navigate( const prefetchFlightRouterState = snapshot.flightRouterState const prefetchSeedData = snapshot.seedData const prefetchHead = route.head + const isPrefetchHeadPartial = route.isHeadPartial const canonicalUrl = route.canonicalUrl - const promiseForDynamicServerResponse = fetchServerResponse(url, { - flightRouterState: currentFlightRouterState, - nextUrl, - }) return navigateUsingPrefetchedRouteTree( + url, + nextUrl, currentCacheNode, currentFlightRouterState, prefetchFlightRouterState, prefetchSeedData, prefetchHead, - canonicalUrl, - promiseForDynamicServerResponse + isPrefetchHeadPartial, + canonicalUrl ) } // There's no matching prefetch for this route in the cache. @@ -114,21 +111,23 @@ export function navigate( tag: NavigationResultTag.Async, data: navigateDynamicallyWithNoPrefetch( url, + nextUrl, currentCacheNode, - currentFlightRouterState, - nextUrl + currentFlightRouterState ), } } function navigateUsingPrefetchedRouteTree( + url: URL, + nextUrl: string | null, currentCacheNode: CacheNode, currentFlightRouterState: FlightRouterState, prefetchFlightRouterState: FlightRouterState, prefetchSeedData: CacheNodeSeedData | null, prefetchHead: React.ReactNode | null, - canonicalUrl: string, - promiseForDynamicServerResponse: Promise + isPrefetchHeadPartial: boolean, + canonicalUrl: string ): SuccessfulNavigationResult | NoOpNavigationResult { // Recursively construct a prefetch tree by reading from the Segment Cache. To // maintain compatibility, we output the same data structures as the old @@ -141,26 +140,42 @@ function navigateUsingPrefetchedRouteTree( currentFlightRouterState, prefetchFlightRouterState, prefetchSeedData, - prefetchHead + prefetchHead, + isPrefetchHeadPartial ) if (task !== null) { - const newCacheNode = task.node - if (newCacheNode !== null) { + if (task.needsDynamicRequest) { + const promiseForDynamicServerResponse = fetchServerResponse(url, { + flightRouterState: currentFlightRouterState, + nextUrl, + }) listenForDynamicRequest(task, promiseForDynamicServerResponse) + } else { + // The prefetched tree does not contain dynamic holes — it's + // fully static. We can skip the dynamic request. } - return { - tag: NavigationResultTag.Success, - data: { - flightRouterState: task.route, - cacheNode: newCacheNode !== null ? newCacheNode : currentCacheNode, - canonicalUrl, - }, - } + return navigationTaskToResult(task, currentCacheNode, canonicalUrl) } // The server sent back an empty tree patch. There's nothing to update. return noOpNavigationResult } +function navigationTaskToResult( + task: PPRNavigationTask, + currentCacheNode: CacheNode, + canonicalUrl: string +): SuccessfulNavigationResult { + const newCacheNode = task.node + return { + tag: NavigationResultTag.Success, + data: { + flightRouterState: task.route, + cacheNode: newCacheNode !== null ? newCacheNode : currentCacheNode, + canonicalUrl, + }, + } +} + function readRenderSnapshotFromCache( now: number, tree: TreePrefetch @@ -243,9 +258,9 @@ function readRenderSnapshotFromCache( async function navigateDynamicallyWithNoPrefetch( url: URL, + nextUrl: string | null, currentCacheNode: CacheNode, - currentFlightRouterState: FlightRouterState, - nextUrl: string | null + currentFlightRouterState: FlightRouterState ): Promise< MPANavigationResult | SuccessfulNavigationResult | NoOpNavigationResult > { @@ -292,17 +307,32 @@ async function navigateDynamicallyWithNoPrefetch( // nor a prefetch head. const prefetchSeedData = null const prefetchHead = null + const isPrefetchHeadPartial = true + + const canonicalUrl = createCanonicalUrl( + canonicalUrlOverride ? canonicalUrlOverride : url + ) // Now we proceed exactly as we would for normal navigation. - return navigateUsingPrefetchedRouteTree( + const task = updateCacheNodeOnNavigation( currentCacheNode, currentFlightRouterState, prefetchFlightRouterState, prefetchSeedData, prefetchHead, - createCanonicalUrl(canonicalUrlOverride ? canonicalUrlOverride : url), - promiseForDynamicServerResponse + isPrefetchHeadPartial ) + if (task !== null) { + if (task.needsDynamicRequest) { + listenForDynamicRequest(task, promiseForDynamicServerResponse) + } else { + // The prefetched tree does not contain dynamic holes — it's + // fully static. We can skip the dynamic request. + } + return navigationTaskToResult(task, currentCacheNode, canonicalUrl) + } + // The server sent back an empty tree patch. There's nothing to update. + return noOpNavigationResult } function simulatePrefetchTreeUsingDynamicTreePatch( diff --git a/test/e2e/app-dir/segment-cache/basic/app/fully-static/page.tsx b/test/e2e/app-dir/segment-cache/basic/app/fully-static/page.tsx new file mode 100644 index 00000000000000..feb16d95c70315 --- /dev/null +++ b/test/e2e/app-dir/segment-cache/basic/app/fully-static/page.tsx @@ -0,0 +1,18 @@ +import Link from 'next/link' + +export default function FullyStaticStart() { + return ( + <> +

+ Demonstrates that when navigating to a fully prefetched route that does + not contain any dynamic data, we do not need to perform an additional + request on navigation. +

+
    +
  • + Target +
    • +
+ + ) +} diff --git a/test/e2e/app-dir/segment-cache/basic/app/fully-static/target-page/page.tsx b/test/e2e/app-dir/segment-cache/basic/app/fully-static/target-page/page.tsx new file mode 100644 index 00000000000000..dafd08c46fa0be --- /dev/null +++ b/test/e2e/app-dir/segment-cache/basic/app/fully-static/target-page/page.tsx @@ -0,0 +1,3 @@ +export default function Target() { + return
Target
+} 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 d44d1e2f424e67..623a6155f3b64a 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 @@ -104,6 +104,33 @@ describe('segment cache (basic tests)', () => { navigationsLock.release() }) + + it('skips dynamic request if prefetched data is fully static', async () => { + const interceptor = createRequestInterceptor() + const browser = await next.browser('/fully-static', { + beforePageLoad(page: Page) { + 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="/fully-static/target-page"]' + ) + const navigationsLock = interceptor.lockNavigations() + await link.click() + + // The page should render immediately because it was prefetched. + const div = await browser.elementById('target-page') + expect(await div.innerHTML()).toBe('Target') + + // We should have skipped the navigation request because all the data was + // fully static. + const numberOfNavigationRequests = (await navigationsLock.release()).size + expect(numberOfNavigationRequests).toBe(0) + }) }) function createRequestInterceptor() { @@ -131,6 +158,7 @@ function createRequestInterceptor() { for (const route of routes) { route.continue() } + return routes }, } },