From 554fc49f41465d914b15dc8eb2ec094f37824f7e Mon Sep 17 00:00:00 2001 From: Josh Story Date: Tue, 30 Jan 2024 10:14:59 -0800 Subject: [PATCH] [Fizz] improve Hoistable handling for Elements and Resources inside Suspense Boundaries (#28069) Updates Fizz to handle Hoistables (Resources and Elements) in a way that better aligns with Suspense fallbacks 1. Hoistable Elements inside a fallback (regardless of how deep and how many additional boundaries are intermediate) will be ignored. The reasoning is fallbacks are transient and since there is not good way to clean up hoistables because they escape their Suspense container its better to not emit them in the first place. SSR fallbacks are already not full fidelity because they never hydrate so this aligns with that somewhat. 2. Hoistable stylesheets in fallbacks will only block the reveal of a parent suspense boundary if the fallback is going to flush with that completed parent suspense boundary. Previously if you rendered a stylesheet Resource inside a fallback any parent suspense boundaries that completed after the shell flushed would include that resource in the set required to resolve before the boundary reveal happens on the client. This is not a semantic change, just a performance optimization 3. preconnect and preload hoistable queues are gone, if you want to optimize resource loading you shoudl use `ReactDOM.preconnect` and `ReactDOM.preload`. `viewport` meta tags get their own queue because they need to go before any preloads since they affect the media state. In addition to those functional changes this PR also refactors the boundary resource tracking by moving it to the task rather than using function calls at the start of each render and flush. Tasks also now track whether they are a fallback task supercedes prior work here: https://github.com/facebook/react/pull/27534 --- .../src/server/ReactFizzConfigDOM.js | 187 ++++----- .../src/server/ReactFizzConfigDOMLegacy.js | 22 +- .../src/__tests__/ReactDOMFloat-test.js | 391 +++++++++++++----- .../src/ReactNoopServer.js | 13 +- packages/react-server/src/ReactFizzServer.js | 180 ++++---- .../src/forks/ReactFizzConfig.custom.js | 11 +- 6 files changed, 488 insertions(+), 316 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 0788f216e5f30..ae409cdeed554 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -147,12 +147,11 @@ export type RenderState = { // external runtime script chunks externalRuntimeScript: null | ExternalRuntimeScript, bootstrapChunks: Array, + importMapChunks: Array, // Hoistable chunks charsetChunks: Array, - preconnectChunks: Array, - importMapChunks: Array, - preloadChunks: Array, + viewportChunks: Array, hoistableChunks: Array, // Headers queues for Resources that can flush early @@ -201,9 +200,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 +453,7 @@ export function createRenderState( externalRuntimeScript: externalRuntimeScript, bootstrapChunks: bootstrapChunks, + importMapChunks, onHeaders, headers, @@ -473,9 +470,7 @@ export function createRenderState( }, charsetChunks: [], - preconnectChunks: [], - importMapChunks, - preloadChunks: [], + viewportChunks: [], hoistableChunks: [], // cleared on flush @@ -497,7 +492,7 @@ export function createRenderState( nonce, // like a module global for currently rendering boundary - boundaryResources: null, + hoistableState: null, stylesToHoist: false, }; @@ -2230,6 +2225,7 @@ function pushMeta( textEmbedded: boolean, insertionMode: InsertionMode, noscriptTagInScope: boolean, + isFallback: boolean, ): null { if (enableFloat) { if ( @@ -2245,11 +2241,24 @@ function pushMeta( target.push(textSeparator); } - if (typeof props.charSet === 'string') { + 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(renderState.preconnectChunks, 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(renderState.hoistableChunks, props, 'meta'); } @@ -2264,9 +2273,11 @@ function pushLink( props: Object, resumableState: ResumableState, renderState: RenderState, + hoistableState: null | HoistableState, textEmbedded: boolean, insertionMode: InsertionMode, noscriptTagInScope: boolean, + isFallback: boolean, ): null { if (enableFloat) { const rel = props.rel; @@ -2384,8 +2395,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 (hoistableState) { + hoistableState.stylesheets.add(resource); } } else { // We need to track whether this boundary should wait on this resource or not. @@ -2396,8 +2407,8 @@ function pushLink( if (styleQueue) { const resource = styleQueue.sheets.get(key); if (resource) { - if (renderState.boundaryResources) { - renderState.boundaryResources.stylesheets.add(resource); + if (hoistableState) { + hoistableState.stylesheets.add(resource); } } } @@ -2422,14 +2433,14 @@ 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); + 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 { @@ -2472,6 +2483,7 @@ function pushStyle( props: Object, resumableState: ResumableState, renderState: RenderState, + hoistableState: null | HoistableState, textEmbedded: boolean, insertionMode: InsertionMode, noscriptTagInScope: boolean, @@ -2571,8 +2583,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 (hoistableState) { + hoistableState.styles.add(styleQueue); } } @@ -2885,6 +2897,7 @@ function pushTitle( renderState: RenderState, insertionMode: InsertionMode, noscriptTagInScope: boolean, + isFallback: boolean, ): ReactNodeList { if (__DEV__) { if (hasOwnProperty.call(props, 'children')) { @@ -2940,8 +2953,15 @@ function pushTitle( !noscriptTagInScope && props.itemProp == null ) { - pushTitleImpl(renderState.hoistableChunks, 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); } @@ -3472,8 +3492,10 @@ export function pushStartInstance( props: Object, resumableState: ResumableState, renderState: RenderState, + hoistableState: null | HoistableState, formatContext: FormatContext, textEmbedded: boolean, + isFallback: boolean, ): ReactNodeList { if (__DEV__) { validateARIAProperties(type, props); @@ -3542,6 +3564,7 @@ export function pushStartInstance( renderState, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), + isFallback, ) : pushStartTitle(target, props); case 'link': @@ -3550,9 +3573,11 @@ export function pushStartInstance( props, resumableState, renderState, + hoistableState, textEmbedded, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), + isFallback, ); case 'script': return enableFloat @@ -3572,6 +3597,7 @@ export function pushStartInstance( props, resumableState, renderState, + hoistableState, textEmbedded, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), @@ -3584,6 +3610,7 @@ export function pushStartInstance( textEmbedded, formatContext.insertionMode, !!(formatContext.tagScope & NOSCRIPT_SCOPE), + isFallback, ); // Newline eating tags case 'listing': @@ -4120,7 +4147,7 @@ export function writeCompletedBoundaryInstruction( resumableState: ResumableState, renderState: RenderState, id: number, - boundaryResources: BoundaryResources, + hoistableState: HoistableState, ): boolean { let requiresStyleInsertion; if (enableFloat) { @@ -4196,11 +4223,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) { @@ -4449,9 +4476,9 @@ function hasStylesToHoist(stylesheet: StylesheetResource): boolean { return false; } -export function writeResourcesForBoundary( +export function writeHoistablesForBoundary( destination: Destination, - boundaryResources: BoundaryResources, + hoistableState: HoistableState, renderState: RenderState, ): boolean { // Reset these on each invocation, they are only safe to read in this function @@ -4459,10 +4486,15 @@ 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); + + // 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; @@ -4629,11 +4661,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 = renderState.viewportChunks; + 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,13 +4690,6 @@ 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; for (i = 0; i < hoistableChunks.length; i++) { @@ -4672,13 +4697,9 @@ 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')); } } @@ -4699,15 +4720,15 @@ export function writeHoistables( // 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(); - 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,13 +4753,6 @@ 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; for (i = 0; i < hoistableChunks.length; i++) { @@ -4769,12 +4783,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 @@ -4962,12 +4976,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 @@ -5214,7 +5228,7 @@ type StylesheetResource = { state: StylesheetState, }; -export type BoundaryResources = { +export type HoistableState = { styles: Set, stylesheets: Set, }; @@ -5226,20 +5240,13 @@ export type StyleQueue = { sheets: Map, }; -export function createBoundaryResources(): BoundaryResources { +export function createHoistableState(): HoistableState { return { styles: new Set(), stylesheets: new Set(), }; } -export function setCurrentlyRenderingBoundaryResourcesTarget( - renderState: RenderState, - boundaryResources: null | BoundaryResources, -) { - renderState.boundaryResources = boundaryResources; -} - function getResourceKey(href: string): string { return href; } @@ -6087,31 +6094,25 @@ function escapeStringForLinkHeaderQuotedParamValueContextReplacer( } 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 hoistResources( - renderState: RenderState, - source: BoundaryResources, +export function hoistHoistables( + parentState: HoistableState, + childState: HoistableState, ): void { - const currentBoundaryResources = renderState.boundaryResources; - if (currentBoundaryResources) { - source.styles.forEach(hoistStyleQueueDependency, currentBoundaryResources); - source.stylesheets.forEach( - hoistStylesheetDependency, - currentBoundaryResources, - ); - } + childState.styles.forEach(hoistStyleQueueDependency, parentState); + childState.stylesheets.forEach(hoistStylesheetDependency, parentState); } // 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..a8f38e283cefc 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, @@ -57,9 +57,7 @@ export type RenderState = { }, resets: BaseRenderState['resets'], charsetChunks: Array, - preconnectChunks: Array, - importMapChunks: Array, - preloadChunks: Array, + viewportChunks: Array, hoistableChunks: Array, preconnects: Set, fontPreloads: Set, @@ -75,7 +73,6 @@ export type RenderState = { scripts: Map, moduleScripts: Map, }, - boundaryResources: ?BoundaryResources, stylesToHoist: boolean, // This is an extra field for the legacy renderer generateStaticMarkup: boolean, @@ -103,13 +100,12 @@ 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, + viewportChunks: renderState.viewportChunks, hoistableChunks: renderState.hoistableChunks, preconnects: renderState.preconnects, fontPreloads: renderState.fontPreloads, @@ -120,7 +116,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,7 +133,7 @@ export const doctypeChunk: PrecomputedChunk = stringToPrecomputedChunk(''); export type { ResumableState, - BoundaryResources, + HoistableState, FormatContext, } from './ReactFizzConfigDOM'; @@ -158,17 +153,16 @@ export { writeClientRenderBoundaryInstruction, writeStartPendingSuspenseBoundary, writeEndPendingSuspenseBoundary, - writeResourcesForBoundary, + writeHoistablesForBoundary, writePlaceholder, writeCompletedRoot, createRootFormatContext, createResumableState, - createBoundaryResources, + createHoistableState, writePreamble, writeHoistables, writePostamble, - hoistResources, - setCurrentlyRenderingBoundaryResourcesTarget, + 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..eaad571ad9759 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,167 @@ body { ); }); + it('does not flush hoistables for fallbacks', async () => { + function App() { + return ( + + + +
fallback1
+ + foo + + }> + <> +
primary1
+ + +
+ +
fallback2
+ + + + }> + <> +
primary2
+ + + + +
+ +
fallback3
+ + +
deep fallback ... primary content
+ +
+ + }> + <> +
primary3
+ + + + +
+ + + ); + } + + await act(() => { + renderToPipeableStream().pipe(writable); + resolveText('first'); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + + + + +
primary1
+
primary2
+
fallback3
+
deep fallback ... primary content
+ + , + ); + + 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( + + + {/* 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
+ + , + ); + + await act(() => { + resolveText('release'); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + + + +
primary1
+ + , + ); + }); + describe('ReactDOM.prefetchDNS(href)', () => { it('creates a dns-prefetch resource when called', async () => { function App({url}) { @@ -4840,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}) { @@ -7746,6 +7967,7 @@ background-color: green; + , @@ -7758,72 +7980,21 @@ background-color: green; {/* charset first */} - {/* preconnect links next */} - - - {/* preloads next */} - + {/* viewport meta next */} + {/* Everything else last */} a title + + + , ); }); - // @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 8816245bded02..4252195f81c1e 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -52,7 +52,7 @@ type Destination = { }; type RenderState = null; -type BoundaryResources = null; +type HoistableState = null; const POP = Buffer.from('/', 'utf8'); @@ -261,17 +261,16 @@ const ReactNoopServer = ReactFizzServer({ boundary.status = 'client-render'; }, + prepareHostDispatcher() {}, + writePreamble() {}, writeHoistables() {}, + writeHoistablesForBoundary() {}, writePostamble() {}, - - createBoundaryResources(): BoundaryResources { + hoistHoistables(parent: HoistableState, child: HoistableState) {}, + createHoistableState(): HoistableState { return null; }, - - setCurrentlyRenderingBoundaryResourcesTarget(resources: BoundaryResources) {}, - - prepareHostDispatcher() {}, emitEarlyPreloads() {}, }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index bf72d4167e0c1..6b82bfe3a45b7 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -26,7 +26,7 @@ import type { RenderState, ResumableState, FormatContext, - BoundaryResources, + HoistableState, } from './ReactFizzConfig'; import type {ContextSnapshot} from './ReactFizzNewContext'; import type {ComponentStackNode} from './ReactFizzComponentStack'; @@ -57,6 +57,7 @@ import { writeClientRenderBoundaryInstruction, writeCompletedBoundaryInstruction, writeCompletedSegmentInstruction, + writeHoistablesForBoundary, pushTextInstance, pushStartInstance, pushEndInstance, @@ -64,13 +65,11 @@ import { pushEndCompletedSuspenseBoundary, pushSegmentFinale, getChildFormatContext, - writeResourcesForBoundary, - writePreamble, writeHoistables, + writePreamble, writePostamble, - hoistResources, - setCurrentlyRenderingBoundaryResourcesTarget, - createBoundaryResources, + hoistHoistables, + createHoistableState, prepareHostDispatcher, supportsRequestStorage, requestStorage, @@ -211,7 +210,8 @@ type SuspenseBoundary = { completedSegments: Array, // completed but not yet flushed segments. 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 }; @@ -223,6 +223,7 @@ type RenderTask = { ping: () => void, blockedBoundary: Root | SuspenseBoundary, blockedSegment: Segment, // the segment we'll write to + 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) @@ -231,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 = { @@ -248,6 +250,7 @@ type ReplayTask = { ping: () => void, blockedBoundary: Root | SuspenseBoundary, blockedSegment: null, // we don't write to anything when we replay + 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) @@ -256,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; @@ -421,6 +425,7 @@ export function createRequest( -1, null, rootSegment, + null, abortSet, null, rootFormatContext, @@ -428,6 +433,7 @@ export function createRequest( rootContextSnapshot, emptyTreeContext, null, + false, ); pingedTasks.push(rootTask); return request; @@ -532,6 +538,7 @@ export function resumeRequest( -1, null, rootSegment, + null, abortSet, null, postponedState.rootFormatContext, @@ -539,6 +546,7 @@ export function resumeRequest( rootContextSnapshot, emptyTreeContext, null, + false, ); pingedTasks.push(rootTask); return request; @@ -556,6 +564,7 @@ export function resumeRequest( children, -1, null, + null, abortSet, null, postponedState.rootFormatContext, @@ -563,6 +572,7 @@ export function resumeRequest( rootContextSnapshot, emptyTreeContext, null, + false, ); pingedTasks.push(rootTask); return request; @@ -601,7 +611,8 @@ function createSuspenseBoundary( byteSize: 0, fallbackAbortableTasks, errorDigest: null, - resources: createBoundaryResources(), + contentState: createHoistableState(), + fallbackState: createHoistableState(), trackedContentKeyPath: null, trackedFallbackNode: null, }; @@ -614,6 +625,7 @@ function createRenderTask( childIndex: number, blockedBoundary: Root | SuspenseBoundary, blockedSegment: Segment, + hoistableState: null | HoistableState, abortSet: Set, keyPath: Root | KeyNode, formatContext: FormatContext, @@ -621,6 +633,7 @@ function createRenderTask( context: ContextSnapshot, treeContext: TreeContext, componentStack: null | ComponentStackNode, + isFallback: boolean, ): RenderTask { request.allPendingTasks++; if (blockedBoundary === null) { @@ -635,6 +648,7 @@ function createRenderTask( ping: () => pingTask(request, task), blockedBoundary, blockedSegment, + hoistableState, abortSet, keyPath, formatContext, @@ -643,6 +657,7 @@ function createRenderTask( treeContext, componentStack, thenableState, + isFallback, }; abortSet.add(task); return task; @@ -655,6 +670,7 @@ function createReplayTask( node: ReactNodeList, childIndex: number, blockedBoundary: Root | SuspenseBoundary, + hoistableState: null | HoistableState, abortSet: Set, keyPath: Root | KeyNode, formatContext: FormatContext, @@ -662,6 +678,7 @@ function createReplayTask( context: ContextSnapshot, treeContext: TreeContext, componentStack: null | ComponentStackNode, + isFallback: boolean, ): ReplayTask { request.allPendingTasks++; if (blockedBoundary === null) { @@ -677,6 +694,7 @@ function createReplayTask( ping: () => pingTask(request, task), blockedBoundary, blockedSegment: null, + hoistableState, abortSet, keyPath, formatContext, @@ -685,6 +703,7 @@ function createReplayTask( treeContext, componentStack, thenableState, + isFallback, }; abortSet.add(task); return task; @@ -892,6 +911,7 @@ function renderSuspenseBoundary( const prevKeyPath = task.keyPath; const parentBoundary = task.blockedBoundary; + const parentHoistableState = task.hoistableState; const parentSegment = task.blockedSegment; // Each time we enter a suspense boundary, we split out into a new segment for @@ -944,13 +964,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.hoistableState = newBoundary.contentState; task.blockedSegment = contentRootSegment; - if (enableFloat) { - setCurrentlyRenderingBoundaryResourcesTarget( - request.renderState, - newBoundary.resources, - ); - } task.keyPath = keyPath; try { @@ -1000,13 +1015,8 @@ 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.hoistableState = parentHoistableState; task.blockedSegment = parentSegment; task.keyPath = prevKeyPath; task.componentStack = previousComponentStack; @@ -1043,6 +1053,7 @@ function renderSuspenseBoundary( -1, parentBoundary, boundarySegment, + newBoundary.fallbackState, fallbackAbortSet, fallbackKeyPath, task.formatContext, @@ -1052,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. @@ -1079,6 +1091,7 @@ function replaySuspenseBoundary( const previousReplaySet: ReplaySet = task.replay; const parentBoundary = task.blockedBoundary; + const parentHoistableState = task.hoistableState; const content: ReactNodeList = props.children; const fallback: ReactNodeList = props.fallback; @@ -1093,13 +1106,8 @@ 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.hoistableState = resumedBoundary.contentState; 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. @@ -1154,13 +1162,8 @@ 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.hoistableState = parentHoistableState; task.replay = previousReplaySet; task.keyPath = prevKeyPath; task.componentStack = previousComponentStack; @@ -1182,6 +1185,7 @@ function replaySuspenseBoundary( fallback, -1, parentBoundary, + resumedBoundary.fallbackState, fallbackAbortSet, fallbackKeyPath, task.formatContext, @@ -1191,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. @@ -1257,8 +1262,10 @@ function renderHostElement( props, request.resumableState, request.renderState, + task.hoistableState, task.formatContext, segment.lastPushedText, + task.isFallback, ); segment.lastPushedText = false; const prevContext = task.formatContext; @@ -2683,6 +2690,7 @@ function spawnNewSuspendedReplayTask( task.node, task.childIndex, task.blockedBoundary, + task.hoistableState, task.abortSet, task.keyPath, task.formatContext, @@ -2692,6 +2700,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; @@ -2727,6 +2736,7 @@ function spawnNewSuspendedRenderTask( task.childIndex, task.blockedBoundary, newSegment, + task.hoistableState, task.abortSet, task.keyPath, task.formatContext, @@ -2736,6 +2746,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; @@ -3345,13 +3356,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( @@ -3456,9 +3460,6 @@ function retryRenderTask( erroredTask(request, task.blockedBoundary, x, errorInfo); return; } finally { - if (enableFloat) { - setCurrentlyRenderingBoundaryResourcesTarget(request.renderState, null); - } if (__DEV__) { currentTaskInDEV = prevTaskInDEV; } @@ -3541,9 +3542,6 @@ function retryReplayTask(request: Request, task: ReplayTask): void { } return; } finally { - if (enableFloat) { - setCurrentlyRenderingBoundaryResourcesTarget(request.renderState, null); - } if (__DEV__) { currentTaskInDEV = prevTaskInDEV; } @@ -3612,10 +3610,26 @@ export function performWork(request: Request): void { } } +function flushPreamble( + request: Request, + destination: Destination, + rootSegment: Segment, +) { + const willFlushAllSegments = + request.allPendingTasks === 0 && request.trackedPostpones === null; + writePreamble( + destination, + request.resumableState, + request.renderState, + willFlushAllSegments, + ); +} + function flushSubtree( request: Request, destination: Destination, segment: Segment, + hoistableState: null | HoistableState, ): boolean { segment.parentFlushed = true; switch (segment.status) { @@ -3645,7 +3659,7 @@ function flushSubtree( for (; chunkIdx < nextChild.index; chunkIdx++) { writeChunk(destination, chunks[chunkIdx]); } - r = flushSegment(request, destination, nextChild); + r = flushSegment(request, destination, nextChild, hoistableState); } // Finally just write all the remaining chunks for (; chunkIdx < chunks.length - 1; chunkIdx++) { @@ -3668,11 +3682,12 @@ function flushSegment( request: Request, destination: Destination, segment: Segment, + hoistableState: null | HoistableState, ): boolean { const boundary = segment.boundary; if (boundary === null) { // Not a suspense boundary. - return flushSubtree(request, destination, segment); + return flushSubtree(request, destination, segment, hoistableState); } boundary.parentFlushed = true; @@ -3690,7 +3705,7 @@ function flushSegment( boundary.errorComponentStack, ); // Flush the fallback. - flushSubtree(request, destination, segment); + flushSubtree(request, destination, segment, hoistableState); return writeEndClientRenderedSuspenseBoundary( destination, @@ -3713,8 +3728,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) { + hoistHoistables(hoistableState, boundary.fallbackState); + } + } // Flush the fallback. - flushSubtree(request, destination, segment); + flushSubtree(request, destination, segment, hoistableState); return writeEndPendingSuspenseBoundary(destination, request.renderState); } else if (boundary.byteSize > request.progressiveChunkSize) { @@ -3735,13 +3757,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); + flushSubtree(request, destination, segment, hoistableState); return writeEndPendingSuspenseBoundary(destination, request.renderState); } else { if (enableFloat) { - hoistResources(request.renderState, boundary.resources); + if (hoistableState) { + hoistHoistables(hoistableState, boundary.contentState); + } } // We can inline this boundary's content as a complete boundary. writeStartCompletedSuspenseBoundary(destination, request.renderState); @@ -3755,7 +3784,7 @@ function flushSegment( } const contentSegment = completedSegments[0]; - flushSegment(request, destination, contentSegment); + flushSegment(request, destination, contentSegment, hoistableState); return writeEndCompletedSuspenseBoundary(destination, request.renderState); } @@ -3781,6 +3810,7 @@ function flushSegmentContainer( request: Request, destination: Destination, segment: Segment, + hoistableState: HoistableState, ): boolean { writeStartSegment( destination, @@ -3788,7 +3818,7 @@ function flushSegmentContainer( segment.parentFormatContext, segment.id, ); - flushSegment(request, destination, segment); + flushSegment(request, destination, segment, hoistableState); return writeEndSegment(destination, segment.parentFormatContext); } @@ -3797,12 +3827,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++) { @@ -3812,9 +3836,9 @@ function flushCompletedBoundary( completedSegments.length = 0; if (enableFloat) { - writeResourcesForBoundary( + writeHoistablesForBoundary( destination, - boundary.resources, + boundary.contentState, request.renderState, ); } @@ -3824,7 +3848,7 @@ function flushCompletedBoundary( request.resumableState, request.renderState, boundary.rootSegmentID, - boundary.resources, + boundary.contentState, ); } @@ -3833,12 +3857,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++) { @@ -3856,13 +3874,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 writeHoistablesForBoundary( destination, - boundary.resources, + boundary.contentState, request.renderState, ); } else { @@ -3881,6 +3895,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 @@ -3893,13 +3909,13 @@ function flushPartiallyCompletedSegment( ); } - return flushSegmentContainer(request, destination, segment); + 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); + return flushSegmentContainer(request, destination, segment, hoistableState); } else { - flushSegmentContainer(request, destination, segment); + flushSegmentContainer(request, destination, segment, hoistableState); return writeCompletedSegmentInstruction( destination, request.resumableState, @@ -3928,15 +3944,10 @@ function flushCompletedQueues( return; } else if (request.pendingRootTasks === 0) { if (enableFloat) { - writePreamble( - destination, - request.resumableState, - request.renderState, - request.allPendingTasks === 0 && request.trackedPostpones === null, - ); + flushPreamble(request, destination, completedRootSegment); } - flushSegment(request, destination, completedRootSegment); + flushSegment(request, destination, completedRootSegment, null); request.completedRootSegment = null; writeCompletedRoot(destination, request.renderState); } else { @@ -3944,7 +3955,6 @@ function flushCompletedQueues( return; } } - if (enableFloat) { writeHoistables(destination, request.resumableState, request.renderState); } diff --git a/packages/react-server/src/forks/ReactFizzConfig.custom.js b/packages/react-server/src/forks/ReactFizzConfig.custom.js index dbf6b32aa8201..eb76985c49218 100644 --- a/packages/react-server/src/forks/ReactFizzConfig.custom.js +++ b/packages/react-server/src/forks/ReactFizzConfig.custom.js @@ -29,8 +29,8 @@ 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; export opaque type HeadersDescriptor = mixed; export type {TransitionStatus}; @@ -86,11 +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 hoistResources = $$$config.hoistResources; -export const createResources = $$$config.createResources; -export const createBoundaryResources = $$$config.createBoundaryResources; -export const setCurrentlyRenderingBoundaryResourcesTarget = - $$$config.setCurrentlyRenderingBoundaryResourcesTarget; -export const writeResourcesForBoundary = $$$config.writeResourcesForBoundary; +export const hoistHoistables = $$$config.hoistHoistables; +export const createHoistableState = $$$config.createHoistableState; export const emitEarlyPreloads = $$$config.emitEarlyPreloads;