From b0172610479c55180546dff6f5bf92f9021d7b9e Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 16 Nov 2023 06:01:09 -0500 Subject: [PATCH] Inline ChildProp (#58519) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I'm working on a refactor to seed the CacheNodes as soon as the Flight payload is received, rather than lazily during the render phase. This means we no longer need to pass a child element prop to LayoutRouter via childProp. ChildProp includes two fields: a segment and a child element. The child element is the part that will soon be removed, because we'll instead always read from the cache nodes. But even after this refactor, we still need to pass the segment to LayoutRouter. So as an incremental step, I've inlined both fields into separate props: - childProp.current -> initialChildNode. This will be removed in a later step in favor of reading from the cache nodes. In fact, we already always read from the cache nodes — childProp is ignored completely once the cache node is populated, hence the updated name. - childProp.segment -> childPropSegment. This probably isn't the best name anymore but I'll leave renaming until later once more of this refactor has settled. --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .../src/client/components/layout-router.tsx | 39 ++++++---- .../app-render/create-component-tree.tsx | 76 +++++++++---------- packages/next/src/server/app-render/types.ts | 11 --- 3 files changed, 57 insertions(+), 69 deletions(-) diff --git a/packages/next/src/client/components/layout-router.tsx b/packages/next/src/client/components/layout-router.tsx index f679a21b32b09..41e067aeb38f6 100644 --- a/packages/next/src/client/components/layout-router.tsx +++ b/packages/next/src/client/components/layout-router.tsx @@ -4,7 +4,6 @@ import type { ChildSegmentMap } from '../../shared/lib/app-router-context.shared import type { FlightRouterState, FlightSegmentPath, - ChildProp, Segment, } from '../../server/app-render/types' import type { ErrorComponent } from './error-boundary' @@ -315,7 +314,7 @@ function InnerLayoutRouter({ parallelRouterKey, url, childNodes, - childProp, + initialChildNode, segmentPath, tree, // TODO-APP: implement `` when available. @@ -325,7 +324,7 @@ function InnerLayoutRouter({ parallelRouterKey: string url: string childNodes: ChildSegmentMap - childProp: ChildProp | null + initialChildNode: React.ReactNode | null segmentPath: FlightSegmentPath tree: FlightRouterState isActive: boolean @@ -341,19 +340,25 @@ function InnerLayoutRouter({ // Read segment path from the parallel router cache node. let childNode = childNodes.get(cacheKey) - // If childProp is available this means it's the Flight / SSR case. - if ( - childProp && - // TODO-APP: verify if this can be null based on user code - childProp.current !== null - ) { + // If initialChildNode is available this means it's the Flight / SSR case. + // TODO: `null` is a valid React Node, so technically we should use some other + // value besides `null` to indicate that the tree is partial. However, we're + // about to remove all the cases that lead to a partial tree, so this soon + // won't be an issue. + if (initialChildNode !== null) { if (!childNode) { // Add the segment's subTreeData to the cache. // This writes to the cache when there is no item in the cache yet. It never *overwrites* existing cache items which is why it's safe in concurrent mode. + + // TODO: We should seed all the CacheNodes as soon as the Flight payload + // is received. We already collect them eagerly on the server, so we + // shouldn't need to wait until the render phase to write them into + // the cache. Requires refactoring the Flight response type. Then we can + // delete this code. childNode = { status: CacheStates.READY, data: null, - subTreeData: childProp.current, + subTreeData: initialChildNode, parallelRoutes: new Map(), } @@ -363,7 +368,7 @@ function InnerLayoutRouter({ // @ts-expect-error we're changing it's type! childNode.status = CacheStates.READY // @ts-expect-error - childNode.subTreeData = childProp.current + childNode.subTreeData = initialChildNode } } } @@ -498,7 +503,8 @@ function LoadingBoundary({ export default function OuterLayoutRouter({ parallelRouterKey, segmentPath, - childProp, + initialChildNode, + childPropSegment, error, errorStyles, errorScripts, @@ -515,7 +521,8 @@ export default function OuterLayoutRouter({ }: { parallelRouterKey: string segmentPath: FlightSegmentPath - childProp: ChildProp + initialChildNode: React.ReactNode | null + childPropSegment: Segment error: ErrorComponent errorStyles: React.ReactNode | undefined errorScripts: React.ReactNode | undefined @@ -550,8 +557,6 @@ export default function OuterLayoutRouter({ // The reason arrays are used in the data format is that these are transferred from the server to the browser so it's optimized to save bytes. const treeSegment = tree[1][parallelRouterKey][0] - const childPropSegment = childProp.segment - // If segment is an array it's a dynamic route and we want to read the dynamic route value as the segment to get from the cache. const currentChildSegmentValue = getSegmentValue(treeSegment) @@ -607,7 +612,9 @@ export default function OuterLayoutRouter({ url={url} tree={tree} childNodes={childNodesForParallelRouter!} - childProp={isChildPropSegment ? childProp : null} + initialChildNode={ + isChildPropSegment ? initialChildNode : null + } segmentPath={segmentPath} cacheKey={cacheKey} isActive={ 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 72948a563846b..ef49043d58c85 100644 --- a/packages/next/src/server/app-render/create-component-tree.tsx +++ b/packages/next/src/server/app-render/create-component-tree.tsx @@ -1,4 +1,4 @@ -import type { ChildProp, FlightSegmentPath } from './types' +import type { FlightSegmentPath } from './types' import React from 'react' import { isClientReference } from '../../lib/client-reference' import { getLayoutOrPageModule } from '../lib/app-dir-module' @@ -301,44 +301,11 @@ export async function createComponentTree({ const notFoundComponent = NotFound && isChildrenRouteKey ? : undefined - function getParallelRoutePair( - currentChildProp: ChildProp, - currentStyles: React.ReactNode - ): [string, React.ReactNode] { - // This is turned back into an object below. - return [ - parallelRouteKey, - : undefined} - loadingStyles={loadingStyles} - loadingScripts={loadingScripts} - // TODO-APP: Add test for loading returning `undefined`. This currently can't be tested as the `webdriver()` tab will wait for the full page to load before returning. - hasLoading={Boolean(Loading)} - error={ErrorComponent} - errorStyles={errorStyles} - errorScripts={errorScripts} - template={ - - } - templateStyles={templateStyles} - templateScripts={templateScripts} - notFound={notFoundComponent} - notFoundStyles={notFoundStyles} - childProp={currentChildProp} - styles={currentStyles} - />, - ] - } - // if we're prefetching and that there's a Loading component, we bail out // otherwise we keep rendering for the prefetch. // We also want to bail out if there's no Loading component in the tree. let currentStyles = undefined - let childElement = null + let initialChildNode = null const childPropSegment = addSearchParamsIfPageSegment( childSegmentParam ? childSegmentParam.treeSegment : childSegment, query @@ -367,15 +334,40 @@ export async function createComponentTree({ }) currentStyles = childComponentStyles - childElement = - } - - const childProp: ChildProp = { - current: childElement, - segment: childPropSegment, + initialChildNode = } - return getParallelRoutePair(childProp, currentStyles) + // This is turned back into an object below. + return [ + parallelRouteKey, + : undefined} + loadingStyles={loadingStyles} + loadingScripts={loadingScripts} + // TODO-APP: Add test for loading returning `undefined`. This currently can't be tested as the `webdriver()` tab will wait for the full page to load before returning. + hasLoading={Boolean(Loading)} + error={ErrorComponent} + errorStyles={errorStyles} + errorScripts={errorScripts} + template={ + + } + templateStyles={templateStyles} + templateScripts={templateScripts} + notFound={notFoundComponent} + notFoundStyles={notFoundStyles} + // TODO: This prop will soon by removed and instead we'll return all + // the child nodes in the entire tree at the top level of the + // Flight response. + initialChildNode={initialChildNode} + childPropSegment={childPropSegment} + styles={currentStyles} + />, + ] } ) ) diff --git a/packages/next/src/server/app-render/types.ts b/packages/next/src/server/app-render/types.ts index 8ec631cbf33b7..1e09664c17cc6 100644 --- a/packages/next/src/server/app-render/types.ts +++ b/packages/next/src/server/app-render/types.ts @@ -91,17 +91,6 @@ export type ActionFlightResponse = // This case happens when `redirect()` is called in a server action. | NextFlightResponse -/** - * Property holding the current subTreeData. - */ -export type ChildProp = { - /** - * Null indicates that the tree is partial - */ - current: React.ReactNode | null - segment: Segment -} - export interface RenderOptsPartial { err?: Error | null dev?: boolean