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..3a70e147e7ea9 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
- - - , ); @@ -8025,6 +7982,10 @@ background-color: green; + + + + loading... , @@ -8039,6 +8000,10 @@ background-color: green; + + + + loading... , @@ -8065,15 +8030,15 @@ background-color: green; + + + +
hello world
- - - - 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,