From 88fe4598f02c6f8c8cddffea667c778f693685ff Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 20 Dec 2023 00:28:12 -0500 Subject: [PATCH] [wip] Possible parallel routes fix --- .../router-reducer/ppr-navigations.ts | 205 ++++++++++++------ .../reducers/navigate-reducer.ts | 24 +- 2 files changed, 157 insertions(+), 72 deletions(-) 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 cb497ec63da946..05c00715e1cf2b 100644 --- a/packages/next/src/client/components/router-reducer/ppr-navigations.ts +++ b/packages/next/src/client/components/router-reducer/ppr-navigations.ts @@ -7,9 +7,9 @@ import type { import type { CacheNode, ChildSegmentMap, - LazyCacheNode, ReadyCacheNode, } from '../../../shared/lib/app-router-context.shared-runtime' +import { DEFAULT_SEGMENT_KEY } from '../../../shared/lib/segment' import { matchSegment } from '../match-segments' import { createRouterCacheKey } from './create-router-cache-key' import type { FetchServerResponseResult } from './fetch-server-response' @@ -21,8 +21,12 @@ import type { FetchServerResponseResult } from './fetch-server-response' // because those include reused nodes, too. This tree is discarded as soon as // the navigation response is received. type Task = { + // The router state that corresponds to the tree that this Task represents. route: FlightRouterState - node: CacheNode + // 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. + node: CacheNode | null children: Map | null } @@ -82,6 +86,14 @@ export function updateCacheNodeOnNavigation( // leaking memory indefinitely. const prefetchParallelRoutes = new Map(oldParallelRoutes) + // As we diff the trees, we may sometimes modify (copy-on-write, not mutate) + // the Route Tree that was returned by the server — for example, in the case + // of default parallel routes, we preserve the currently active segment. To + // avoid mutating the original tree, we clone the router state children along + // the return path. + let patchedRouterStateChildren: { + [parallelRouteKey: string]: FlightRouterState + } = {} let taskChildren = null for (let parallelRouteKey in newRouterStateChildren) { const newRouterStateChild: FlightRouterState = @@ -101,11 +113,11 @@ export function updateCacheNodeOnNavigation( : undefined let taskChild: Task | null - if ( - oldRouterStateChild !== undefined && - matchSegment(newSegmentChild, oldRouterStateChild[0]) - ) { - if (oldCacheNodeChild !== undefined) { + if (matchSegment(newSegmentChild, oldRouterStateChild[0])) { + if ( + oldCacheNodeChild !== undefined && + oldRouterStateChild !== undefined + ) { // This segment exists in both the old and new trees. if (prefetchDataChild !== undefined && prefetchDataChild !== null) { // Recursively update the children. @@ -121,67 +133,62 @@ export function updateCacheNodeOnNavigation( // shouldn't happen because the Route Tree and the Seed Data tree // should always be the same shape, but until we unify those types // it's still possible. For now we're going to deopt and trigger a - // lazy fetch during render. The main case I'm aware of where this - // happens is default pages. - // TODO: Special case default pages for now? - const missingChild: LazyCacheNode = { - lazyData: null, - rsc: null, - prefetchRsc: null, - head: null, - prefetchHead: null, - parallelRoutes: new Map(), - } - taskChild = { - route: newRouterStateChild, - node: missingChild, - children: null, - } + // lazy fetch during render. + taskChild = spawnTaskForMissingData(newRouterStateChild) } } else { - // This segment exists in both the old and the new Router State Tree, - // but there's no existing Cache Node for it. The server likely won't - // send any data for it. - const missingChild: LazyCacheNode = { - lazyData: null, - rsc: null, - prefetchRsc: null, - head: null, - prefetchHead: null, - parallelRoutes: new Map(), - } - taskChild = { - route: newRouterStateChild, - node: missingChild, - children: null, - } + // Either there's no existing Cache Node for this segment, or this + // segment doesn't exist in the old Router State tree. Switch to the + // "create" path. + taskChild = spawnPendingTask( + newRouterStateChild, + prefetchDataChild !== undefined ? prefetchDataChild : null, + prefetchHead + ) } } else { - // This segment doesn't exist in the current Router State tree. Switch to - // the "create" path. - const pendingCacheNode = createPendingCacheNode( - newRouterStateChild, - prefetchDataChild !== undefined ? prefetchDataChild : null, - prefetchHead - ) - const newTask: Task = { - route: newRouterStateChild, - node: pendingCacheNode, - children: null, + // The segment does not match. + if (newSegmentChild === DEFAULT_SEGMENT_KEY) { + // This is a special case related to default routes. When there's no + // matching segment for a parallel route, Next.js preserves the + // currently active segment during a client navigation — but not for + // initial render. The server leaves it to the client to account for + // this. So we need to handle it here. + // + // 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) + } else { + // This is a new tree. Switch to the "create" path. + taskChild = spawnPendingTask( + newRouterStateChild, + prefetchDataChild !== undefined ? prefetchDataChild : null, + prefetchHead + ) } - taskChild = newTask } if (taskChild !== null) { + // Something changed in the child tree. Keep track of the child task. if (taskChildren === null) { taskChildren = new Map() } taskChildren.set(parallelRouteKey, taskChild) - const newCacheNodeChild = taskChild.node - const newSegmentMapChild: ChildSegmentMap = new Map(oldSegmentMapChild) - newSegmentMapChild.set(newSegmentKeyChild, newCacheNodeChild) - prefetchParallelRoutes.set(parallelRouteKey, newSegmentMapChild) + if (newCacheNodeChild !== null) { + const newSegmentMapChild: ChildSegmentMap = new Map(oldSegmentMapChild) + newSegmentMapChild.set(newSegmentKeyChild, newCacheNodeChild) + prefetchParallelRoutes.set(parallelRouteKey, newSegmentMapChild) + } + + // 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 + } else { + // The child didn't change. We can use the prefetched router state. + patchedRouterStateChildren[parallelRouteKey] = newRouterStateChild } } @@ -208,12 +215,76 @@ export function updateCacheNodeOnNavigation( } return { - route: newRouterState, + // Return a cloned copy of the router state with updated children. + route: patchRouterStateWithNewChildren( + newRouterState, + patchedRouterStateChildren + ), node: newCacheNode, children: taskChildren, } } +function patchRouterStateWithNewChildren( + baseRouterState: FlightRouterState, + newChildren: { [parallelRouteKey: string]: FlightRouterState } +): FlightRouterState { + const clone: FlightRouterState = [baseRouterState[0], newChildren] + // Based on equivalent logic in apply-router-state-patch-to-tree, but should + // confirm whether we need to copy all of these fields. Not sure the server + // ever sends, e.g. the refetch marker. + if (2 in baseRouterState) { + clone[2] = baseRouterState[2] + } + if (3 in baseRouterState) { + clone[3] = baseRouterState[3] + } + if (4 in baseRouterState) { + clone[4] = baseRouterState[4] + } + return clone +} + +function spawnPendingTask( + routerState: FlightRouterState, + prefetchData: CacheNodeSeedData | null, + prefetchHead: React.ReactNode +): Task { + // Create a task that will later be fulfilled by data from the server. + const pendingCacheNode = createPendingCacheNode( + routerState, + prefetchData, + prefetchHead + ) + return { + route: routerState, + node: pendingCacheNode, + children: null, + } +} + +function spawnReusedTask(reusedRouterState: FlightRouterState): 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, + children: null, + } +} + +function spawnTaskForMissingData(routerState: FlightRouterState): Task { + // Create a task for a new subtree that wasn't prefetched by the server. + // This shouldn't really ever happen but it's here just in case the Seed Data + // Tree and the Router State Tree disagree unexpectedly. + const pendingCacheNode = createPendingCacheNode(routerState, null, null) + return { + route: routerState, + node: pendingCacheNode, + children: null, + } +} + export function listenForDynamicRequest( task: Task, responsePromise: Promise @@ -342,16 +413,19 @@ function finishTaskUsingDynamicDataPayload( // 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 + const taskNode = task.node if (taskChildren === null) { // We've reached the leaf node of the pending task. We can now switch to // the normal algorithm. - finishPendingCacheNode( - task.node, - task.route, - serverRouterState, - dynamicData, - dynamicHead - ) + if (taskNode !== null) { + finishPendingCacheNode( + taskNode, + task.route, + serverRouterState, + dynamicData, + dynamicHead + ) + } return } // The server returned more data than we need to finish the task. Skip over @@ -544,7 +618,10 @@ function finishPendingCacheNode( } export function abortTask(task: Task, error: any): void { - abortPendingCacheNode(task.route, task.node, error) + const cacheNode = task.node + if (cacheNode !== null) { + abortPendingCacheNode(task.route, cacheNode, error) + } } 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 11cbd34cd07768..1b6b8121b1beb4 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 @@ -413,27 +413,34 @@ function navigateReducer_PPR( } if ( - // TODO: As far as I can tell, the NEXT_DID_POSTPONE_HEADER header - // is never set, even in production. Need to figure this out before - // landing. To unblock local debugging, I've temporarily disabled - // this check. - // - // _postponed && prefetchEntryCacheStatus !== PrefetchCacheEntryStatus.stale && + // This is just a paranoid check. When PPR is enabled, the server + // will always send back a static response that's rendered from + // the root. If for some reason it doesn't, we fall back to the + // non-PPR implementation. flightDataPath.length === 3 ) { + const prefetchedTree: FlightRouterState = flightDataPath[0] const seedData = flightDataPath[1] const head = flightDataPath[2] const task = updateCacheNodeOnNavigation( currentCache, currentTree, - newTree, + prefetchedTree, seedData, head ) - if (task !== null) { + if (task !== null && task.node !== 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 applyRouterStatePatchToTreeSkipDefault. + // TODO: We should remove applyRouterStatePatchToTreeSkipDefault + // from the PPR path entirely. + const patchedRouterState: FlightRouterState = task.route + newTree = patchedRouterState + const newCache = task.node // The prefetched tree has dynamic holes in it. We initiate a @@ -465,6 +472,7 @@ function navigateReducer_PPR( // TODO: What if the head changed but not any of the segment data? // Is that possible? If so, we should clone the whole tree and // update the head. + newTree = prefetchedTree } } else { // The static response does not include any dynamic holes, so