From e443eb19052aba2e943fc63657c5b8b846048452 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Mon, 27 Nov 2023 13:34:52 -0800 Subject: [PATCH 1/3] Refactor change to convey boundary state is not specific to resources Hoistable elements and resources in fallbacks should not flush unless the fallback itself flushes. Previously we used a renderState-local binding to track the resources for the currently rendering boundary. Instead we now track this in the task itself and pass it to the functions that depend on it. This cleans up an unfortunate factoring that I put in before where during flushing we had to mimic the currently rendering Boundary to hoist correctly. This new factoring does the same thing but in a much clearer way. I do track the Boundary state separately from the Boundary itself on the task as a hot path optimization to avoid having to existence check the boundary in `pushStartInstance`. Conceptually tracking the boundary itself is sufficient but this eliminates an extra condition check. The implementation ended up not merging the boundary resource concept with hoistable state due to the very different nature in which these things need to hoist (one during task completion and the other during flush but only when flushing late boundaries). Given that I've gone back to resource specific naming rather than calling it BoundaryState. --- .../src/server/ReactFizzConfigDOM.js | 146 ++++----- .../src/server/ReactFizzConfigDOMLegacy.js | 21 +- .../src/__tests__/ReactDOMFloat-test.js | 285 +++++++++++++----- .../src/ReactNoopServer.js | 10 +- packages/react-server/src/ReactFizzServer.js | 270 ++++++++++++----- .../src/forks/ReactFizzConfig.custom.js | 8 +- 6 files changed, 491 insertions(+), 249 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 0788f216e5f30..067f668262be4 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -149,11 +149,7 @@ export type RenderState = { bootstrapChunks: Array, // Hoistable chunks - charsetChunks: Array, - preconnectChunks: Array, importMapChunks: Array, - preloadChunks: Array, - hoistableChunks: Array, // Headers queues for Resources that can flush early onHeaders: void | ((headers: HeadersDescriptor) => void), @@ -201,9 +197,6 @@ export type RenderState = { moduleScripts: Map, }, - // Module-global-like reference for current boundary resources - boundaryResources: ?BoundaryResources, - // Module-global-like reference for flushing/hoisting state of style resources // We need to track whether the current request has flushed any style resources // without sending an instruction to hoist them. we do that here @@ -457,6 +450,7 @@ export function createRenderState( externalRuntimeScript: externalRuntimeScript, bootstrapChunks: bootstrapChunks, + importMapChunks, onHeaders, headers, @@ -472,12 +466,6 @@ export function createRenderState( style: {}, }, - charsetChunks: [], - preconnectChunks: [], - importMapChunks, - preloadChunks: [], - hoistableChunks: [], - // cleared on flush preconnects: new Set(), fontPreloads: new Set(), @@ -497,7 +485,6 @@ export function createRenderState( nonce, // like a module global for currently rendering boundary - boundaryResources: null, stylesToHoist: false, }; @@ -2226,7 +2213,7 @@ function pushStartTextArea( function pushMeta( target: Array, props: Object, - renderState: RenderState, + hoistableState: HoistableState, textEmbedded: boolean, insertionMode: InsertionMode, noscriptTagInScope: boolean, @@ -2246,12 +2233,12 @@ function pushMeta( } if (typeof props.charSet === 'string') { - return pushSelfClosing(renderState.charsetChunks, props, 'meta'); + return pushSelfClosing(hoistableState.charset, props, 'meta'); } else if (props.name === 'viewport') { // "viewport" isn't related to preconnect but it has the right priority - return pushSelfClosing(renderState.preconnectChunks, props, 'meta'); + return pushSelfClosing(hoistableState.viewport, props, 'meta'); } else { - return pushSelfClosing(renderState.hoistableChunks, props, 'meta'); + return pushSelfClosing(hoistableState.chunks, props, 'meta'); } } } else { @@ -2264,6 +2251,8 @@ function pushLink( props: Object, resumableState: ResumableState, renderState: RenderState, + boundaryResources: null | BoundaryResources, + hoistableState: HoistableState, textEmbedded: boolean, insertionMode: InsertionMode, noscriptTagInScope: boolean, @@ -2384,8 +2373,8 @@ function pushLink( // We add the newly created resource to our StyleQueue and if necessary // track the resource with the currently rendering boundary styleQueue.sheets.set(key, resource); - if (renderState.boundaryResources) { - renderState.boundaryResources.stylesheets.add(resource); + if (boundaryResources) { + boundaryResources.stylesheets.add(resource); } } else { // We need to track whether this boundary should wait on this resource or not. @@ -2396,8 +2385,8 @@ function pushLink( if (styleQueue) { const resource = styleQueue.sheets.get(key); if (resource) { - if (renderState.boundaryResources) { - renderState.boundaryResources.stylesheets.add(resource); + if (boundaryResources) { + boundaryResources.stylesheets.add(resource); } } } @@ -2422,15 +2411,7 @@ function pushLink( target.push(textSeparator); } - switch (props.rel) { - case 'preconnect': - case 'dns-prefetch': - return pushLinkImpl(renderState.preconnectChunks, props); - case 'preload': - return pushLinkImpl(renderState.preloadChunks, props); - default: - return pushLinkImpl(renderState.hoistableChunks, props); - } + return pushLinkImpl(hoistableState.chunks, props); } } else { return pushLinkImpl(target, props); @@ -2472,6 +2453,7 @@ function pushStyle( props: Object, resumableState: ResumableState, renderState: RenderState, + boundaryResources: null | BoundaryResources, textEmbedded: boolean, insertionMode: InsertionMode, noscriptTagInScope: boolean, @@ -2571,8 +2553,8 @@ function pushStyle( // it. However, it's possible when you resume that the style has already been emitted // and then it wouldn't be recreated in the RenderState and there's no need to track // it again since we should've hoisted it to the shell already. - if (renderState.boundaryResources) { - renderState.boundaryResources.styles.add(styleQueue); + if (boundaryResources) { + boundaryResources.styles.add(styleQueue); } } @@ -2882,7 +2864,7 @@ function pushStartMenuItem( function pushTitle( target: Array, props: Object, - renderState: RenderState, + hoistableState: HoistableState, insertionMode: InsertionMode, noscriptTagInScope: boolean, ): ReactNodeList { @@ -2940,7 +2922,7 @@ function pushTitle( !noscriptTagInScope && props.itemProp == null ) { - pushTitleImpl(renderState.hoistableChunks, props); + pushTitleImpl(hoistableState.chunks, props); return null; } else { return pushTitleImpl(target, props); @@ -3472,6 +3454,8 @@ export function pushStartInstance( props: Object, resumableState: ResumableState, renderState: RenderState, + boundaryResources: null | BoundaryResources, + hoistableState: HoistableState, formatContext: FormatContext, textEmbedded: boolean, ): ReactNodeList { @@ -3539,7 +3523,7 @@ export function pushStartInstance( ? pushTitle( target, props, - renderState, + hoistableState, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), ) @@ -3550,6 +3534,8 @@ export function pushStartInstance( props, resumableState, renderState, + boundaryResources, + hoistableState, textEmbedded, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), @@ -3572,6 +3558,7 @@ export function pushStartInstance( props, resumableState, renderState, + boundaryResources, textEmbedded, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), @@ -3580,7 +3567,7 @@ export function pushStartInstance( return pushMeta( target, props, - renderState, + hoistableState, textEmbedded, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), @@ -4574,6 +4561,7 @@ export function writePreamble( destination: Destination, resumableState: ResumableState, renderState: RenderState, + hoistableState: HoistableState, willFlushAllSegments: boolean, ): void { // This function must be called exactly once on every request @@ -4619,7 +4607,7 @@ export function writePreamble( } // Emit high priority Hoistables - const charsetChunks = renderState.charsetChunks; + const charsetChunks = hoistableState.charset; for (i = 0; i < charsetChunks.length; i++) { writeChunk(destination, charsetChunks[i]); } @@ -4629,11 +4617,11 @@ export function writePreamble( renderState.preconnects.forEach(flushResource, destination); renderState.preconnects.clear(); - const preconnectChunks = renderState.preconnectChunks; - for (i = 0; i < preconnectChunks.length; i++) { - writeChunk(destination, preconnectChunks[i]); + const viewportChunks = hoistableState.viewport; + for (i = 0; i < viewportChunks.length; i++) { + writeChunk(destination, viewportChunks[i]); } - preconnectChunks.length = 0; + viewportChunks.length = 0; renderState.fontPreloads.forEach(flushResource, destination); renderState.fontPreloads.clear(); @@ -4658,15 +4646,8 @@ export function writePreamble( renderState.bulkPreloads.forEach(flushResource, destination); renderState.bulkPreloads.clear(); - // Write embedding preloadChunks - const preloadChunks = renderState.preloadChunks; - for (i = 0; i < preloadChunks.length; i++) { - writeChunk(destination, preloadChunks[i]); - } - preloadChunks.length = 0; - - // Write embedding hoistableChunks - const hoistableChunks = renderState.hoistableChunks; + // Write hoistableState chunks + const hoistableChunks = hoistableState.chunks; for (i = 0; i < hoistableChunks.length; i++) { writeChunk(destination, hoistableChunks[i]); } @@ -4691,6 +4672,7 @@ export function writeHoistables( destination: Destination, resumableState: ResumableState, renderState: RenderState, + hoistableState: HoistableState, ): void { let i = 0; @@ -4702,12 +4684,6 @@ export function writeHoistables( renderState.preconnects.forEach(flushResource, destination); renderState.preconnects.clear(); - const preconnectChunks = renderState.preconnectChunks; - for (i = 0; i < preconnectChunks.length; i++) { - writeChunk(destination, preconnectChunks[i]); - } - preconnectChunks.length = 0; - renderState.fontPreloads.forEach(flushResource, destination); renderState.fontPreloads.clear(); @@ -4732,15 +4708,8 @@ export function writeHoistables( renderState.bulkPreloads.forEach(flushResource, destination); renderState.bulkPreloads.clear(); - // Write embedding preloadChunks - const preloadChunks = renderState.preloadChunks; - for (i = 0; i < preloadChunks.length; i++) { - writeChunk(destination, preloadChunks[i]); - } - preloadChunks.length = 0; - - // Write embedding hoistableChunks - const hoistableChunks = renderState.hoistableChunks; + // Write hoistableState chunks + const hoistableChunks = hoistableState.chunks; for (i = 0; i < hoistableChunks.length; i++) { writeChunk(destination, hoistableChunks[i]); } @@ -5214,7 +5183,22 @@ type StylesheetResource = { state: StylesheetState, }; +export type HoistableState = { + charset: Array, + viewport: Array, + chunks: Array, +}; + +export function createHoistableState(): HoistableState { + return { + charset: [], + viewport: [], + chunks: [], + }; +} + export type BoundaryResources = { + // style dependencies styles: Set, stylesheets: Set, }; @@ -5233,13 +5217,6 @@ export function createBoundaryResources(): BoundaryResources { }; } -export function setCurrentlyRenderingBoundaryResourcesTarget( - renderState: RenderState, - boundaryResources: null | BoundaryResources, -) { - renderState.boundaryResources = boundaryResources; -} - function getResourceKey(href: string): string { return href; } @@ -6086,6 +6063,15 @@ function escapeStringForLinkHeaderQuotedParamValueContextReplacer( } } +export function hoistHoistables( + target: HoistableState, + source: HoistableState, +) { + target.charset.push(...source.charset); + target.viewport.push(...source.viewport); + target.chunks.push(...source.chunks); +} + function hoistStyleQueueDependency( this: BoundaryResources, styleQueue: StyleQueue, @@ -6100,18 +6086,12 @@ function hoistStylesheetDependency( this.stylesheets.add(stylesheet); } -export function hoistResources( - renderState: RenderState, +export function hoistBoundaryResources( + target: BoundaryResources, source: BoundaryResources, ): void { - const currentBoundaryResources = renderState.boundaryResources; - if (currentBoundaryResources) { - source.styles.forEach(hoistStyleQueueDependency, currentBoundaryResources); - source.stylesheets.forEach( - hoistStylesheetDependency, - currentBoundaryResources, - ); - } + source.styles.forEach(hoistStyleQueueDependency, target); + source.stylesheets.forEach(hoistStylesheetDependency, target); } // This function is called at various times depending on whether we are rendering diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js index 286b8895ffc43..fb0fef7b3cc14 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js @@ -10,7 +10,6 @@ import type { RenderState as BaseRenderState, ResumableState, - BoundaryResources, StyleQueue, Resource, HeadersDescriptor, @@ -48,6 +47,7 @@ export type RenderState = { headChunks: null | Array, externalRuntimeScript: null | any, bootstrapChunks: Array, + importMapChunks: Array, onHeaders: void | ((headers: HeadersDescriptor) => void), headers: null | { preconnects: string, @@ -56,11 +56,6 @@ export type RenderState = { remainingCapacity: number, }, resets: BaseRenderState['resets'], - charsetChunks: Array, - preconnectChunks: Array, - importMapChunks: Array, - preloadChunks: Array, - hoistableChunks: Array, preconnects: Set, fontPreloads: Set, highImagePreloads: Set, @@ -75,7 +70,6 @@ export type RenderState = { scripts: Map, moduleScripts: Map, }, - boundaryResources: ?BoundaryResources, stylesToHoist: boolean, // This is an extra field for the legacy renderer generateStaticMarkup: boolean, @@ -103,14 +97,10 @@ export function createRenderState( headChunks: renderState.headChunks, externalRuntimeScript: renderState.externalRuntimeScript, bootstrapChunks: renderState.bootstrapChunks, + importMapChunks: renderState.importMapChunks, onHeaders: renderState.onHeaders, headers: renderState.headers, resets: renderState.resets, - charsetChunks: renderState.charsetChunks, - preconnectChunks: renderState.preconnectChunks, - importMapChunks: renderState.importMapChunks, - preloadChunks: renderState.preloadChunks, - hoistableChunks: renderState.hoistableChunks, preconnects: renderState.preconnects, fontPreloads: renderState.fontPreloads, highImagePreloads: renderState.highImagePreloads, @@ -120,7 +110,6 @@ export function createRenderState( scripts: renderState.scripts, bulkPreloads: renderState.bulkPreloads, preloads: renderState.preloads, - boundaryResources: renderState.boundaryResources, stylesToHoist: renderState.stylesToHoist, // This is an extra field for the legacy renderer @@ -138,6 +127,7 @@ export const doctypeChunk: PrecomputedChunk = stringToPrecomputedChunk(''); export type { ResumableState, + HoistableState, BoundaryResources, FormatContext, } from './ReactFizzConfigDOM'; @@ -164,11 +154,12 @@ export { createRootFormatContext, createResumableState, createBoundaryResources, + createHoistableState, writePreamble, writeHoistables, writePostamble, - hoistResources, - setCurrentlyRenderingBoundaryResourcesTarget, + hoistBoundaryResources, + hoistHoistables, prepareHostDispatcher, resetResumableState, completeResumableState, diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index 2cc4df5d779c5..7f64612f2b828 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -671,60 +671,6 @@ describe('ReactDOMFloat', () => { ); }); - // @gate enableFloat - it('emits resources before everything else when rendering with no head', async () => { - function App() { - return ( - <> - foo - - - ); - } - - await act(() => { - buffer = `${ReactDOMFizzServer.renderToString( - , - )}foo`; - }); - expect(getMeaningfulChildren(document)).toEqual( - - - - foo - - foo - , - ); - }); - - // @gate enableFloat - it('emits resources before everything else when rendering with just a head', async () => { - function App() { - return ( - - foo - - - ); - } - - await act(() => { - buffer = `${ReactDOMFizzServer.renderToString( - , - )}foo`; - }); - expect(getMeaningfulChildren(document)).toEqual( - - - - foo - - foo - , - ); - }); - // @gate enableFloat it('emits an implicit element to hold resources when none is rendered but an is rendered', async () => { const chunks = []; @@ -4773,6 +4719,210 @@ body { ); }); + it('does not flush hoistables for fallbacks unless the fallback also flushes', async () => { + function App() { + return ( + + + +
fallback1
+ + + }> + <> +
primary1
+ + + + +
fallback2
+ + + }> + <> +
primary2
+ + + + + +
fallback3
+ + + }> + <> +
primary3
+ + + + + + + ); + } + + await act(() => { + renderToPipeableStream().pipe(writable); + resolveText('first'); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + +
primary1
+
primary2
+
fallback3
+ + , + ); + + await act(() => { + resolveText('second'); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + +
primary1
+
primary2
+
primary3
+ + + , + ); + }); + + it('avoids flushing hoistables from completed boundaries nested inside fallbacks', async () => { + function App() { + return ( + + + +
nested fallback1
+ + + }> + <> +
nested primary1
+ + + + }> + + <> +
primary1
+ + + + + + ); + } + + await act(() => { + renderToPipeableStream().pipe(writable); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + + + +
nested primary1
+ + , + ); + + await act(() => { + resolveText('release'); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + + + +
primary1
+ + + , + ); + }); + + it('avoid flushing hoistables of completed inner boundaries when inside an incomplete outer boundary', async () => { + function App() { + return ( + + + + +
blocked outer
+ + +
outer
+ + +
inner
+ + + + + + ); + } + + await act(() => { + renderToPipeableStream().pipe(writable); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + + , + ); + + await act(() => { + resolveText('release'); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + +
blocked outer
+
outer
+
inner
+ + + + + , + ); + }); + describe('ReactDOM.prefetchDNS(href)', () => { it('creates a dns-prefetch resource when called', async () => { function App({url}) { @@ -7746,6 +7896,7 @@ background-color: green; + , @@ -7758,15 +7909,15 @@ background-color: green; {/* charset first */} - {/* preconnect links next */} - - - {/* preloads next */} - + {/* viewport meta next */} + {/* Everything else last */} a title + + + , @@ -7925,10 +8076,6 @@ background-color: green; - - - - loading... , @@ -7943,10 +8090,6 @@ background-color: green; - - - - loading... , @@ -7973,15 +8116,15 @@ background-color: green; - - - -
hello world
+ + + + diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index 8816245bded02..562ecffc47ffa 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -52,6 +52,7 @@ type Destination = { }; type RenderState = null; +type HoistableState = null; type BoundaryResources = null; const POP = Buffer.from('/', 'utf8'); @@ -269,7 +270,14 @@ const ReactNoopServer = ReactFizzServer({ return null; }, - setCurrentlyRenderingBoundaryResourcesTarget(resources: BoundaryResources) {}, + createHoistableState(): HoistableState { + return null; + }, + + hoistHoistables( + parentHoistableState: HoistableState, + hoistableState: HoistableState, + ) {}, prepareHostDispatcher() {}, emitEarlyPreloads() {}, diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 16357649bbb3d..c4b15f61e5b37 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -25,8 +25,9 @@ import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy'; import type { RenderState, ResumableState, - FormatContext, + HoistableState, BoundaryResources, + FormatContext, } from './ReactFizzConfig'; import type {ContextSnapshot} from './ReactFizzNewContext'; import type {ComponentStackNode} from './ReactFizzComponentStack'; @@ -68,9 +69,10 @@ import { writePreamble, writeHoistables, writePostamble, - hoistResources, - setCurrentlyRenderingBoundaryResourcesTarget, + hoistBoundaryResources, + hoistHoistables, createBoundaryResources, + createHoistableState, prepareHostDispatcher, supportsRequestStorage, requestStorage, @@ -209,6 +211,7 @@ type SuspenseBoundary = { parentFlushed: boolean, pendingTasks: number, // when it reaches zero we can show this boundary's content completedSegments: Array, // completed but not yet flushed segments. + fallbackHoistables: null | Hoistables, byteSize: number, // used to determine whether to inline children boundaries. fallbackAbortableTasks: Set, // used to cancel task on the fallback if the boundary completes or gets canceled. resources: BoundaryResources, @@ -222,7 +225,10 @@ type RenderTask = { childIndex: number, ping: () => void, blockedBoundary: Root | SuspenseBoundary, + blockedBoundaryResources: null | BoundaryResources, // Conceptually part of the blockedBoundary but split out for performance blockedSegment: Segment, // the segment we'll write to + blockedHoistables: Hoistables, // the hoistables we'll write to + parentHoistables: null | Hoistables, abortSet: Set, // the abortable set that this task belongs to keyPath: Root | KeyNode, // the path of all parent keys currently rendering formatContext: FormatContext, // the format's specific context (e.g. HTML/SVG/MathML) @@ -247,7 +253,10 @@ type ReplayTask = { childIndex: number, ping: () => void, blockedBoundary: Root | SuspenseBoundary, + blockedBoundaryResources: null | BoundaryResources, // Conceptually part of the blockedBoundary but split out for performance blockedSegment: null, // we don't write to anything when we replay + blockedHoistables: Hoistables, // contains hoistable state for any child tasks that resume + parentHoistables: null | Hoistables, abortSet: Set, // the abortable set that this task belongs to keyPath: Root | KeyNode, // the path of all parent keys currently rendering formatContext: FormatContext, // the format's specific context (e.g. HTML/SVG/MathML) @@ -285,6 +294,11 @@ type Segment = { textEmbedded: boolean, }; +type Hoistables = { + state: HoistableState, + fallbacks: Set, +}; + const OPEN = 0; const CLOSING = 1; const CLOSED = 2; @@ -294,6 +308,7 @@ export opaque type Request = { flushScheduled: boolean, +resumableState: ResumableState, +renderState: RenderState, + +hoistables: Hoistables, +rootFormatContext: FormatContext, +progressiveChunkSize: number, status: 0 | 1 | 2, @@ -372,11 +387,13 @@ export function createRequest( prepareHostDispatcher(); const pingedTasks: Array = []; const abortSet: Set = new Set(); + const hoistables = createHoistables(); const request: Request = { destination: null, flushScheduled: false, resumableState, renderState, + hoistables, rootFormatContext, progressiveChunkSize: progressiveChunkSize === undefined @@ -421,6 +438,8 @@ export function createRequest( -1, null, rootSegment, + hoistables, + null, abortSet, null, rootFormatContext, @@ -483,11 +502,13 @@ export function resumeRequest( prepareHostDispatcher(); const pingedTasks: Array = []; const abortSet: Set = new Set(); + const hoistables = createHoistables(); const request: Request = { destination: null, flushScheduled: false, resumableState: postponedState.resumableState, renderState, + hoistables, rootFormatContext: postponedState.rootFormatContext, progressiveChunkSize: postponedState.progressiveChunkSize, status: OPEN, @@ -532,6 +553,8 @@ export function resumeRequest( -1, null, rootSegment, + hoistables, + null, abortSet, null, postponedState.rootFormatContext, @@ -556,6 +579,8 @@ export function resumeRequest( children, -1, null, + hoistables, + null, abortSet, null, postponedState.rootFormatContext, @@ -591,6 +616,7 @@ function pingTask(request: Request, task: Task): void { function createSuspenseBoundary( request: Request, fallbackAbortableTasks: Set, + fallbackHoistables: null | Hoistables, ): SuspenseBoundary { return { status: PENDING, @@ -598,6 +624,7 @@ function createSuspenseBoundary( parentFlushed: false, pendingTasks: 0, completedSegments: [], + fallbackHoistables, byteSize: 0, fallbackAbortableTasks, errorDigest: null, @@ -614,6 +641,8 @@ function createRenderTask( childIndex: number, blockedBoundary: Root | SuspenseBoundary, blockedSegment: Segment, + blockedHoistables: Hoistables, + parentHoistables: null | Hoistables, abortSet: Set, keyPath: Root | KeyNode, formatContext: FormatContext, @@ -623,10 +652,13 @@ function createRenderTask( componentStack: null | ComponentStackNode, ): RenderTask { request.allPendingTasks++; + let blockedBoundaryResources; if (blockedBoundary === null) { request.pendingRootTasks++; + blockedBoundaryResources = null; } else { blockedBoundary.pendingTasks++; + blockedBoundaryResources = blockedBoundary.resources; } const task: RenderTask = { replay: null, @@ -634,7 +666,10 @@ function createRenderTask( childIndex, ping: () => pingTask(request, task), blockedBoundary, + blockedBoundaryResources, blockedSegment, + blockedHoistables, + parentHoistables, abortSet, keyPath, formatContext, @@ -655,6 +690,8 @@ function createReplayTask( node: ReactNodeList, childIndex: number, blockedBoundary: Root | SuspenseBoundary, + blockedHoistables: Hoistables, + parentHoistables: null | Hoistables, abortSet: Set, keyPath: Root | KeyNode, formatContext: FormatContext, @@ -664,10 +701,13 @@ function createReplayTask( componentStack: null | ComponentStackNode, ): ReplayTask { request.allPendingTasks++; + let blockedBoundaryResources; if (blockedBoundary === null) { request.pendingRootTasks++; + blockedBoundaryResources = null; } else { blockedBoundary.pendingTasks++; + blockedBoundaryResources = blockedBoundary.resources; } replay.pendingTasks++; const task: ReplayTask = { @@ -676,7 +716,10 @@ function createReplayTask( childIndex, ping: () => pingTask(request, task), blockedBoundary, + blockedBoundaryResources, blockedSegment: null, + blockedHoistables, + parentHoistables, abortSet, keyPath, formatContext, @@ -712,6 +755,13 @@ function createPendingSegment( }; } +function createHoistables(): Hoistables { + return { + state: createHoistableState(), + fallbacks: new Set(), + }; +} + // DEV-only global reference to the currently executing task let currentTaskInDEV: null | Task = null; function getCurrentStackInDEV(): string { @@ -892,7 +942,10 @@ function renderSuspenseBoundary( const prevKeyPath = task.keyPath; const parentBoundary = task.blockedBoundary; + const parentBoundaryResources = task.blockedBoundaryResources; const parentSegment = task.blockedSegment; + const parentHoistables = task.blockedHoistables; + const grandParentHoistables = task.parentHoistables; // Each time we enter a suspense boundary, we split out into a new segment for // the fallback so that we can later replace that segment with the content. @@ -902,7 +955,13 @@ function renderSuspenseBoundary( const content: ReactNodeList = props.children; const fallbackAbortSet: Set = new Set(); - const newBoundary = createSuspenseBoundary(request, fallbackAbortSet); + const fallbackHoistables = createHoistables(); + parentHoistables.fallbacks.add(fallbackHoistables); + const newBoundary = createSuspenseBoundary( + request, + fallbackAbortSet, + fallbackHoistables, + ); if (request.trackedPostpones !== null) { newBoundary.trackedContentKeyPath = keyPath; } @@ -944,13 +1003,10 @@ function renderSuspenseBoundary( // context switching. We just need to temporarily switch which boundary and which segment // we're writing to. If something suspends, it'll spawn new suspended task with that context. task.blockedBoundary = newBoundary; + task.blockedBoundaryResources = newBoundary.resources; task.blockedSegment = contentRootSegment; - if (enableFloat) { - setCurrentlyRenderingBoundaryResourcesTarget( - request.renderState, - newBoundary.resources, - ); - } + task.blockedHoistables = createHoistables(); + task.parentHoistables = parentHoistables; task.keyPath = keyPath; try { @@ -972,6 +1028,7 @@ function renderSuspenseBoundary( // We are returning early so we need to restore the task.componentStack = previousComponentStack; + mergeHoistables.call(parentHoistables, task.blockedHoistables); return; } } catch (error: mixed) { @@ -1000,14 +1057,11 @@ function renderSuspenseBoundary( // We don't need to schedule any task because we know the parent has written yet. // We do need to fallthrough to create the fallback though. } finally { - if (enableFloat) { - setCurrentlyRenderingBoundaryResourcesTarget( - request.renderState, - parentBoundary ? parentBoundary.resources : null, - ); - } task.blockedBoundary = parentBoundary; + task.blockedBoundaryResources = parentBoundaryResources; task.blockedSegment = parentSegment; + task.blockedHoistables = parentHoistables; + task.parentHoistables = grandParentHoistables; task.keyPath = prevKeyPath; task.componentStack = previousComponentStack; } @@ -1034,6 +1088,7 @@ function renderSuspenseBoundary( newBoundary.trackedFallbackNode = fallbackReplayNode; } } + // We create suspended task for the fallback because we don't want to actually work // on it yet in case we finish the main content, so we queue for later. const suspendedFallbackTask = createRenderTask( @@ -1043,6 +1098,8 @@ function renderSuspenseBoundary( -1, parentBoundary, boundarySegment, + fallbackHoistables, + null, fallbackAbortSet, fallbackKeyPath, task.formatContext, @@ -1079,12 +1136,21 @@ function replaySuspenseBoundary( const previousReplaySet: ReplaySet = task.replay; const parentBoundary = task.blockedBoundary; + const parentBoundaryResources = task.blockedBoundaryResources; + const parentHoistables = task.blockedHoistables; + const grandParentHoistables = task.parentHoistables; const content: ReactNodeList = props.children; const fallback: ReactNodeList = props.fallback; const fallbackAbortSet: Set = new Set(); - const resumedBoundary = createSuspenseBoundary(request, fallbackAbortSet); + const fallbackHoistables = createHoistables(); + parentHoistables.fallbacks.add(fallbackHoistables); + const resumedBoundary = createSuspenseBoundary( + request, + fallbackAbortSet, + fallbackHoistables, + ); resumedBoundary.parentFlushed = true; // We restore the same id of this boundary as was used during prerender. resumedBoundary.rootSegmentID = id; @@ -1093,14 +1159,10 @@ function replaySuspenseBoundary( // context switching. We just need to temporarily switch which boundary and replay node // we're writing to. If something suspends, it'll spawn new suspended task with that context. task.blockedBoundary = resumedBoundary; + task.blockedBoundaryResources = resumedBoundary.resources; + task.blockedHoistables = createHoistables(); + task.parentHoistables = parentHoistables; task.replay = {nodes: childNodes, slots: childSlots, pendingTasks: 1}; - if (enableFloat) { - setCurrentlyRenderingBoundaryResourcesTarget( - request.renderState, - resumedBoundary.resources, - ); - } - try { // We use the safe form because we don't handle suspending here. Only error handling. renderNode(request, task, content, -1); @@ -1154,13 +1216,10 @@ function replaySuspenseBoundary( // We don't need to schedule any task because we know the parent has written yet. // We do need to fallthrough to create the fallback though. } finally { - if (enableFloat) { - setCurrentlyRenderingBoundaryResourcesTarget( - request.renderState, - parentBoundary ? parentBoundary.resources : null, - ); - } task.blockedBoundary = parentBoundary; + task.blockedBoundaryResources = parentBoundaryResources; + task.blockedHoistables = parentHoistables; + task.parentHoistables = grandParentHoistables; task.replay = previousReplaySet; task.keyPath = prevKeyPath; task.componentStack = previousComponentStack; @@ -1182,6 +1241,8 @@ function replaySuspenseBoundary( fallback, -1, parentBoundary, + fallbackHoistables, + null, fallbackAbortSet, fallbackKeyPath, task.formatContext, @@ -1251,12 +1312,15 @@ function renderHostElement( task.keyPath = prevKeyPath; } else { // Render + const hoistables = task.blockedHoistables; const children = pushStartInstance( segment.chunks, type, props, request.resumableState, request.renderState, + task.blockedBoundaryResources, + hoistables.state, task.formatContext, segment.lastPushedText, ); @@ -2741,6 +2805,8 @@ function spawnNewSuspendedReplayTask( task.node, task.childIndex, task.blockedBoundary, + task.blockedHoistables, + task.parentHoistables, task.abortSet, task.keyPath, task.formatContext, @@ -2785,6 +2851,8 @@ function spawnNewSuspendedRenderTask( task.childIndex, task.blockedBoundary, newSegment, + task.blockedHoistables, + task.parentHoistables, task.abortSet, task.keyPath, task.formatContext, @@ -3062,11 +3130,13 @@ function abortTaskSoft(this: Request, task: Task): void { // It's used for when we didn't need this task to complete the tree. // If task was needed, then it should use abortTask instead. const request: Request = this; - const boundary = task.blockedBoundary; const segment = task.blockedSegment; if (segment !== null) { + const boundary = task.blockedBoundary; + const hoistables = task.blockedHoistables; + const parentHoistables = task.parentHoistables; segment.status = ABORTED; - finishedTask(request, boundary, segment); + finishedTask(request, boundary, segment, hoistables, parentHoistables); } } @@ -3077,7 +3147,7 @@ function abortRemainingSuspenseBoundary( errorDigest: ?string, errorInfo: ThrownInfo, ): void { - const resumedBoundary = createSuspenseBoundary(request, new Set()); + const resumedBoundary = createSuspenseBoundary(request, new Set(), null); resumedBoundary.parentFlushed = true; // We restore the same id of this boundary as was used during prerender. resumedBoundary.rootSegmentID = rootSegmentID; @@ -3325,10 +3395,31 @@ function queueCompletedSegment( } } +// Merges the internal state of Hoistables from a source to a target. afterwards +// both source and target will share the same unified state. This technique is used +// so we can resolve tasks in any order and always accumulate up to the root Hoistable. +// This function uses the `this` argument to allow for slightly optimized calling from +// a forEach over a Set. +function mergeHoistables(this: Hoistables, source: Hoistables) { + if (enableFloat) { + const target = this; + hoistHoistables(target.state, source.state); + source.fallbacks.forEach(h => { + target.fallbacks.add(h); + }); + // we assign the parent state and fallbacks to the child state so if a child task completes + // it will hoist and merge with the parent Hoistables + source.state = target.state; + source.fallbacks = target.fallbacks; + } +} + function finishedTask( request: Request, boundary: Root | SuspenseBoundary, segment: null | Segment, + hoistables: Hoistables, + parentHoistables: null | Hoistables, ) { if (boundary === null) { if (segment !== null && segment.parentFlushed) { @@ -3367,6 +3458,20 @@ function finishedTask( request.completedBoundaries.push(boundary); } + if (enableFloat && parentHoistables) { + // We have completed a boundary and need to merge this boundary's Hoistables with the parent + // Hoistables for this task. First we remove the fallback Hoistables. If they already flushed + // we can't do anythign about it but we don't want to flush them now if unflushed because the fallback + // will never show + if (boundary.fallbackHoistables) { + parentHoistables.fallbacks.delete(boundary.fallbackHoistables); + } + // Next we merge the boundary Hoistables into the task Hoistables. In the process the boundary assumes + // the task Hoistables internal state so later if a child task also completes it will merge with + // the appropriate sets + mergeHoistables.call(parentHoistables, hoistables); + } + // We can now cancel any pending task on the fallback since we won't need to show it anymore. // This needs to happen after we read the parentFlushed flags because aborting can finish // work which can trigger user code, which can start flushing, which can change those flags. @@ -3403,13 +3508,6 @@ function finishedTask( } function retryTask(request: Request, task: Task): void { - if (enableFloat) { - const blockedBoundary = task.blockedBoundary; - setCurrentlyRenderingBoundaryResourcesTarget( - request.renderState, - blockedBoundary ? blockedBoundary.resources : null, - ); - } const segment = task.blockedSegment; if (segment === null) { retryReplayTask( @@ -3474,7 +3572,13 @@ function retryRenderTask( task.abortSet.delete(task); segment.status = COMPLETED; - finishedTask(request, task.blockedBoundary, segment); + finishedTask( + request, + task.blockedBoundary, + segment, + task.blockedHoistables, + task.parentHoistables, + ); } catch (thrownValue) { resetHooksState(); @@ -3515,7 +3619,13 @@ function retryRenderTask( const postponeInfo = getThrownInfo(request, task.componentStack); logPostpone(request, postponeInstance.message, postponeInfo); trackPostpone(request, trackedPostpones, task, segment); - finishedTask(request, task.blockedBoundary, segment); + finishedTask( + request, + task.blockedBoundary, + segment, + task.blockedHoistables, + task.parentHoistables, + ); return; } } @@ -3526,9 +3636,6 @@ function retryRenderTask( erroredTask(request, task.blockedBoundary, x, errorInfo); return; } finally { - if (enableFloat) { - setCurrentlyRenderingBoundaryResourcesTarget(request.renderState, null); - } if (__DEV__) { currentTaskInDEV = prevTaskInDEV; } @@ -3578,7 +3685,13 @@ function retryReplayTask(request: Request, task: ReplayTask): void { task.replay.pendingTasks--; task.abortSet.delete(task); - finishedTask(request, task.blockedBoundary, null); + finishedTask( + request, + task.blockedBoundary, + null, + task.blockedHoistables, + task.parentHoistables, + ); } catch (thrownValue) { resetHooksState(); @@ -3623,9 +3736,6 @@ function retryReplayTask(request: Request, task: ReplayTask): void { } return; } finally { - if (enableFloat) { - setCurrentlyRenderingBoundaryResourcesTarget(request.renderState, null); - } if (__DEV__) { currentTaskInDEV = prevTaskInDEV; } @@ -3698,6 +3808,7 @@ function flushSubtree( request: Request, destination: Destination, segment: Segment, + rootBoundary: null | SuspenseBoundary, ): boolean { segment.parentFlushed = true; switch (segment.status) { @@ -3727,7 +3838,7 @@ function flushSubtree( for (; chunkIdx < nextChild.index; chunkIdx++) { writeChunk(destination, chunks[chunkIdx]); } - r = flushSegment(request, destination, nextChild); + r = flushSegment(request, destination, nextChild, rootBoundary); } // Finally just write all the remaining chunks for (; chunkIdx < chunks.length - 1; chunkIdx++) { @@ -3750,11 +3861,12 @@ function flushSegment( request: Request, destination: Destination, segment: Segment, + rootBoundary: null | SuspenseBoundary, ): boolean { const boundary = segment.boundary; if (boundary === null) { // Not a suspense boundary. - return flushSubtree(request, destination, segment); + return flushSubtree(request, destination, segment, rootBoundary); } boundary.parentFlushed = true; @@ -3772,7 +3884,7 @@ function flushSegment( boundary.errorComponentStack, ); // Flush the fallback. - flushSubtree(request, destination, segment); + flushSubtree(request, destination, segment, rootBoundary); return writeEndClientRenderedSuspenseBoundary( destination, @@ -3796,7 +3908,7 @@ function flushSegment( writeStartPendingSuspenseBoundary(destination, request.renderState, id); // Flush the fallback. - flushSubtree(request, destination, segment); + flushSubtree(request, destination, segment, rootBoundary); return writeEndPendingSuspenseBoundary(destination, request.renderState); } else if (boundary.byteSize > request.progressiveChunkSize) { @@ -3818,12 +3930,12 @@ function flushSegment( ); // Flush the fallback. - flushSubtree(request, destination, segment); + flushSubtree(request, destination, segment, rootBoundary); return writeEndPendingSuspenseBoundary(destination, request.renderState); } else { - if (enableFloat) { - hoistResources(request.renderState, boundary.resources); + if (enableFloat && rootBoundary && rootBoundary !== boundary) { + hoistBoundaryResources(rootBoundary.resources, boundary.resources); } // We can inline this boundary's content as a complete boundary. writeStartCompletedSuspenseBoundary(destination, request.renderState); @@ -3837,7 +3949,7 @@ function flushSegment( } const contentSegment = completedSegments[0]; - flushSegment(request, destination, contentSegment); + flushSegment(request, destination, contentSegment, rootBoundary); return writeEndCompletedSuspenseBoundary(destination, request.renderState); } @@ -3863,6 +3975,7 @@ function flushSegmentContainer( request: Request, destination: Destination, segment: Segment, + boundary: SuspenseBoundary, ): boolean { writeStartSegment( destination, @@ -3870,7 +3983,7 @@ function flushSegmentContainer( segment.parentFormatContext, segment.id, ); - flushSegment(request, destination, segment); + flushSegment(request, destination, segment, boundary); return writeEndSegment(destination, segment.parentFormatContext); } @@ -3879,12 +3992,6 @@ function flushCompletedBoundary( destination: Destination, boundary: SuspenseBoundary, ): boolean { - if (enableFloat) { - setCurrentlyRenderingBoundaryResourcesTarget( - request.renderState, - boundary.resources, - ); - } const completedSegments = boundary.completedSegments; let i = 0; for (; i < completedSegments.length; i++) { @@ -3915,12 +4022,6 @@ function flushPartialBoundary( destination: Destination, boundary: SuspenseBoundary, ): boolean { - if (enableFloat) { - setCurrentlyRenderingBoundaryResourcesTarget( - request.renderState, - boundary.resources, - ); - } const completedSegments = boundary.completedSegments; let i = 0; for (; i < completedSegments.length; i++) { @@ -3975,13 +4076,13 @@ function flushPartiallyCompletedSegment( ); } - return flushSegmentContainer(request, destination, segment); + return flushSegmentContainer(request, destination, segment, boundary); } else if (segmentID === boundary.rootSegmentID) { // When we emit postponed boundaries, we might have assigned the ID already // but it's still the root segment so we can't inject it into the parent yet. - return flushSegmentContainer(request, destination, segment); + return flushSegmentContainer(request, destination, segment, boundary); } else { - flushSegmentContainer(request, destination, segment); + flushSegmentContainer(request, destination, segment, boundary); return writeCompletedSegmentInstruction( destination, request.resumableState, @@ -3991,6 +4092,17 @@ function flushPartiallyCompletedSegment( } } +function prepareToFlushHoistables(request: Request) { + // At the moment we flush we merge all fallback Hoistables visible to the request's Hoistables + // object. These represent hoistables for Boundaries that will flush a fallback because the + // primary content isn't ready yet. If a boundary completes before this step then the fallback + // Hoistables would have already been removed from this set so we know it only includes necessary + // fallback hoistables + const requestHoistables = request.hoistables; + requestHoistables.fallbacks.forEach(mergeHoistables, requestHoistables); + requestHoistables.fallbacks.clear(); +} + function flushCompletedQueues( request: Request, destination: Destination, @@ -4010,15 +4122,17 @@ function flushCompletedQueues( return; } else if (request.pendingRootTasks === 0) { if (enableFloat) { + prepareToFlushHoistables(request); writePreamble( destination, request.resumableState, request.renderState, + request.hoistables.state, request.allPendingTasks === 0 && request.trackedPostpones === null, ); } - flushSegment(request, destination, completedRootSegment); + flushSegment(request, destination, completedRootSegment, null); request.completedRootSegment = null; writeCompletedRoot(destination, request.renderState); } else { @@ -4028,7 +4142,13 @@ function flushCompletedQueues( } if (enableFloat) { - writeHoistables(destination, request.resumableState, request.renderState); + prepareToFlushHoistables(request); + writeHoistables( + destination, + request.resumableState, + request.renderState, + request.hoistables.state, + ); } // We emit client rendering instructions for already emitted boundaries first. diff --git a/packages/react-server/src/forks/ReactFizzConfig.custom.js b/packages/react-server/src/forks/ReactFizzConfig.custom.js index dbf6b32aa8201..7a2c256fc025f 100644 --- a/packages/react-server/src/forks/ReactFizzConfig.custom.js +++ b/packages/react-server/src/forks/ReactFizzConfig.custom.js @@ -29,6 +29,7 @@ import type {TransitionStatus} from 'react-reconciler/src/ReactFiberConfig'; declare var $$$config: any; export opaque type Destination = mixed; // eslint-disable-line no-undef export opaque type RenderState = mixed; +export opaque type HoistableState = mixed; export opaque type ResumableState = mixed; export opaque type BoundaryResources = mixed; export opaque type FormatContext = mixed; @@ -87,10 +88,9 @@ export const NotPendingTransition = $$$config.NotPendingTransition; export const writePreamble = $$$config.writePreamble; export const writeHoistables = $$$config.writeHoistables; export const writePostamble = $$$config.writePostamble; -export const hoistResources = $$$config.hoistResources; -export const createResources = $$$config.createResources; +export const hoistBoundaryResources = $$$config.hoistBoundaryResources; +export const hoistHoistables = $$$config.hoistHoistables; +export const createHoistableState = $$$config.createHoistableState; export const createBoundaryResources = $$$config.createBoundaryResources; -export const setCurrentlyRenderingBoundaryResourcesTarget = - $$$config.setCurrentlyRenderingBoundaryResourcesTarget; export const writeResourcesForBoundary = $$$config.writeResourcesForBoundary; export const emitEarlyPreloads = $$$config.emitEarlyPreloads; From 852f521897f70a5277deb6b32e81daa2efea0833 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Tue, 23 Jan 2024 13:24:20 -0800 Subject: [PATCH 2/3] Uses a simpler approach to trackign hoistables and only writes them either during the preamble or when a boundary complets. The cost is that during the preamble flush there is an extra traversal of the segments that will write in the preamble. While this is not ideal for perf it does make the tracking significantly easier to follow and overall probalby strikes a better balance of practicality and maintainability over raw performance. --- .../src/server/ReactFizzConfigDOM.js | 266 +++++++++----- .../src/server/ReactFizzConfigDOMLegacy.js | 15 +- .../src/__tests__/ReactDOMFloat-test.js | 51 --- .../src/ReactNoopServer.js | 19 +- packages/react-server/src/ReactFizzServer.js | 340 +++++++----------- .../src/forks/ReactFizzConfig.custom.js | 11 +- 6 files changed, 328 insertions(+), 374 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 067f668262be4..c5eeaa5809b78 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -147,9 +147,12 @@ export type RenderState = { // external runtime script chunks externalRuntimeScript: null | ExternalRuntimeScript, bootstrapChunks: Array, + importMapChunks: Array, // Hoistable chunks - importMapChunks: Array, + charsetChunks: Array, + viewportChunks: Array, + hoistableChunks: Array, // Headers queues for Resources that can flush early onHeaders: void | ((headers: HeadersDescriptor) => void), @@ -466,6 +469,10 @@ export function createRenderState( style: {}, }, + charsetChunks: [], + viewportChunks: [], + hoistableChunks: [], + // cleared on flush preconnects: new Set(), fontPreloads: new Set(), @@ -485,6 +492,7 @@ export function createRenderState( nonce, // like a module global for currently rendering boundary + hoistableState: null, stylesToHoist: false, }; @@ -2213,7 +2221,8 @@ function pushStartTextArea( function pushMeta( target: Array, props: Object, - hoistableState: HoistableState, + renderState: RenderState, + hoistableState: null | HoistableState, textEmbedded: boolean, insertionMode: InsertionMode, noscriptTagInScope: boolean, @@ -2233,12 +2242,30 @@ function pushMeta( } if (typeof props.charSet === 'string') { - return pushSelfClosing(hoistableState.charset, props, 'meta'); + return pushSelfClosing( + hoistableState + ? hoistableState.charsetChunks + : renderState.charsetChunks, + props, + 'meta', + ); } else if (props.name === 'viewport') { // "viewport" isn't related to preconnect but it has the right priority - return pushSelfClosing(hoistableState.viewport, props, 'meta'); + return pushSelfClosing( + hoistableState + ? hoistableState.viewportChunks + : renderState.viewportChunks, + props, + 'meta', + ); } else { - return pushSelfClosing(hoistableState.chunks, props, 'meta'); + return pushSelfClosing( + hoistableState + ? hoistableState.hoistableChunks + : renderState.hoistableChunks, + props, + 'meta', + ); } } } else { @@ -2251,8 +2278,7 @@ function pushLink( props: Object, resumableState: ResumableState, renderState: RenderState, - boundaryResources: null | BoundaryResources, - hoistableState: HoistableState, + hoistableState: null | HoistableState, textEmbedded: boolean, insertionMode: InsertionMode, noscriptTagInScope: boolean, @@ -2373,8 +2399,8 @@ function pushLink( // We add the newly created resource to our StyleQueue and if necessary // track the resource with the currently rendering boundary styleQueue.sheets.set(key, resource); - if (boundaryResources) { - boundaryResources.stylesheets.add(resource); + if (hoistableState) { + hoistableState.stylesheets.add(resource); } } else { // We need to track whether this boundary should wait on this resource or not. @@ -2385,8 +2411,8 @@ function pushLink( if (styleQueue) { const resource = styleQueue.sheets.get(key); if (resource) { - if (boundaryResources) { - boundaryResources.stylesheets.add(resource); + if (hoistableState) { + hoistableState.stylesheets.add(resource); } } } @@ -2411,7 +2437,10 @@ function pushLink( target.push(textSeparator); } - return pushLinkImpl(hoistableState.chunks, props); + const hoistableChunks = hoistableState + ? hoistableState.hoistableChunks + : renderState.hoistableChunks; + return pushLinkImpl(hoistableChunks, props); } } else { return pushLinkImpl(target, props); @@ -2453,7 +2482,7 @@ function pushStyle( props: Object, resumableState: ResumableState, renderState: RenderState, - boundaryResources: null | BoundaryResources, + hoistableState: null | HoistableState, textEmbedded: boolean, insertionMode: InsertionMode, noscriptTagInScope: boolean, @@ -2553,8 +2582,8 @@ function pushStyle( // it. However, it's possible when you resume that the style has already been emitted // and then it wouldn't be recreated in the RenderState and there's no need to track // it again since we should've hoisted it to the shell already. - if (boundaryResources) { - boundaryResources.styles.add(styleQueue); + if (hoistableState) { + hoistableState.styles.add(styleQueue); } } @@ -2864,7 +2893,8 @@ function pushStartMenuItem( function pushTitle( target: Array, props: Object, - hoistableState: HoistableState, + renderState: RenderState, + hoistableState: null | HoistableState, insertionMode: InsertionMode, noscriptTagInScope: boolean, ): ReactNodeList { @@ -2922,7 +2952,10 @@ function pushTitle( !noscriptTagInScope && props.itemProp == null ) { - pushTitleImpl(hoistableState.chunks, props); + const hoistableTarget = hoistableState + ? hoistableState.hoistableChunks + : renderState.hoistableChunks; + pushTitleImpl(hoistableTarget, props); return null; } else { return pushTitleImpl(target, props); @@ -3454,8 +3487,7 @@ export function pushStartInstance( props: Object, resumableState: ResumableState, renderState: RenderState, - boundaryResources: null | BoundaryResources, - hoistableState: HoistableState, + hoistableState: null | HoistableState, formatContext: FormatContext, textEmbedded: boolean, ): ReactNodeList { @@ -3523,6 +3555,7 @@ export function pushStartInstance( ? pushTitle( target, props, + renderState, hoistableState, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), @@ -3534,7 +3567,6 @@ export function pushStartInstance( props, resumableState, renderState, - boundaryResources, hoistableState, textEmbedded, formatContext.insertionMode, @@ -3558,7 +3590,7 @@ export function pushStartInstance( props, resumableState, renderState, - boundaryResources, + hoistableState, textEmbedded, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), @@ -3567,6 +3599,7 @@ export function pushStartInstance( return pushMeta( target, props, + renderState, hoistableState, textEmbedded, formatContext.insertionMode, @@ -4107,7 +4140,7 @@ export function writeCompletedBoundaryInstruction( resumableState: ResumableState, renderState: RenderState, id: number, - boundaryResources: BoundaryResources, + hoistableState: HoistableState, ): boolean { let requiresStyleInsertion; if (enableFloat) { @@ -4183,11 +4216,11 @@ export function writeCompletedBoundaryInstruction( // e.g. ["A", "B"] if (scriptFormat) { writeChunk(destination, completeBoundaryScript3a); - // boundaryResources encodes an array literal - writeStyleResourceDependenciesInJS(destination, boundaryResources); + // hoistableState encodes an array literal + writeStyleResourceDependenciesInJS(destination, hoistableState); } else { writeChunk(destination, completeBoundaryData3a); - writeStyleResourceDependenciesInAttr(destination, boundaryResources); + writeStyleResourceDependenciesInAttr(destination, hoistableState); } } else { if (scriptFormat) { @@ -4436,9 +4469,35 @@ function hasStylesToHoist(stylesheet: StylesheetResource): boolean { return false; } -export function writeResourcesForBoundary( +export function writeHoistablesForPartialBoundary( + destination: Destination, + hoistableState: HoistableState, + renderState: RenderState, +): boolean { + // Reset these on each invocation, they are only safe to read in this function + currentlyRenderingBoundaryHasStylesToHoist = false; + destinationHasCapacity = true; + + // Flush style tags for each precedence this boundary depends on + hoistableState.styles.forEach(flushStyleTagsLateForBoundary, destination); + + // Determine if this boundary has stylesheets that need to be awaited upon completion + hoistableState.stylesheets.forEach(hasStylesToHoist); + + // We don't actually want to flush any hoistables until the boundary is complete so we omit + // any further writing here. This is becuase unlike Resources, Hoistable Elements act more like + // regular elements, each rendered element has a unique representation in the DOM. We don't want + // these elements to appear in the DOM early, before the boundary has actually completed + + if (currentlyRenderingBoundaryHasStylesToHoist) { + renderState.stylesToHoist = true; + } + return destinationHasCapacity; +} + +export function writeHoistablesForCompletedBoundary( destination: Destination, - boundaryResources: BoundaryResources, + hoistableState: HoistableState, renderState: RenderState, ): boolean { // Reset these on each invocation, they are only safe to read in this function @@ -4446,10 +4505,40 @@ export function writeResourcesForBoundary( destinationHasCapacity = true; // Flush style tags for each precedence this boundary depends on - boundaryResources.styles.forEach(flushStyleTagsLateForBoundary, destination); + hoistableState.styles.forEach(flushStyleTagsLateForBoundary, destination); // Determine if this boundary has stylesheets that need to be awaited upon completion - boundaryResources.stylesheets.forEach(hasStylesToHoist); + hoistableState.stylesheets.forEach(hasStylesToHoist); + + // Flush Hoistable Elements + let i; + const charsetChunks = hoistableState.charsetChunks; + for (i = 0; i < charsetChunks.length - 1; i++) { + writeChunk(destination, charsetChunks[i]); + } + if (i < charsetChunks.length) { + destinationHasCapacity = writeChunkAndReturn(destination, charsetChunks[i]); + } + const viewportChunks = hoistableState.viewportChunks; + for (i = 0; i < viewportChunks.length - 1; i++) { + writeChunk(destination, charsetChunks[i]); + } + if (i < viewportChunks.length) { + destinationHasCapacity = writeChunkAndReturn( + destination, + viewportChunks[i], + ); + } + const hoistableChunks = hoistableState.hoistableChunks; + for (i = 0; i < hoistableChunks.length - 1; i++) { + writeChunk(destination, hoistableChunks[i]); + } + if (i < hoistableChunks.length) { + destinationHasCapacity = writeChunkAndReturn( + destination, + hoistableChunks[i], + ); + } if (currentlyRenderingBoundaryHasStylesToHoist) { renderState.stylesToHoist = true; @@ -4561,7 +4650,6 @@ export function writePreamble( destination: Destination, resumableState: ResumableState, renderState: RenderState, - hoistableState: HoistableState, willFlushAllSegments: boolean, ): void { // This function must be called exactly once on every request @@ -4607,7 +4695,7 @@ export function writePreamble( } // Emit high priority Hoistables - const charsetChunks = hoistableState.charset; + const charsetChunks = renderState.charsetChunks; for (i = 0; i < charsetChunks.length; i++) { writeChunk(destination, charsetChunks[i]); } @@ -4617,7 +4705,7 @@ export function writePreamble( renderState.preconnects.forEach(flushResource, destination); renderState.preconnects.clear(); - const viewportChunks = hoistableState.viewport; + const viewportChunks = renderState.viewportChunks; for (i = 0; i < viewportChunks.length; i++) { writeChunk(destination, viewportChunks[i]); } @@ -4646,8 +4734,8 @@ export function writePreamble( renderState.bulkPreloads.forEach(flushResource, destination); renderState.bulkPreloads.clear(); - // Write hoistableState chunks - const hoistableChunks = hoistableState.chunks; + // Write embedding hoistableChunks + const hoistableChunks = renderState.hoistableChunks; for (i = 0; i < hoistableChunks.length; i++) { writeChunk(destination, hoistableChunks[i]); } @@ -4664,23 +4752,15 @@ export function writePreamble( } } -// We don't bother reporting backpressure at the moment because we expect to -// flush the entire preamble in a single pass. This probably should be modified -// in the future to be backpressure sensitive but that requires a larger refactor -// of the flushing code in Fizz. +// This is an opportunity to write hoistables however in the current implemention +// the only hoistables that make sense to write here are Resources. Hoistable Elements +// would have already been written as part of the preamble or will be written as part +// of a boundary completion and thus don't need to be written here. export function writeHoistables( destination: Destination, resumableState: ResumableState, renderState: RenderState, - hoistableState: HoistableState, ): void { - let i = 0; - - // Emit high priority Hoistables - - // We omit charsetChunks because we have already sent the shell and if it wasn't - // already sent it is too late now. - renderState.preconnects.forEach(flushResource, destination); renderState.preconnects.clear(); @@ -4707,13 +4787,6 @@ export function writeHoistables( renderState.bulkPreloads.forEach(flushResource, destination); renderState.bulkPreloads.clear(); - - // Write hoistableState chunks - const hoistableChunks = hoistableState.chunks; - for (i = 0; i < hoistableChunks.length; i++) { - writeChunk(destination, hoistableChunks[i]); - } - hoistableChunks.length = 0; } export function writePostamble( @@ -4738,12 +4811,12 @@ const arrayCloseBracket = stringToPrecomputedChunk(']'); // [["JS_escaped_string1", "JS_escaped_string2"]] function writeStyleResourceDependenciesInJS( destination: Destination, - boundaryResources: BoundaryResources, + hoistableState: HoistableState, ): void { writeChunk(destination, arrayFirstOpenBracket); let nextArrayOpenBrackChunk = arrayFirstOpenBracket; - boundaryResources.stylesheets.forEach(resource => { + hoistableState.stylesheets.forEach(resource => { if (resource.state === PREAMBLE) { // We can elide this dependency because it was flushed in the shell and // should be ready before content is shown on the client @@ -4931,12 +5004,12 @@ function writeStyleResourceAttributeInJS( // [["JSON_escaped_string1", "JSON_escaped_string2"]] function writeStyleResourceDependenciesInAttr( destination: Destination, - boundaryResources: BoundaryResources, + hoistableState: HoistableState, ): void { writeChunk(destination, arrayFirstOpenBracket); let nextArrayOpenBrackChunk = arrayFirstOpenBracket; - boundaryResources.stylesheets.forEach(resource => { + hoistableState.stylesheets.forEach(resource => { if (resource.state === PREAMBLE) { // We can elide this dependency because it was flushed in the shell and // should be ready before content is shown on the client @@ -5184,23 +5257,12 @@ type StylesheetResource = { }; export type HoistableState = { - charset: Array, - viewport: Array, - chunks: Array, -}; - -export function createHoistableState(): HoistableState { - return { - charset: [], - viewport: [], - chunks: [], - }; -} - -export type BoundaryResources = { - // style dependencies styles: Set, stylesheets: Set, + // Hoistable chunks + charsetChunks: Array, + viewportChunks: Array, + hoistableChunks: Array, }; export type StyleQueue = { @@ -5210,10 +5272,13 @@ export type StyleQueue = { sheets: Map, }; -export function createBoundaryResources(): BoundaryResources { +export function createHoistableState(): HoistableState { return { styles: new Set(), stylesheets: new Set(), + charsetChunks: [], + viewportChunks: [], + hoistableChunks: [], }; } @@ -6063,35 +6128,58 @@ function escapeStringForLinkHeaderQuotedParamValueContextReplacer( } } -export function hoistHoistables( - target: HoistableState, - source: HoistableState, -) { - target.charset.push(...source.charset); - target.viewport.push(...source.viewport); - target.chunks.push(...source.chunks); -} - function hoistStyleQueueDependency( - this: BoundaryResources, + this: HoistableState, styleQueue: StyleQueue, ) { this.styles.add(styleQueue); } function hoistStylesheetDependency( - this: BoundaryResources, + this: HoistableState, stylesheet: StylesheetResource, ) { this.stylesheets.add(stylesheet); } -export function hoistBoundaryResources( - target: BoundaryResources, - source: BoundaryResources, +export function hoistToBoundary( + parentState: HoistableState, + childState: HoistableState, ): void { - source.styles.forEach(hoistStyleQueueDependency, target); - source.stylesheets.forEach(hoistStylesheetDependency, target); + childState.styles.forEach(hoistStyleQueueDependency, parentState); + childState.stylesheets.forEach(hoistStylesheetDependency, parentState); + let i; + const charsetChunks = childState.charsetChunks; + for (i = 0; i < charsetChunks.length; i++) { + parentState.charsetChunks.push(charsetChunks[i]); + } + const viewportChunks = childState.viewportChunks; + for (i = 0; i < charsetChunks.length; i++) { + parentState.viewportChunks.push(viewportChunks[i]); + } + const hoistableChunks = childState.hoistableChunks; + for (i = 0; i < hoistableChunks.length; i++) { + parentState.hoistableChunks.push(hoistableChunks[i]); + } +} + +export function hoistToRoot( + renderState: RenderState, + hoistableState: HoistableState, +): void { + let i; + const charsetChunks = hoistableState.charsetChunks; + for (i = 0; i < charsetChunks.length; i++) { + renderState.charsetChunks.push(charsetChunks[i]); + } + const viewportChunks = hoistableState.viewportChunks; + for (i = 0; i < charsetChunks.length; i++) { + renderState.viewportChunks.push(viewportChunks[i]); + } + const hoistableChunks = hoistableState.hoistableChunks; + for (i = 0; i < hoistableChunks.length; i++) { + renderState.hoistableChunks.push(hoistableChunks[i]); + } } // This function is called at various times depending on whether we are rendering diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js index fb0fef7b3cc14..35554654cd07f 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js @@ -56,6 +56,9 @@ export type RenderState = { remainingCapacity: number, }, resets: BaseRenderState['resets'], + charsetChunks: Array, + viewportChunks: Array, + hoistableChunks: Array, preconnects: Set, fontPreloads: Set, highImagePreloads: Set, @@ -101,6 +104,9 @@ export function createRenderState( onHeaders: renderState.onHeaders, headers: renderState.headers, resets: renderState.resets, + charsetChunks: renderState.charsetChunks, + viewportChunks: renderState.viewportChunks, + hoistableChunks: renderState.hoistableChunks, preconnects: renderState.preconnects, fontPreloads: renderState.fontPreloads, highImagePreloads: renderState.highImagePreloads, @@ -128,7 +134,6 @@ export const doctypeChunk: PrecomputedChunk = stringToPrecomputedChunk(''); export type { ResumableState, HoistableState, - BoundaryResources, FormatContext, } from './ReactFizzConfigDOM'; @@ -148,18 +153,18 @@ export { writeClientRenderBoundaryInstruction, writeStartPendingSuspenseBoundary, writeEndPendingSuspenseBoundary, - writeResourcesForBoundary, + writeHoistablesForPartialBoundary, + writeHoistablesForCompletedBoundary, writePlaceholder, writeCompletedRoot, createRootFormatContext, createResumableState, - createBoundaryResources, createHoistableState, writePreamble, writeHoistables, writePostamble, - hoistBoundaryResources, - hoistHoistables, + hoistToBoundary, + hoistToRoot, prepareHostDispatcher, resetResumableState, completeResumableState, diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index 7f64612f2b828..cb48446087912 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -7924,57 +7924,6 @@ background-color: green; ); }); - // @gate enableFloat - it('emits hoistables before other content when streaming in late', async () => { - let content = ''; - writable.on('data', chunk => (content += chunk)); - - await act(() => { - const {pipe} = renderToPipeableStream( - - - - - -
foo
- - - - - , - ); - pipe(writable); - }); - - expect(getMeaningfulChildren(document)).toEqual( - - - - - - , - ); - content = ''; - - await act(() => { - resolveText('foo'); - }); - - expect(content.slice(0, 30)).toEqual('
- - - - -
foo
- - - , - ); - }); - // @gate enableFloat it('supports rendering hoistables outside of scope', async () => { await act(() => { diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index 562ecffc47ffa..ae508385ba9b4 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -53,7 +53,6 @@ type Destination = { type RenderState = null; type HoistableState = null; -type BoundaryResources = null; const POP = Buffer.from('/', 'utf8'); @@ -262,24 +261,18 @@ const ReactNoopServer = ReactFizzServer({ boundary.status = 'client-render'; }, + prepareHostDispatcher() {}, + writePreamble() {}, writeHoistables() {}, writePostamble() {}, - - createBoundaryResources(): BoundaryResources { - return null; - }, - + hoistToRoot(renderState: RenderState, hoistableState: HoistableState) {}, + hoistToBoundary(parent: HoistableState, child: HoistableState) {}, createHoistableState(): HoistableState { return null; }, - - hoistHoistables( - parentHoistableState: HoistableState, - hoistableState: HoistableState, - ) {}, - - prepareHostDispatcher() {}, + writeHoistablesForPartialBoundary() {}, + writeHoistablesForCompleteBoundary() {}, emitEarlyPreloads() {}, }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index c4b15f61e5b37..ec59a382d3b5d 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -25,9 +25,8 @@ import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy'; import type { RenderState, ResumableState, - HoistableState, - BoundaryResources, FormatContext, + HoistableState, } from './ReactFizzConfig'; import type {ContextSnapshot} from './ReactFizzNewContext'; import type {ComponentStackNode} from './ReactFizzComponentStack'; @@ -65,13 +64,13 @@ import { pushEndCompletedSuspenseBoundary, pushSegmentFinale, getChildFormatContext, - writeResourcesForBoundary, - writePreamble, + writeHoistablesForPartialBoundary, + writeHoistablesForCompletedBoundary, writeHoistables, + writePreamble, writePostamble, - hoistBoundaryResources, - hoistHoistables, - createBoundaryResources, + hoistToBoundary, + hoistToRoot, createHoistableState, prepareHostDispatcher, supportsRequestStorage, @@ -211,10 +210,10 @@ type SuspenseBoundary = { parentFlushed: boolean, pendingTasks: number, // when it reaches zero we can show this boundary's content completedSegments: Array, // completed but not yet flushed segments. - fallbackHoistables: null | Hoistables, byteSize: number, // used to determine whether to inline children boundaries. fallbackAbortableTasks: Set, // used to cancel task on the fallback if the boundary completes or gets canceled. - resources: BoundaryResources, + contentState: HoistableState, + fallbackState: HoistableState, trackedContentKeyPath: null | KeyNode, // used to track the path for replay nodes trackedFallbackNode: null | ReplayNode, // used to track the fallback for replay nodes }; @@ -225,10 +224,8 @@ type RenderTask = { childIndex: number, ping: () => void, blockedBoundary: Root | SuspenseBoundary, - blockedBoundaryResources: null | BoundaryResources, // Conceptually part of the blockedBoundary but split out for performance blockedSegment: Segment, // the segment we'll write to - blockedHoistables: Hoistables, // the hoistables we'll write to - parentHoistables: null | Hoistables, + hoistableState: null | HoistableState, // Boundary state we'll mutate while rendering. This may not equal the state of the blockedBoundary abortSet: Set, // the abortable set that this task belongs to keyPath: Root | KeyNode, // the path of all parent keys currently rendering formatContext: FormatContext, // the format's specific context (e.g. HTML/SVG/MathML) @@ -253,10 +250,8 @@ type ReplayTask = { childIndex: number, ping: () => void, blockedBoundary: Root | SuspenseBoundary, - blockedBoundaryResources: null | BoundaryResources, // Conceptually part of the blockedBoundary but split out for performance blockedSegment: null, // we don't write to anything when we replay - blockedHoistables: Hoistables, // contains hoistable state for any child tasks that resume - parentHoistables: null | Hoistables, + hoistableState: null | HoistableState, // Boundary state we'll mutate while rendering. This may not equal the state of the blockedBoundary abortSet: Set, // the abortable set that this task belongs to keyPath: Root | KeyNode, // the path of all parent keys currently rendering formatContext: FormatContext, // the format's specific context (e.g. HTML/SVG/MathML) @@ -294,11 +289,6 @@ type Segment = { textEmbedded: boolean, }; -type Hoistables = { - state: HoistableState, - fallbacks: Set, -}; - const OPEN = 0; const CLOSING = 1; const CLOSED = 2; @@ -308,7 +298,6 @@ export opaque type Request = { flushScheduled: boolean, +resumableState: ResumableState, +renderState: RenderState, - +hoistables: Hoistables, +rootFormatContext: FormatContext, +progressiveChunkSize: number, status: 0 | 1 | 2, @@ -387,13 +376,11 @@ export function createRequest( prepareHostDispatcher(); const pingedTasks: Array = []; const abortSet: Set = new Set(); - const hoistables = createHoistables(); const request: Request = { destination: null, flushScheduled: false, resumableState, renderState, - hoistables, rootFormatContext, progressiveChunkSize: progressiveChunkSize === undefined @@ -438,7 +425,6 @@ export function createRequest( -1, null, rootSegment, - hoistables, null, abortSet, null, @@ -502,13 +488,11 @@ export function resumeRequest( prepareHostDispatcher(); const pingedTasks: Array = []; const abortSet: Set = new Set(); - const hoistables = createHoistables(); const request: Request = { destination: null, flushScheduled: false, resumableState: postponedState.resumableState, renderState, - hoistables, rootFormatContext: postponedState.rootFormatContext, progressiveChunkSize: postponedState.progressiveChunkSize, status: OPEN, @@ -553,7 +537,6 @@ export function resumeRequest( -1, null, rootSegment, - hoistables, null, abortSet, null, @@ -579,7 +562,6 @@ export function resumeRequest( children, -1, null, - hoistables, null, abortSet, null, @@ -616,7 +598,6 @@ function pingTask(request: Request, task: Task): void { function createSuspenseBoundary( request: Request, fallbackAbortableTasks: Set, - fallbackHoistables: null | Hoistables, ): SuspenseBoundary { return { status: PENDING, @@ -624,11 +605,11 @@ function createSuspenseBoundary( parentFlushed: false, pendingTasks: 0, completedSegments: [], - fallbackHoistables, byteSize: 0, fallbackAbortableTasks, errorDigest: null, - resources: createBoundaryResources(), + contentState: createHoistableState(), + fallbackState: createHoistableState(), trackedContentKeyPath: null, trackedFallbackNode: null, }; @@ -641,8 +622,7 @@ function createRenderTask( childIndex: number, blockedBoundary: Root | SuspenseBoundary, blockedSegment: Segment, - blockedHoistables: Hoistables, - parentHoistables: null | Hoistables, + hoistableState: null | HoistableState, abortSet: Set, keyPath: Root | KeyNode, formatContext: FormatContext, @@ -652,13 +632,10 @@ function createRenderTask( componentStack: null | ComponentStackNode, ): RenderTask { request.allPendingTasks++; - let blockedBoundaryResources; if (blockedBoundary === null) { request.pendingRootTasks++; - blockedBoundaryResources = null; } else { blockedBoundary.pendingTasks++; - blockedBoundaryResources = blockedBoundary.resources; } const task: RenderTask = { replay: null, @@ -666,10 +643,8 @@ function createRenderTask( childIndex, ping: () => pingTask(request, task), blockedBoundary, - blockedBoundaryResources, blockedSegment, - blockedHoistables, - parentHoistables, + hoistableState, abortSet, keyPath, formatContext, @@ -690,8 +665,7 @@ function createReplayTask( node: ReactNodeList, childIndex: number, blockedBoundary: Root | SuspenseBoundary, - blockedHoistables: Hoistables, - parentHoistables: null | Hoistables, + hoistableState: null | HoistableState, abortSet: Set, keyPath: Root | KeyNode, formatContext: FormatContext, @@ -701,13 +675,10 @@ function createReplayTask( componentStack: null | ComponentStackNode, ): ReplayTask { request.allPendingTasks++; - let blockedBoundaryResources; if (blockedBoundary === null) { request.pendingRootTasks++; - blockedBoundaryResources = null; } else { blockedBoundary.pendingTasks++; - blockedBoundaryResources = blockedBoundary.resources; } replay.pendingTasks++; const task: ReplayTask = { @@ -716,10 +687,8 @@ function createReplayTask( childIndex, ping: () => pingTask(request, task), blockedBoundary, - blockedBoundaryResources, blockedSegment: null, - blockedHoistables, - parentHoistables, + hoistableState, abortSet, keyPath, formatContext, @@ -755,13 +724,6 @@ function createPendingSegment( }; } -function createHoistables(): Hoistables { - return { - state: createHoistableState(), - fallbacks: new Set(), - }; -} - // DEV-only global reference to the currently executing task let currentTaskInDEV: null | Task = null; function getCurrentStackInDEV(): string { @@ -942,10 +904,8 @@ function renderSuspenseBoundary( const prevKeyPath = task.keyPath; const parentBoundary = task.blockedBoundary; - const parentBoundaryResources = task.blockedBoundaryResources; + const parentHoistableState = task.hoistableState; const parentSegment = task.blockedSegment; - const parentHoistables = task.blockedHoistables; - const grandParentHoistables = task.parentHoistables; // Each time we enter a suspense boundary, we split out into a new segment for // the fallback so that we can later replace that segment with the content. @@ -955,13 +915,7 @@ function renderSuspenseBoundary( const content: ReactNodeList = props.children; const fallbackAbortSet: Set = new Set(); - const fallbackHoistables = createHoistables(); - parentHoistables.fallbacks.add(fallbackHoistables); - const newBoundary = createSuspenseBoundary( - request, - fallbackAbortSet, - fallbackHoistables, - ); + const newBoundary = createSuspenseBoundary(request, fallbackAbortSet); if (request.trackedPostpones !== null) { newBoundary.trackedContentKeyPath = keyPath; } @@ -1003,10 +957,8 @@ function renderSuspenseBoundary( // context switching. We just need to temporarily switch which boundary and which segment // we're writing to. If something suspends, it'll spawn new suspended task with that context. task.blockedBoundary = newBoundary; - task.blockedBoundaryResources = newBoundary.resources; + task.hoistableState = newBoundary.contentState; task.blockedSegment = contentRootSegment; - task.blockedHoistables = createHoistables(); - task.parentHoistables = parentHoistables; task.keyPath = keyPath; try { @@ -1028,7 +980,6 @@ function renderSuspenseBoundary( // We are returning early so we need to restore the task.componentStack = previousComponentStack; - mergeHoistables.call(parentHoistables, task.blockedHoistables); return; } } catch (error: mixed) { @@ -1058,10 +1009,8 @@ function renderSuspenseBoundary( // We do need to fallthrough to create the fallback though. } finally { task.blockedBoundary = parentBoundary; - task.blockedBoundaryResources = parentBoundaryResources; + task.hoistableState = parentHoistableState; task.blockedSegment = parentSegment; - task.blockedHoistables = parentHoistables; - task.parentHoistables = grandParentHoistables; task.keyPath = prevKeyPath; task.componentStack = previousComponentStack; } @@ -1088,7 +1037,6 @@ function renderSuspenseBoundary( newBoundary.trackedFallbackNode = fallbackReplayNode; } } - // We create suspended task for the fallback because we don't want to actually work // on it yet in case we finish the main content, so we queue for later. const suspendedFallbackTask = createRenderTask( @@ -1098,8 +1046,7 @@ function renderSuspenseBoundary( -1, parentBoundary, boundarySegment, - fallbackHoistables, - null, + newBoundary.fallbackState, fallbackAbortSet, fallbackKeyPath, task.formatContext, @@ -1136,21 +1083,13 @@ function replaySuspenseBoundary( const previousReplaySet: ReplaySet = task.replay; const parentBoundary = task.blockedBoundary; - const parentBoundaryResources = task.blockedBoundaryResources; - const parentHoistables = task.blockedHoistables; - const grandParentHoistables = task.parentHoistables; + const parentHoistableState = task.hoistableState; const content: ReactNodeList = props.children; const fallback: ReactNodeList = props.fallback; const fallbackAbortSet: Set = new Set(); - const fallbackHoistables = createHoistables(); - parentHoistables.fallbacks.add(fallbackHoistables); - const resumedBoundary = createSuspenseBoundary( - request, - fallbackAbortSet, - fallbackHoistables, - ); + const resumedBoundary = createSuspenseBoundary(request, fallbackAbortSet); resumedBoundary.parentFlushed = true; // We restore the same id of this boundary as was used during prerender. resumedBoundary.rootSegmentID = id; @@ -1159,10 +1098,9 @@ function replaySuspenseBoundary( // context switching. We just need to temporarily switch which boundary and replay node // we're writing to. If something suspends, it'll spawn new suspended task with that context. task.blockedBoundary = resumedBoundary; - task.blockedBoundaryResources = resumedBoundary.resources; - task.blockedHoistables = createHoistables(); - task.parentHoistables = parentHoistables; + task.hoistableState = resumedBoundary.contentState; task.replay = {nodes: childNodes, slots: childSlots, pendingTasks: 1}; + try { // We use the safe form because we don't handle suspending here. Only error handling. renderNode(request, task, content, -1); @@ -1217,9 +1155,7 @@ function replaySuspenseBoundary( // We do need to fallthrough to create the fallback though. } finally { task.blockedBoundary = parentBoundary; - task.blockedBoundaryResources = parentBoundaryResources; - task.blockedHoistables = parentHoistables; - task.parentHoistables = grandParentHoistables; + task.hoistableState = parentHoistableState; task.replay = previousReplaySet; task.keyPath = prevKeyPath; task.componentStack = previousComponentStack; @@ -1241,8 +1177,7 @@ function replaySuspenseBoundary( fallback, -1, parentBoundary, - fallbackHoistables, - null, + resumedBoundary.fallbackState, fallbackAbortSet, fallbackKeyPath, task.formatContext, @@ -1312,15 +1247,13 @@ function renderHostElement( task.keyPath = prevKeyPath; } else { // Render - const hoistables = task.blockedHoistables; const children = pushStartInstance( segment.chunks, type, props, request.resumableState, request.renderState, - task.blockedBoundaryResources, - hoistables.state, + task.hoistableState, task.formatContext, segment.lastPushedText, ); @@ -2805,8 +2738,7 @@ function spawnNewSuspendedReplayTask( task.node, task.childIndex, task.blockedBoundary, - task.blockedHoistables, - task.parentHoistables, + task.hoistableState, task.abortSet, task.keyPath, task.formatContext, @@ -2851,8 +2783,7 @@ function spawnNewSuspendedRenderTask( task.childIndex, task.blockedBoundary, newSegment, - task.blockedHoistables, - task.parentHoistables, + task.hoistableState, task.abortSet, task.keyPath, task.formatContext, @@ -3130,13 +3061,11 @@ function abortTaskSoft(this: Request, task: Task): void { // It's used for when we didn't need this task to complete the tree. // If task was needed, then it should use abortTask instead. const request: Request = this; + const boundary = task.blockedBoundary; const segment = task.blockedSegment; if (segment !== null) { - const boundary = task.blockedBoundary; - const hoistables = task.blockedHoistables; - const parentHoistables = task.parentHoistables; segment.status = ABORTED; - finishedTask(request, boundary, segment, hoistables, parentHoistables); + finishedTask(request, boundary, segment); } } @@ -3147,7 +3076,7 @@ function abortRemainingSuspenseBoundary( errorDigest: ?string, errorInfo: ThrownInfo, ): void { - const resumedBoundary = createSuspenseBoundary(request, new Set(), null); + const resumedBoundary = createSuspenseBoundary(request, new Set()); resumedBoundary.parentFlushed = true; // We restore the same id of this boundary as was used during prerender. resumedBoundary.rootSegmentID = rootSegmentID; @@ -3395,31 +3324,10 @@ function queueCompletedSegment( } } -// Merges the internal state of Hoistables from a source to a target. afterwards -// both source and target will share the same unified state. This technique is used -// so we can resolve tasks in any order and always accumulate up to the root Hoistable. -// This function uses the `this` argument to allow for slightly optimized calling from -// a forEach over a Set. -function mergeHoistables(this: Hoistables, source: Hoistables) { - if (enableFloat) { - const target = this; - hoistHoistables(target.state, source.state); - source.fallbacks.forEach(h => { - target.fallbacks.add(h); - }); - // we assign the parent state and fallbacks to the child state so if a child task completes - // it will hoist and merge with the parent Hoistables - source.state = target.state; - source.fallbacks = target.fallbacks; - } -} - function finishedTask( request: Request, boundary: Root | SuspenseBoundary, segment: null | Segment, - hoistables: Hoistables, - parentHoistables: null | Hoistables, ) { if (boundary === null) { if (segment !== null && segment.parentFlushed) { @@ -3458,20 +3366,6 @@ function finishedTask( request.completedBoundaries.push(boundary); } - if (enableFloat && parentHoistables) { - // We have completed a boundary and need to merge this boundary's Hoistables with the parent - // Hoistables for this task. First we remove the fallback Hoistables. If they already flushed - // we can't do anythign about it but we don't want to flush them now if unflushed because the fallback - // will never show - if (boundary.fallbackHoistables) { - parentHoistables.fallbacks.delete(boundary.fallbackHoistables); - } - // Next we merge the boundary Hoistables into the task Hoistables. In the process the boundary assumes - // the task Hoistables internal state so later if a child task also completes it will merge with - // the appropriate sets - mergeHoistables.call(parentHoistables, hoistables); - } - // We can now cancel any pending task on the fallback since we won't need to show it anymore. // This needs to happen after we read the parentFlushed flags because aborting can finish // work which can trigger user code, which can start flushing, which can change those flags. @@ -3572,13 +3466,7 @@ function retryRenderTask( task.abortSet.delete(task); segment.status = COMPLETED; - finishedTask( - request, - task.blockedBoundary, - segment, - task.blockedHoistables, - task.parentHoistables, - ); + finishedTask(request, task.blockedBoundary, segment); } catch (thrownValue) { resetHooksState(); @@ -3619,13 +3507,7 @@ function retryRenderTask( const postponeInfo = getThrownInfo(request, task.componentStack); logPostpone(request, postponeInstance.message, postponeInfo); trackPostpone(request, trackedPostpones, task, segment); - finishedTask( - request, - task.blockedBoundary, - segment, - task.blockedHoistables, - task.parentHoistables, - ); + finishedTask(request, task.blockedBoundary, segment); return; } } @@ -3685,13 +3567,7 @@ function retryReplayTask(request: Request, task: ReplayTask): void { task.replay.pendingTasks--; task.abortSet.delete(task); - finishedTask( - request, - task.blockedBoundary, - null, - task.blockedHoistables, - task.parentHoistables, - ); + finishedTask(request, task.blockedBoundary, null); } catch (thrownValue) { resetHooksState(); @@ -3804,11 +3680,66 @@ export function performWork(request: Request): void { } } +function preparePreambleForSubtree(request: Request, segment: Segment): void { + if (segment.status === COMPLETED) { + const children = segment.children; + for (let i = 0; i < children.length; i++) { + const child = children[i]; + prepareSegmentForPreamble(request, child); + } + } +} + +function prepareSegmentForPreamble(request: Request, segment: Segment): void { + const boundary = segment.boundary; + if (boundary === null) { + // Not a suspense boundary. + preparePreambleForSubtree(request, segment); + } else { + if (boundary.status === COMPLETED) { + // we are going to flush this boundary's primary content rather than a fallback + hoistToRoot(request.renderState, boundary.contentState); + // Traverse down the primary content path. + const completedSegments = boundary.completedSegments; + const contentSegment = completedSegments[0]; + if (contentSegment) { + // It is an invariant that a previously unvisited boundary have a single root + // segment however we know this will be caught in the normal flushing path + // so we simply guard the condition here and avoid throwing + preparePreambleForSubtree(request, segment); + } + } else { + // We are going to flush this boundary's fallback content and should include + // it's hoistables as well + hoistToRoot(request.renderState, boundary.fallbackState); + // Traverse the fallback path and see if there are any deeper boundaries with hoistables + // to collect + preparePreambleForSubtree(request, segment); + } + } +} + +function flushPreamble( + request: Request, + destination: Destination, + rootSegment: Segment, +) { + prepareSegmentForPreamble(request, rootSegment); + const willFlushAllSegments = + request.allPendingTasks === 0 && request.trackedPostpones === null; + writePreamble( + destination, + request.resumableState, + request.renderState, + willFlushAllSegments, + ); +} + function flushSubtree( request: Request, destination: Destination, segment: Segment, - rootBoundary: null | SuspenseBoundary, + hoistableState: null | HoistableState, ): boolean { segment.parentFlushed = true; switch (segment.status) { @@ -3838,7 +3769,7 @@ function flushSubtree( for (; chunkIdx < nextChild.index; chunkIdx++) { writeChunk(destination, chunks[chunkIdx]); } - r = flushSegment(request, destination, nextChild, rootBoundary); + r = flushSegment(request, destination, nextChild, hoistableState); } // Finally just write all the remaining chunks for (; chunkIdx < chunks.length - 1; chunkIdx++) { @@ -3861,12 +3792,12 @@ function flushSegment( request: Request, destination: Destination, segment: Segment, - rootBoundary: null | SuspenseBoundary, + hoistableState: null | HoistableState, ): boolean { const boundary = segment.boundary; if (boundary === null) { // Not a suspense boundary. - return flushSubtree(request, destination, segment, rootBoundary); + return flushSubtree(request, destination, segment, hoistableState); } boundary.parentFlushed = true; @@ -3884,7 +3815,7 @@ function flushSegment( boundary.errorComponentStack, ); // Flush the fallback. - flushSubtree(request, destination, segment, rootBoundary); + flushSubtree(request, destination, segment, hoistableState); return writeEndClientRenderedSuspenseBoundary( destination, @@ -3907,8 +3838,15 @@ function flushSegment( const id = boundary.rootSegmentID; writeStartPendingSuspenseBoundary(destination, request.renderState, id); + // We are going to flush the fallback so we need to hoist the fallback + // state to the parent boundary + if (enableFloat) { + if (hoistableState) { + hoistToBoundary(hoistableState, boundary.fallbackState); + } + } // Flush the fallback. - flushSubtree(request, destination, segment, rootBoundary); + flushSubtree(request, destination, segment, hoistableState); return writeEndPendingSuspenseBoundary(destination, request.renderState); } else if (boundary.byteSize > request.progressiveChunkSize) { @@ -3929,13 +3867,20 @@ function flushSegment( boundary.rootSegmentID, ); + // While we are going to flush the fallback we are going to follow it up with + // the completed boundary immediately so we make the choice to omit fallback + // boundary state from the parent since it will be replaced when the boundary + // flushes later in this pass or in a future flush + // Flush the fallback. - flushSubtree(request, destination, segment, rootBoundary); + flushSubtree(request, destination, segment, hoistableState); return writeEndPendingSuspenseBoundary(destination, request.renderState); } else { - if (enableFloat && rootBoundary && rootBoundary !== boundary) { - hoistBoundaryResources(rootBoundary.resources, boundary.resources); + if (enableFloat) { + if (hoistableState) { + hoistToBoundary(hoistableState, boundary.contentState); + } } // We can inline this boundary's content as a complete boundary. writeStartCompletedSuspenseBoundary(destination, request.renderState); @@ -3949,7 +3894,7 @@ function flushSegment( } const contentSegment = completedSegments[0]; - flushSegment(request, destination, contentSegment, rootBoundary); + flushSegment(request, destination, contentSegment, hoistableState); return writeEndCompletedSuspenseBoundary(destination, request.renderState); } @@ -3975,7 +3920,7 @@ function flushSegmentContainer( request: Request, destination: Destination, segment: Segment, - boundary: SuspenseBoundary, + hoistableState: HoistableState, ): boolean { writeStartSegment( destination, @@ -3983,7 +3928,7 @@ function flushSegmentContainer( segment.parentFormatContext, segment.id, ); - flushSegment(request, destination, segment, boundary); + flushSegment(request, destination, segment, hoistableState); return writeEndSegment(destination, segment.parentFormatContext); } @@ -4001,9 +3946,9 @@ function flushCompletedBoundary( completedSegments.length = 0; if (enableFloat) { - writeResourcesForBoundary( + writeHoistablesForCompletedBoundary( destination, - boundary.resources, + boundary.contentState, request.renderState, ); } @@ -4013,7 +3958,7 @@ function flushCompletedBoundary( request.resumableState, request.renderState, boundary.rootSegmentID, - boundary.resources, + boundary.contentState, ); } @@ -4039,13 +3984,9 @@ function flushPartialBoundary( completedSegments.splice(0, i); if (enableFloat) { - // The way this is structured we only write resources for partial boundaries - // if there is no backpressure. Later before we complete the boundary we - // will write resources regardless of backpressure before we emit the - // completion instruction - return writeResourcesForBoundary( + return writeHoistablesForPartialBoundary( destination, - boundary.resources, + boundary.contentState, request.renderState, ); } else { @@ -4064,6 +4005,8 @@ function flushPartiallyCompletedSegment( return true; } + const hoistableState = boundary.contentState; + const segmentID = segment.id; if (segmentID === -1) { // This segment wasn't previously referred to. This happens at the root of @@ -4076,13 +4019,13 @@ function flushPartiallyCompletedSegment( ); } - return flushSegmentContainer(request, destination, segment, boundary); + return flushSegmentContainer(request, destination, segment, hoistableState); } else if (segmentID === boundary.rootSegmentID) { // When we emit postponed boundaries, we might have assigned the ID already // but it's still the root segment so we can't inject it into the parent yet. - return flushSegmentContainer(request, destination, segment, boundary); + return flushSegmentContainer(request, destination, segment, hoistableState); } else { - flushSegmentContainer(request, destination, segment, boundary); + flushSegmentContainer(request, destination, segment, hoistableState); return writeCompletedSegmentInstruction( destination, request.resumableState, @@ -4092,17 +4035,6 @@ function flushPartiallyCompletedSegment( } } -function prepareToFlushHoistables(request: Request) { - // At the moment we flush we merge all fallback Hoistables visible to the request's Hoistables - // object. These represent hoistables for Boundaries that will flush a fallback because the - // primary content isn't ready yet. If a boundary completes before this step then the fallback - // Hoistables would have already been removed from this set so we know it only includes necessary - // fallback hoistables - const requestHoistables = request.hoistables; - requestHoistables.fallbacks.forEach(mergeHoistables, requestHoistables); - requestHoistables.fallbacks.clear(); -} - function flushCompletedQueues( request: Request, destination: Destination, @@ -4122,14 +4054,7 @@ function flushCompletedQueues( return; } else if (request.pendingRootTasks === 0) { if (enableFloat) { - prepareToFlushHoistables(request); - writePreamble( - destination, - request.resumableState, - request.renderState, - request.hoistables.state, - request.allPendingTasks === 0 && request.trackedPostpones === null, - ); + flushPreamble(request, destination, completedRootSegment); } flushSegment(request, destination, completedRootSegment, null); @@ -4140,15 +4065,8 @@ function flushCompletedQueues( return; } } - if (enableFloat) { - prepareToFlushHoistables(request); - writeHoistables( - destination, - request.resumableState, - request.renderState, - request.hoistables.state, - ); + writeHoistables(destination, request.resumableState, request.renderState); } // We emit client rendering instructions for already emitted boundaries first. diff --git a/packages/react-server/src/forks/ReactFizzConfig.custom.js b/packages/react-server/src/forks/ReactFizzConfig.custom.js index 7a2c256fc025f..e113500a4bf54 100644 --- a/packages/react-server/src/forks/ReactFizzConfig.custom.js +++ b/packages/react-server/src/forks/ReactFizzConfig.custom.js @@ -31,7 +31,6 @@ export opaque type Destination = mixed; // eslint-disable-line no-undef export opaque type RenderState = mixed; export opaque type HoistableState = mixed; export opaque type ResumableState = mixed; -export opaque type BoundaryResources = mixed; export opaque type FormatContext = mixed; export opaque type HeadersDescriptor = mixed; export type {TransitionStatus}; @@ -88,9 +87,11 @@ export const NotPendingTransition = $$$config.NotPendingTransition; export const writePreamble = $$$config.writePreamble; export const writeHoistables = $$$config.writeHoistables; export const writePostamble = $$$config.writePostamble; -export const hoistBoundaryResources = $$$config.hoistBoundaryResources; -export const hoistHoistables = $$$config.hoistHoistables; +export const hoistToBoundary = $$$config.hoistToBoundary; +export const hoistToRoot = $$$config.hoistToRoot; export const createHoistableState = $$$config.createHoistableState; -export const createBoundaryResources = $$$config.createBoundaryResources; -export const writeResourcesForBoundary = $$$config.writeResourcesForBoundary; +export const writeHoistablesForPartialBoundary = + $$$config.writeHoistablesForPartialBoundary; +export const writeHoistablesForCompletedBoundary = + $$$config.writeHoistablesForCompletedBoundary; export const emitEarlyPreloads = $$$config.emitEarlyPreloads; From eed492ee5aaa90c499fe41df05dbfb856b19da0f Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 24 Jan 2024 15:09:52 -0800 Subject: [PATCH 3/3] Updates the approach to emit hoistables eagerly but only if the hoistable is not in a fallback tree. Effectively it makes hoistable elements deep in any fallback noops. The logic here is that fallbacks aren't hydrated, they're always either replaced by the server or client rendered. In either case the primary content should be replacing the fallback hositables and since hoistable elements are really just metadata the imperative need to transiently render them is not as well justified. This avoids a host of complexities around deleting hoistables from fallbacks when the boundary reveals it's content. Along with this change I made it so primary content hositables flush eagerly and do not wait for their containing boundary to complete first. This is a progmatic solution to the problem of prerendering. when prerendering we assume that all transent state is entirely flushed in the static prelude however by holding back hoistables until the boundary is complete this is violated. We would either need to conditionally emit hoistables for incomplete boundaries when prerending or we would need to serialize the state of the hoistables to recover them on the resuem path. The arguments for holding them back are that it aligns with client hoistable semantics better and if the boundary never hydrates the hoistables will get orphaned. Both of these problems are not insignificant but are also not necessarily blockers and so this approach attempts to balance complexity over impact by emitting them eagerly. --- .../src/server/ReactFizzConfigDOM.js | 209 ++++++----------- .../src/server/ReactFizzConfigDOMLegacy.js | 6 +- .../src/__tests__/ReactDOMFloat-test.js | 211 ++++++++++++------ .../src/ReactNoopServer.js | 6 +- packages/react-server/src/ReactFizzServer.js | 68 ++---- .../src/forks/ReactFizzConfig.custom.js | 8 +- 6 files changed, 242 insertions(+), 266 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index c5eeaa5809b78..ae409cdeed554 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -2222,10 +2222,10 @@ function pushMeta( target: Array, props: Object, renderState: RenderState, - hoistableState: null | HoistableState, textEmbedded: boolean, insertionMode: InsertionMode, noscriptTagInScope: boolean, + isFallback: boolean, ): null { if (enableFloat) { if ( @@ -2241,31 +2241,26 @@ function pushMeta( target.push(textSeparator); } - if (typeof props.charSet === 'string') { - return pushSelfClosing( - hoistableState - ? hoistableState.charsetChunks - : renderState.charsetChunks, - props, - 'meta', - ); + if (isFallback) { + // Hoistable Elements for fallbacks are simply omitted. we don't want to emit them early + // because they are likely superceded by primary content and we want to avoid needing to clean + // them up when the primary content is ready. They are never hydrated on the client anyway because + // boundaries in fallback are awaited or client render, in either case there is never hydration + return null; + } else if (typeof props.charSet === 'string') { + // "charset" Should really be config and not picked up from tags however since this is + // the only way to embed the tag today we flush it on a special queue on the Request so it + // can go before everything else. Like viewport this means that the tag will escape it's + // parent container. + return pushSelfClosing(renderState.charsetChunks, props, 'meta'); } else if (props.name === 'viewport') { - // "viewport" isn't related to preconnect but it has the right priority - return pushSelfClosing( - hoistableState - ? hoistableState.viewportChunks - : renderState.viewportChunks, - props, - 'meta', - ); + // "viewport" is flushed on the Request so it can go earlier that Float resources that + // might be affected by it. This means it can escape the boundary it is rendered within. + // This is a pragmatic solution to viewport being incredibly sensitive to document order + // without requiring all hoistables to be flushed too early. + return pushSelfClosing(renderState.viewportChunks, props, 'meta'); } else { - return pushSelfClosing( - hoistableState - ? hoistableState.hoistableChunks - : renderState.hoistableChunks, - props, - 'meta', - ); + return pushSelfClosing(renderState.hoistableChunks, props, 'meta'); } } } else { @@ -2282,6 +2277,7 @@ function pushLink( textEmbedded: boolean, insertionMode: InsertionMode, noscriptTagInScope: boolean, + isFallback: boolean, ): null { if (enableFloat) { const rel = props.rel; @@ -2437,10 +2433,15 @@ function pushLink( target.push(textSeparator); } - const hoistableChunks = hoistableState - ? hoistableState.hoistableChunks - : renderState.hoistableChunks; - return pushLinkImpl(hoistableChunks, props); + if (isFallback) { + // Hoistable Elements for fallbacks are simply omitted. we don't want to emit them early + // because they are likely superceded by primary content and we want to avoid needing to clean + // them up when the primary content is ready. They are never hydrated on the client anyway because + // boundaries in fallback are awaited or client render, in either case there is never hydration + return null; + } else { + return pushLinkImpl(renderState.hoistableChunks, props); + } } } else { return pushLinkImpl(target, props); @@ -2894,9 +2895,9 @@ function pushTitle( target: Array, props: Object, renderState: RenderState, - hoistableState: null | HoistableState, insertionMode: InsertionMode, noscriptTagInScope: boolean, + isFallback: boolean, ): ReactNodeList { if (__DEV__) { if (hasOwnProperty.call(props, 'children')) { @@ -2952,11 +2953,15 @@ function pushTitle( !noscriptTagInScope && props.itemProp == null ) { - const hoistableTarget = hoistableState - ? hoistableState.hoistableChunks - : renderState.hoistableChunks; - pushTitleImpl(hoistableTarget, props); - return null; + if (isFallback) { + // Hoistable Elements for fallbacks are simply omitted. we don't want to emit them early + // because they are likely superceded by primary content and we want to avoid needing to clean + // them up when the primary content is ready. They are never hydrated on the client anyway because + // boundaries in fallback are awaited or client render, in either case there is never hydration + return null; + } else { + pushTitleImpl(renderState.hoistableChunks, props); + } } else { return pushTitleImpl(target, props); } @@ -3490,6 +3495,7 @@ export function pushStartInstance( hoistableState: null | HoistableState, formatContext: FormatContext, textEmbedded: boolean, + isFallback: boolean, ): ReactNodeList { if (__DEV__) { validateARIAProperties(type, props); @@ -3556,9 +3562,9 @@ export function pushStartInstance( target, props, renderState, - hoistableState, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), + isFallback, ) : pushStartTitle(target, props); case 'link': @@ -3571,6 +3577,7 @@ export function pushStartInstance( textEmbedded, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), + isFallback, ); case 'script': return enableFloat @@ -3600,10 +3607,10 @@ export function pushStartInstance( target, props, renderState, - hoistableState, textEmbedded, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), + isFallback, ); // Newline eating tags case 'listing': @@ -4469,7 +4476,7 @@ function hasStylesToHoist(stylesheet: StylesheetResource): boolean { return false; } -export function writeHoistablesForPartialBoundary( +export function writeHoistablesForBoundary( destination: Destination, hoistableState: HoistableState, renderState: RenderState, @@ -4495,57 +4502,6 @@ export function writeHoistablesForPartialBoundary( return destinationHasCapacity; } -export function writeHoistablesForCompletedBoundary( - destination: Destination, - hoistableState: HoistableState, - renderState: RenderState, -): boolean { - // Reset these on each invocation, they are only safe to read in this function - currentlyRenderingBoundaryHasStylesToHoist = false; - destinationHasCapacity = true; - - // Flush style tags for each precedence this boundary depends on - hoistableState.styles.forEach(flushStyleTagsLateForBoundary, destination); - - // Determine if this boundary has stylesheets that need to be awaited upon completion - hoistableState.stylesheets.forEach(hasStylesToHoist); - - // Flush Hoistable Elements - let i; - const charsetChunks = hoistableState.charsetChunks; - for (i = 0; i < charsetChunks.length - 1; i++) { - writeChunk(destination, charsetChunks[i]); - } - if (i < charsetChunks.length) { - destinationHasCapacity = writeChunkAndReturn(destination, charsetChunks[i]); - } - const viewportChunks = hoistableState.viewportChunks; - for (i = 0; i < viewportChunks.length - 1; i++) { - writeChunk(destination, charsetChunks[i]); - } - if (i < viewportChunks.length) { - destinationHasCapacity = writeChunkAndReturn( - destination, - viewportChunks[i], - ); - } - const hoistableChunks = hoistableState.hoistableChunks; - for (i = 0; i < hoistableChunks.length - 1; i++) { - writeChunk(destination, hoistableChunks[i]); - } - if (i < hoistableChunks.length) { - destinationHasCapacity = writeChunkAndReturn( - destination, - hoistableChunks[i], - ); - } - - if (currentlyRenderingBoundaryHasStylesToHoist) { - renderState.stylesToHoist = true; - } - return destinationHasCapacity; -} - function flushResource(this: Destination, resource: Resource) { for (let i = 0; i < resource.length; i++) { writeChunk(this, resource[i]); @@ -4741,26 +4697,35 @@ export function writePreamble( } hoistableChunks.length = 0; - // Flush closing head if necessary if (htmlChunks && headChunks === null) { - // We have an rendered but no rendered. We however inserted - // a up above so we need to emit the now. This is safe because - // if the main content contained the it would also have provided a - // . This means that all the content inside is either or - // invalid HTML + // we have an but we inserted an implicit tag. We need + // to close it since the main content won't have it writeChunk(destination, endChunkForTag('head')); } } -// This is an opportunity to write hoistables however in the current implemention -// the only hoistables that make sense to write here are Resources. Hoistable Elements -// would have already been written as part of the preamble or will be written as part -// of a boundary completion and thus don't need to be written here. +// We don't bother reporting backpressure at the moment because we expect to +// flush the entire preamble in a single pass. This probably should be modified +// in the future to be backpressure sensitive but that requires a larger refactor +// of the flushing code in Fizz. export function writeHoistables( destination: Destination, resumableState: ResumableState, renderState: RenderState, ): void { + let i = 0; + + // Emit high priority Hoistables + + // We omit charsetChunks because we have already sent the shell and if it wasn't + // already sent it is too late now. + + const viewportChunks = renderState.viewportChunks; + for (i = 0; i < viewportChunks.length; i++) { + writeChunk(destination, viewportChunks[i]); + } + viewportChunks.length = 0; + renderState.preconnects.forEach(flushResource, destination); renderState.preconnects.clear(); @@ -4787,6 +4752,13 @@ export function writeHoistables( renderState.bulkPreloads.forEach(flushResource, destination); renderState.bulkPreloads.clear(); + + // Write embedding hoistableChunks + const hoistableChunks = renderState.hoistableChunks; + for (i = 0; i < hoistableChunks.length; i++) { + writeChunk(destination, hoistableChunks[i]); + } + hoistableChunks.length = 0; } export function writePostamble( @@ -5259,10 +5231,6 @@ type StylesheetResource = { export type HoistableState = { styles: Set, stylesheets: Set, - // Hoistable chunks - charsetChunks: Array, - viewportChunks: Array, - hoistableChunks: Array, }; export type StyleQueue = { @@ -5276,9 +5244,6 @@ export function createHoistableState(): HoistableState { return { styles: new Set(), stylesheets: new Set(), - charsetChunks: [], - viewportChunks: [], - hoistableChunks: [], }; } @@ -6142,44 +6107,12 @@ function hoistStylesheetDependency( this.stylesheets.add(stylesheet); } -export function hoistToBoundary( +export function hoistHoistables( parentState: HoistableState, childState: HoistableState, ): void { childState.styles.forEach(hoistStyleQueueDependency, parentState); childState.stylesheets.forEach(hoistStylesheetDependency, parentState); - let i; - const charsetChunks = childState.charsetChunks; - for (i = 0; i < charsetChunks.length; i++) { - parentState.charsetChunks.push(charsetChunks[i]); - } - const viewportChunks = childState.viewportChunks; - for (i = 0; i < charsetChunks.length; i++) { - parentState.viewportChunks.push(viewportChunks[i]); - } - const hoistableChunks = childState.hoistableChunks; - for (i = 0; i < hoistableChunks.length; i++) { - parentState.hoistableChunks.push(hoistableChunks[i]); - } -} - -export function hoistToRoot( - renderState: RenderState, - hoistableState: HoistableState, -): void { - let i; - const charsetChunks = hoistableState.charsetChunks; - for (i = 0; i < charsetChunks.length; i++) { - renderState.charsetChunks.push(charsetChunks[i]); - } - const viewportChunks = hoistableState.viewportChunks; - for (i = 0; i < charsetChunks.length; i++) { - renderState.viewportChunks.push(viewportChunks[i]); - } - const hoistableChunks = hoistableState.hoistableChunks; - for (i = 0; i < hoistableChunks.length; i++) { - renderState.hoistableChunks.push(hoistableChunks[i]); - } } // This function is called at various times depending on whether we are rendering diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js index 35554654cd07f..a8f38e283cefc 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js @@ -153,8 +153,7 @@ export { writeClientRenderBoundaryInstruction, writeStartPendingSuspenseBoundary, writeEndPendingSuspenseBoundary, - writeHoistablesForPartialBoundary, - writeHoistablesForCompletedBoundary, + writeHoistablesForBoundary, writePlaceholder, writeCompletedRoot, createRootFormatContext, @@ -163,8 +162,7 @@ export { writePreamble, writeHoistables, writePostamble, - hoistToBoundary, - hoistToRoot, + hoistHoistables, prepareHostDispatcher, resetResumableState, completeResumableState, diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index cb48446087912..eaad571ad9759 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -4719,7 +4719,7 @@ body { ); }); - it('does not flush hoistables for fallbacks unless the fallback also flushes', async () => { + it('does not flush hoistables for fallbacks', async () => { function App() { return ( @@ -4729,6 +4729,7 @@ body { <>
fallback1
+ foo }> <> @@ -4741,12 +4742,14 @@ body { <>
fallback2
+ }> <>
primary2
- - + + +
fallback3
+ +
deep fallback ... primary content
+ + }> <>
primary3
- - + + + @@ -4777,12 +4785,12 @@ body { -
primary1
primary2
fallback3
+
deep fallback ... primary content
, ); @@ -4796,7 +4804,6 @@ body { -
primary1
@@ -4846,9 +4853,12 @@ body { expect(getMeaningfulChildren(document)).toEqual( - + {/* The primary content hoistables emit */} + + {/* The fallback content emits but the hoistables do not even if they + inside a nested suspense boundary that is resolved */}
nested primary1
, @@ -4861,63 +4871,10 @@ body { expect(getMeaningfulChildren(document)).toEqual( - +
primary1
- - - , - ); - }); - - it('avoid flushing hoistables of completed inner boundaries when inside an incomplete outer boundary', async () => { - function App() { - return ( - - - - -
blocked outer
- - -
outer
- - -
inner
- - - - - - ); - } - - await act(() => { - renderToPipeableStream().pipe(writable); - }); - - expect(getMeaningfulChildren(document)).toEqual( - - - - , - ); - - await act(() => { - resolveText('release'); - }); - - expect(getMeaningfulChildren(document)).toEqual( - - - -
blocked outer
-
outer
-
inner
- - - , ); @@ -4990,6 +4947,120 @@ body { }); }); + it('does not wait for stylesheets of completed fallbacks', async () => { + function Unblock({value}) { + resolveText(value); + return null; + } + function App() { + return ( + + + +
hello world
+ + + +
inner fallback
+ + + }> + +
inner boundary
+ + + +
inner blocked fallback
+ + }> + +
inner blocked boundary
+ + + + + + + ); + } + + await act(() => { + renderToPipeableStream().pipe(writable); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + loading... + , + ); + + await act(async () => { + resolveText('unblock inner boundaries'); + }); + expect(getMeaningfulChildren(document)).toEqual( + + + + loading... + + + + , + ); + + await act(() => { + resolveText('completed inner'); + }); + expect(getMeaningfulChildren(document)).toEqual( + + + + loading... + + + + , + ); + + await act(() => { + resolveText('complete root'); + }); + await act(() => { + loadStylesheets(); + }); + expect(getMeaningfulChildren(document)).toEqual( + + + + + +
hello world
+
inner boundary
+
inner blocked fallback
+ + + + , + ); + }); + describe('ReactDOM.preconnect(href, { crossOrigin })', () => { it('creates a preconnect resource when called', async () => { function App({url}) { @@ -8025,6 +8096,10 @@ background-color: green; + + + + loading... , @@ -8039,6 +8114,10 @@ background-color: green; + + + + loading... , @@ -8065,15 +8144,15 @@ background-color: green; + + + +
hello world
- - - - diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index ae508385ba9b4..4252195f81c1e 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -265,14 +265,12 @@ const ReactNoopServer = ReactFizzServer({ writePreamble() {}, writeHoistables() {}, + writeHoistablesForBoundary() {}, writePostamble() {}, - hoistToRoot(renderState: RenderState, hoistableState: HoistableState) {}, - hoistToBoundary(parent: HoistableState, child: HoistableState) {}, + hoistHoistables(parent: HoistableState, child: HoistableState) {}, createHoistableState(): HoistableState { return null; }, - writeHoistablesForPartialBoundary() {}, - writeHoistablesForCompleteBoundary() {}, emitEarlyPreloads() {}, }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index ec59a382d3b5d..2c9d83e680fb9 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -57,6 +57,7 @@ import { writeClientRenderBoundaryInstruction, writeCompletedBoundaryInstruction, writeCompletedSegmentInstruction, + writeHoistablesForBoundary, pushTextInstance, pushStartInstance, pushEndInstance, @@ -64,13 +65,10 @@ import { pushEndCompletedSuspenseBoundary, pushSegmentFinale, getChildFormatContext, - writeHoistablesForPartialBoundary, - writeHoistablesForCompletedBoundary, writeHoistables, writePreamble, writePostamble, - hoistToBoundary, - hoistToRoot, + hoistHoistables, createHoistableState, prepareHostDispatcher, supportsRequestStorage, @@ -234,6 +232,7 @@ type RenderTask = { treeContext: TreeContext, // the current tree context that this task is executing in componentStack: null | ComponentStackNode, // stack frame description of the currently rendering component thenableState: null | ThenableState, + isFallback: boolean, // whether this task is rendering inside a fallback tree }; type ReplaySet = { @@ -260,6 +259,7 @@ type ReplayTask = { treeContext: TreeContext, // the current tree context that this task is executing in componentStack: null | ComponentStackNode, // stack frame description of the currently rendering component thenableState: null | ThenableState, + isFallback: boolean, // whether this task is rendering inside a fallback tree }; export type Task = RenderTask | ReplayTask; @@ -433,6 +433,7 @@ export function createRequest( rootContextSnapshot, emptyTreeContext, null, + false, ); pingedTasks.push(rootTask); return request; @@ -545,6 +546,7 @@ export function resumeRequest( rootContextSnapshot, emptyTreeContext, null, + false, ); pingedTasks.push(rootTask); return request; @@ -570,6 +572,7 @@ export function resumeRequest( rootContextSnapshot, emptyTreeContext, null, + false, ); pingedTasks.push(rootTask); return request; @@ -630,6 +633,7 @@ function createRenderTask( context: ContextSnapshot, treeContext: TreeContext, componentStack: null | ComponentStackNode, + isFallback: boolean, ): RenderTask { request.allPendingTasks++; if (blockedBoundary === null) { @@ -653,6 +657,7 @@ function createRenderTask( treeContext, componentStack, thenableState, + isFallback, }; abortSet.add(task); return task; @@ -673,6 +678,7 @@ function createReplayTask( context: ContextSnapshot, treeContext: TreeContext, componentStack: null | ComponentStackNode, + isFallback: boolean, ): ReplayTask { request.allPendingTasks++; if (blockedBoundary === null) { @@ -697,6 +703,7 @@ function createReplayTask( treeContext, componentStack, thenableState, + isFallback, }; abortSet.add(task); return task; @@ -1056,6 +1063,7 @@ function renderSuspenseBoundary( // This stack should be the Suspense boundary stack because while the fallback is actually a child segment // of the parent boundary from a component standpoint the fallback is a child of the Suspense boundary itself suspenseComponentStack, + true, ); // TODO: This should be queued at a separate lower priority queue so that we only work // on preparing fallbacks if we don't have any more main content to task on. @@ -1187,6 +1195,7 @@ function replaySuspenseBoundary( // This stack should be the Suspense boundary stack because while the fallback is actually a child segment // of the parent boundary from a component standpoint the fallback is a child of the Suspense boundary itself suspenseComponentStack, + true, ); // TODO: This should be queued at a separate lower priority queue so that we only work // on preparing fallbacks if we don't have any more main content to task on. @@ -1256,6 +1265,7 @@ function renderHostElement( task.hoistableState, task.formatContext, segment.lastPushedText, + task.isFallback, ); segment.lastPushedText = false; const prevContext = task.formatContext; @@ -2748,6 +2758,7 @@ function spawnNewSuspendedReplayTask( // We pop one task off the stack because the node that suspended will be tried again, // which will add it back onto the stack. task.componentStack !== null ? task.componentStack.parent : null, + task.isFallback, ); const ping = newTask.ping; @@ -2793,6 +2804,7 @@ function spawnNewSuspendedRenderTask( // We pop one task off the stack because the node that suspended will be tried again, // which will add it back onto the stack. task.componentStack !== null ? task.componentStack.parent : null, + task.isFallback, ); const ping = newTask.ping; @@ -3680,51 +3692,11 @@ export function performWork(request: Request): void { } } -function preparePreambleForSubtree(request: Request, segment: Segment): void { - if (segment.status === COMPLETED) { - const children = segment.children; - for (let i = 0; i < children.length; i++) { - const child = children[i]; - prepareSegmentForPreamble(request, child); - } - } -} - -function prepareSegmentForPreamble(request: Request, segment: Segment): void { - const boundary = segment.boundary; - if (boundary === null) { - // Not a suspense boundary. - preparePreambleForSubtree(request, segment); - } else { - if (boundary.status === COMPLETED) { - // we are going to flush this boundary's primary content rather than a fallback - hoistToRoot(request.renderState, boundary.contentState); - // Traverse down the primary content path. - const completedSegments = boundary.completedSegments; - const contentSegment = completedSegments[0]; - if (contentSegment) { - // It is an invariant that a previously unvisited boundary have a single root - // segment however we know this will be caught in the normal flushing path - // so we simply guard the condition here and avoid throwing - preparePreambleForSubtree(request, segment); - } - } else { - // We are going to flush this boundary's fallback content and should include - // it's hoistables as well - hoistToRoot(request.renderState, boundary.fallbackState); - // Traverse the fallback path and see if there are any deeper boundaries with hoistables - // to collect - preparePreambleForSubtree(request, segment); - } - } -} - function flushPreamble( request: Request, destination: Destination, rootSegment: Segment, ) { - prepareSegmentForPreamble(request, rootSegment); const willFlushAllSegments = request.allPendingTasks === 0 && request.trackedPostpones === null; writePreamble( @@ -3842,7 +3814,7 @@ function flushSegment( // state to the parent boundary if (enableFloat) { if (hoistableState) { - hoistToBoundary(hoistableState, boundary.fallbackState); + hoistHoistables(hoistableState, boundary.fallbackState); } } // Flush the fallback. @@ -3879,7 +3851,7 @@ function flushSegment( } else { if (enableFloat) { if (hoistableState) { - hoistToBoundary(hoistableState, boundary.contentState); + hoistHoistables(hoistableState, boundary.contentState); } } // We can inline this boundary's content as a complete boundary. @@ -3946,7 +3918,7 @@ function flushCompletedBoundary( completedSegments.length = 0; if (enableFloat) { - writeHoistablesForCompletedBoundary( + writeHoistablesForBoundary( destination, boundary.contentState, request.renderState, @@ -3984,7 +3956,7 @@ function flushPartialBoundary( completedSegments.splice(0, i); if (enableFloat) { - return writeHoistablesForPartialBoundary( + return writeHoistablesForBoundary( destination, boundary.contentState, request.renderState, diff --git a/packages/react-server/src/forks/ReactFizzConfig.custom.js b/packages/react-server/src/forks/ReactFizzConfig.custom.js index e113500a4bf54..eb76985c49218 100644 --- a/packages/react-server/src/forks/ReactFizzConfig.custom.js +++ b/packages/react-server/src/forks/ReactFizzConfig.custom.js @@ -86,12 +86,8 @@ export const NotPendingTransition = $$$config.NotPendingTransition; // ------------------------- export const writePreamble = $$$config.writePreamble; export const writeHoistables = $$$config.writeHoistables; +export const writeHoistablesForBoundary = $$$config.writeHoistablesForBoundary; export const writePostamble = $$$config.writePostamble; -export const hoistToBoundary = $$$config.hoistToBoundary; -export const hoistToRoot = $$$config.hoistToRoot; +export const hoistHoistables = $$$config.hoistHoistables; export const createHoistableState = $$$config.createHoistableState; -export const writeHoistablesForPartialBoundary = - $$$config.writeHoistablesForPartialBoundary; -export const writeHoistablesForCompletedBoundary = - $$$config.writeHoistablesForCompletedBoundary; export const emitEarlyPreloads = $$$config.emitEarlyPreloads;