Skip to content

Commit

Permalink
Inline ChildProp (#58519)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
acdlite and kodiakhq[bot] authored Nov 16, 2023
1 parent c26c771 commit b017261
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 69 deletions.
39 changes: 23 additions & 16 deletions packages/next/src/client/components/layout-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -315,7 +314,7 @@ function InnerLayoutRouter({
parallelRouterKey,
url,
childNodes,
childProp,
initialChildNode,
segmentPath,
tree,
// TODO-APP: implement `<Offscreen>` when available.
Expand All @@ -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
Expand All @@ -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(),
}

Expand All @@ -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
}
}
}
Expand Down Expand Up @@ -498,7 +503,8 @@ function LoadingBoundary({
export default function OuterLayoutRouter({
parallelRouterKey,
segmentPath,
childProp,
initialChildNode,
childPropSegment,
error,
errorStyles,
errorScripts,
Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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={
Expand Down
76 changes: 34 additions & 42 deletions packages/next/src/server/app-render/create-component-tree.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
/>,
]
}
)
)
Expand Down
11 changes: 0 additions & 11 deletions packages/next/src/server/app-render/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b017261

Please sign in to comment.