From dfbcfd30531cb3b6e2f0bba0e1443c3eab0ba6d5 Mon Sep 17 00:00:00 2001 From: Zack Tanner <1939140+ztanner@users.noreply.github.com> Date: Wed, 3 Jul 2024 16:12:28 -0700 Subject: [PATCH] move layerAssets into CacheNodeSeedData --- .../src/client/components/layout-router.tsx | 3 - .../next/src/server/app-render/app-render.tsx | 47 +++---- .../app-render/create-component-tree.tsx | 131 +++++++++--------- .../walk-tree-with-flight-router-state.tsx | 23 +-- .../acceptance-app/hydration-error.test.ts | 27 +++- .../parallel-routes-css.test.ts | 3 +- 6 files changed, 110 insertions(+), 124 deletions(-) diff --git a/packages/next/src/client/components/layout-router.tsx b/packages/next/src/client/components/layout-router.tsx index 0be0401f016c1..c82673c2fc3c3 100644 --- a/packages/next/src/client/components/layout-router.tsx +++ b/packages/next/src/client/components/layout-router.tsx @@ -497,7 +497,6 @@ export default function OuterLayoutRouter({ template, notFound, notFoundStyles, - styles, }: { parallelRouterKey: string segmentPath: FlightSegmentPath @@ -509,7 +508,6 @@ export default function OuterLayoutRouter({ template: React.ReactNode notFound: React.ReactNode | undefined notFoundStyles: React.ReactNode | undefined - styles?: React.ReactNode }) { const context = useContext(LayoutRouterContext) if (!context) { @@ -542,7 +540,6 @@ export default function OuterLayoutRouter({ return ( <> - {styles} {preservedSegments.map((preservedSegment) => { const preservedSegmentValue = getSegmentValue(preservedSegment) const cacheKey = createRouterCacheKey(preservedSegment) diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 1ea98441f40a2..88c15d8a517af 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -475,7 +475,7 @@ async function ReactServerApp({ tree, ctx, asNotFound }: ReactServerAppProps) { createDynamicallyTrackedSearchParams, }) - const { seedData, styles } = await createComponentTree({ + const seedData = await createComponentTree({ ctx, createSegmentPath: (child) => child, loaderTree: tree, @@ -498,30 +498,27 @@ async function ReactServerApp({ tree, ctx, asNotFound }: ReactServerAppProps) { typeof varyHeader === 'string' && varyHeader.includes(NEXT_URL) return ( - <> - {styles} - - - {/* Adding requestId as react key to make metadata remount for each render */} - - - } - globalErrorComponent={GlobalError} - // This is used to provide debug information (when in development mode) - // about which slots were not filled by page components while creating the component tree. - missingSlots={missingSlots} - /> - + + + {/* Adding requestId as react key to make metadata remount for each render */} + + + } + globalErrorComponent={GlobalError} + // This is used to provide debug information (when in development mode) + // about which slots were not filled by page components while creating the component tree. + missingSlots={missingSlots} + /> ) } 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 19d5513b4215f..384a3bea68c9b 100644 --- a/packages/next/src/server/app-render/create-component-tree.tsx +++ b/packages/next/src/server/app-render/create-component-tree.tsx @@ -1,5 +1,5 @@ import type { FlightSegmentPath, CacheNodeSeedData } from './types' -import React, { type ReactNode } from 'react' +import React from 'react' import { isClientReference } from '../../lib/client-reference' import { getLayoutOrPageModule } from '../lib/app-dir-module' import type { LoaderTree } from '../lib/app-dir-module' @@ -16,11 +16,6 @@ import { NextNodeServerSpan } from '../lib/trace/constants' import { StaticGenBailoutError } from '../../client/components/static-generation-bailout' import type { LoadingModuleData } from '../../shared/lib/app-router-context.shared-runtime' -type ComponentTree = { - seedData: CacheNodeSeedData - styles: ReactNode -} - type Params = { [key: string]: string | string[] } @@ -41,7 +36,7 @@ export function createComponentTree(props: { metadataOutlet?: React.ReactNode ctx: AppRenderContext missingSlots?: Set -}): Promise { +}): Promise { return getTracer().trace( NextNodeServerSpan.createComponentTree, { @@ -83,7 +78,7 @@ async function createComponentTreeInternal({ metadataOutlet?: React.ReactNode ctx: AppRenderContext missingSlots?: Set -}): Promise { +}): Promise { const { renderOpts: { nextConfigOutput, experimental }, staticGenerationStore, @@ -374,7 +369,6 @@ async function createComponentTreeInternal({ // 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 childCacheNodeSeedData: CacheNodeSeedData | null = null if ( @@ -426,27 +420,25 @@ async function createComponentTreeInternal({ } } - const { seedData, styles: childComponentStyles } = - await createComponentTreeInternal({ - createSegmentPath: (child) => { - return createSegmentPath([...currentSegmentPath, ...child]) - }, - loaderTree: parallelRoute, - parentParams: currentParams, - rootLayoutIncluded: rootLayoutIncludedAtThisLevelOrAbove, - injectedCSS: injectedCSSWithCurrentLayout, - injectedJS: injectedJSWithCurrentLayout, - injectedFontPreloadTags: injectedFontPreloadTagsWithCurrentLayout, - asNotFound, - // The metadataOutlet is responsible for throwing any errors that were caught during metadata resolution. - // We only want to render an outlet once per segment, as otherwise the error will be triggered - // multiple times causing an uncaught error. - metadataOutlet: isChildrenRouteKey ? metadataOutlet : undefined, - ctx, - missingSlots, - }) - - currentStyles = childComponentStyles + const seedData = await createComponentTreeInternal({ + createSegmentPath: (child) => { + return createSegmentPath([...currentSegmentPath, ...child]) + }, + loaderTree: parallelRoute, + parentParams: currentParams, + rootLayoutIncluded: rootLayoutIncludedAtThisLevelOrAbove, + injectedCSS: injectedCSSWithCurrentLayout, + injectedJS: injectedJSWithCurrentLayout, + injectedFontPreloadTags: injectedFontPreloadTagsWithCurrentLayout, + asNotFound, + // The metadataOutlet is responsible for throwing any errors that were caught during metadata resolution. + // We only want to render an outlet once per segment, as otherwise the error will be triggered + // multiple times causing an uncaught error. + metadataOutlet: isChildrenRouteKey ? metadataOutlet : undefined, + ctx, + missingSlots, + }) + childCacheNodeSeedData = seedData } @@ -469,7 +461,6 @@ async function createComponentTreeInternal({ templateScripts={templateScripts} notFound={notFoundComponent} notFoundStyles={notFoundStyles} - styles={currentStyles} />, childCacheNodeSeedData, ] @@ -494,20 +485,20 @@ async function createComponentTreeInternal({ // When the segment does not have a layout or page we still have to add the layout router to ensure the path holds the loading component if (!Component) { - return { - seedData: [ - actualSegment, - parallelRouteCacheNodeSeedData, - // TODO: I don't think the extra fragment is necessary. React treats top - // level fragments as transparent, i.e. the runtime behavior should be - // identical even without it. But maybe there's some findDOMNode-related - // reason that I'm not aware of, so I'm leaving it as-is out of extreme - // caution, for now. - <>{parallelRouteProps.children}, - loadingData, - ], - styles: layerAssets, - } + return [ + actualSegment, + parallelRouteCacheNodeSeedData, + // TODO: I don't think the extra fragment is necessary. React treats top + // level fragments as transparent, i.e. the runtime behavior should be + // identical even without it. But maybe there's some findDOMNode-related + // reason that I'm not aware of, so I'm leaving it as-is out of extreme + // caution, for now. + <> + {layerAssets} + {parallelRouteProps.children} + , + loadingData, + ] } // If force-dynamic is used and the current render supports postponing, we @@ -525,19 +516,19 @@ async function createComponentTreeInternal({ staticGenerationStore.forceDynamic && staticGenerationStore.prerenderState ) { - return { - seedData: [ - actualSegment, - parallelRouteCacheNodeSeedData, + return [ + actualSegment, + parallelRouteCacheNodeSeedData, + <> , - loadingData, - ], - styles: layerAssets, - } + /> + {layerAssets} + , + loadingData, + ] } const isClientComponent = isClientReference(layoutOrPageMod) @@ -592,6 +583,7 @@ async function createComponentTreeInternal({ <> {metadataOutlet} + {layerAssets} ) } else { @@ -602,21 +594,26 @@ async function createComponentTreeInternal({ <> {metadataOutlet} + {layerAssets} ) } } else { // For layouts we just render the component - segmentElement = + segmentElement = ( + <> + {layerAssets} + + + ) } - return { - seedData: [ - actualSegment, - parallelRouteCacheNodeSeedData, - <> - {segmentElement} - {/* This null is currently critical. The wrapped Component can render null and if there was not fragment + return [ + actualSegment, + parallelRouteCacheNodeSeedData, + <> + {segmentElement} + {/* This null is currently critical. The wrapped Component can render null and if there was not fragment surrounding it this would look like a pending tree data state on the client which will cause an error and break the app. Long-term we need to move away from using null as a partial tree identifier since it is a valid return type for the components we wrap. Once we make this change we can safely remove the @@ -624,10 +621,8 @@ async function createComponentTreeInternal({ If the Component above renders null the actual tree data will look like `[null, null]`. If we remove the extra null it will look like `null` (the array is elided) and this is what confuses the client router. TODO-APP update router to use a Symbol for partial tree detection */} - {null} - , - loadingData, - ], - styles: layerAssets, - } + {null} + , + loadingData, + ] } diff --git a/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx b/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx index 88251aa1086fb..41fd089ee2869 100644 --- a/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx +++ b/packages/next/src/server/app-render/walk-tree-with-flight-router-state.tsx @@ -4,7 +4,6 @@ import type { FlightSegmentPath, Segment, } from './types' -import React from 'react' import { canSegmentBeOverridden, matchSegment, @@ -16,9 +15,7 @@ import { addSearchParamsIfPageSegment, createFlightRouterStateFromLoaderTree, } from './create-flight-router-state-from-loader-tree' -import { parseLoaderTree } from './parse-loader-tree' import type { CreateSegmentPath, AppRenderContext } from './app-render' -import { getLayerAssets } from './get-layer-assets' import { hasLoadingComponentInTree } from './has-loading-component-in-tree' import { createComponentTree } from './create-component-tree' import { DEFAULT_SEGMENT_KEY } from '../../shared/lib/segment' @@ -140,7 +137,7 @@ export async function walkTreeWithFlightRouterState({ return [[overriddenSegment, routerState, null, null]] } else { // Create component tree using the slice of the loaderTree - const { seedData } = await createComponentTree( + const seedData = await createComponentTree( // This ensures flightRouterPath is valid and filters down the tree { ctx, @@ -158,23 +155,7 @@ export async function walkTreeWithFlightRouterState({ } ) - // Create head - const { layoutOrPagePath } = parseLoaderTree(loaderTreeToFilter) - const layerAssets = getLayerAssets({ - ctx, - layoutOrPagePath, - injectedCSS: new Set(injectedCSS), - injectedJS: new Set(injectedJS), - injectedFontPreloadTags: new Set(injectedFontPreloadTags), - }) - const head = ( - <> - {layerAssets} - {rscPayloadHead} - - ) - - return [[overriddenSegment, routerState, seedData, head]] + return [[overriddenSegment, routerState, seedData, rscPayloadHead]] } } diff --git a/test/development/acceptance-app/hydration-error.test.ts b/test/development/acceptance-app/hydration-error.test.ts index 942e3e9eb4be4..824b1f54ae0f2 100644 --- a/test/development/acceptance-app/hydration-error.test.ts +++ b/test/development/acceptance-app/hydration-error.test.ts @@ -59,6 +59,7 @@ describe('Error overlay for hydration errors', () => { if (isTurbopack) { expect(pseudoHtml).toMatchInlineSnapshot(` "... + ... + client - server" `) @@ -123,6 +124,7 @@ describe('Error overlay for hydration errors', () => { if (isTurbopack) { expect(pseudoHtml).toMatchInlineSnapshot(` "... + ... +
" `) } else { @@ -174,6 +176,7 @@ describe('Error overlay for hydration errors', () => { expect(pseudoHtml).toEqual(outdent` ... ... + ... + second -