Inline ChildProp (#58519)

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>

Author: acdlite

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 `<Offscreen>` 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={

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 ? <NotFound /> : undefined
 
-        function getParallelRoutePair(
-          currentChildProp: ChildProp,
-          currentStyles: React.ReactNode
-        ): [string, React.ReactNode] {
-          // This is turned back into an object below.
-          return [
-            parallelRouteKey,
-            <LayoutRouter
-              parallelRouterKey={parallelRouteKey}
-              segmentPath={createSegmentPath(currentSegmentPath)}
-              loading={Loading ? <Loading /> : 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={
-                <Template>
-                  <RenderFromTemplateContext />
-                </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 = <ChildComponent />
-        }
-
-        const childProp: ChildProp = {
-          current: childElement,
-          segment: childPropSegment,
+          initialChildNode = <ChildComponent />
         }
 
-        return getParallelRoutePair(childProp, currentStyles)
+        // This is turned back into an object below.
+        return [
+          parallelRouteKey,
+          <LayoutRouter
+            parallelRouterKey={parallelRouteKey}
+            segmentPath={createSegmentPath(currentSegmentPath)}
+            loading={Loading ? <Loading /> : 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={
+              <Template>
+                <RenderFromTemplateContext />
+              </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}
+          />,
+        ]
       }
     )
   )

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