From c8c4d22321f70cc1ecea1ec9f487c07e65807194 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Fri, 5 Aug 2022 16:33:02 -0700 Subject: [PATCH 01/12] implement preamble and postambl for react-dom/server --- .../src/__tests__/ReactDOMFizzServer-test.js | 137 +++++++++++++++++- .../src/server/ReactDOMServerFormatConfig.js | 24 +++ .../src/ReactNoopServer.js | 2 + packages/react-server/src/ReactFizzServer.js | 47 ++++-- 4 files changed, 197 insertions(+), 13 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 60a3a3938df3e..c18125a7b542a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -128,6 +128,8 @@ describe('ReactDOMFizzServer', () => { buffer = ''; const fakeBody = document.createElement('body'); fakeBody.innerHTML = bufferedContent; + const parent = + container.nodeName === '#document' ? container.body : container; while (fakeBody.firstChild) { const node = fakeBody.firstChild; if ( @@ -137,13 +139,37 @@ describe('ReactDOMFizzServer', () => { const script = document.createElement('script'); script.textContent = node.textContent; fakeBody.removeChild(node); - container.appendChild(script); + parent.appendChild(script); } else { - container.appendChild(node); + parent.appendChild(node); } } } + async function actIntoEmptyDocument(callback) { + await callback(); + // Await one turn around the event loop. + // This assumes that we'll flush everything we have so far. + await new Promise(resolve => { + setImmediate(resolve); + }); + if (hasErrored) { + throw fatalError; + } + // JSDOM doesn't support stream HTML parser so we need to give it a proper fragment. + // We also want to execute any scripts that are embedded. + // We assume that we have now received a proper fragment of HTML. + const bufferedContent = buffer; + // Test Environment + const jsdom = new JSDOM(bufferedContent, { + runScripts: 'dangerously', + }); + window = jsdom.window; + document = jsdom.window.document; + container = document; + buffer = ''; + } + function getVisibleChildren(element) { const children = []; let node = element.firstChild; @@ -4194,6 +4220,113 @@ describe('ReactDOMFizzServer', () => { ); }); + it('emits html and head start tags (the preamble) before other content if rendered in the shell', async () => { + await actIntoEmptyDocument(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + <> + a title + + + a body + + , + ); + pipe(writable); + }); + expect(getVisibleChildren(document)).toEqual( + + + a title + + a body + , + ); + + // Hydrate the same thing on the client. We expect this to still fail because is not a Resource + // and is unmatched on hydration + const errors = []; + const root = ReactDOMClient.hydrateRoot( + document, + <> + <title data-baz="baz">a title + + + a body + + , + { + onRecoverableError: (err, errInfo) => { + errors.push(err.message); + }, + }, + ); + expect(() => { + try { + expect(() => { + expect(Scheduler).toFlushWithoutYielding(); + }).toThrow('Invalid insertion of HTML node in #document node.'); + } catch (e) { + console.log('e', e); + } + }).toErrorDev( + [ + 'Warning: Expected server HTML to contain a matching in <#document>.', + 'Warning: An error occurred during hydration. The server HTML was replaced with client content in <#document>.', + 'Warning: validateDOMNesting(...): <title> cannot appear as a child of <#document>', + ], + {withoutStack: 1}, + ); + expect(errors).toEqual([ + 'Hydration failed because the initial UI does not match what was rendered on the server.', + 'There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering.', + ]); + }); + + it('holds back body and html closing tags (the postamble) until all pending tasks are completed', async () => { + const chunks = []; + writable.on('data', chunk => { + chunks.push(chunk); + }); + + await actIntoEmptyDocument(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + <html> + <head /> + <body> + first + <Suspense> + <AsyncText text="second" /> + </Suspense> + </body> + </html>, + ); + pipe(writable); + }); + + expect(getVisibleChildren(document)).toEqual( + <html> + <head /> + <body>{'first'}</body> + </html>, + ); + + await act(() => { + resolveText('second'); + }); + + expect(getVisibleChildren(document)).toEqual( + <html> + <head /> + <body> + {'first'} + {'second'} + </body> + </html>, + ); + + expect(chunks.pop()).toEqual('</body></html>'); + }); + describe('text separators', () => { // To force performWork to start before resolving AsyncText but before piping we need to wait until // after scheduleWork which currently uses setImmediate to delay performWork diff --git a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js index 36c9469d60818..0dccd7ee34bd3 100644 --- a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js @@ -242,6 +242,26 @@ export function getChildFormatContext( return parentContext; } +export function isPreambleInsertion(type: string): boolean { + switch (type) { + case 'html': + case 'head': { + return true; + } + } + return false; +} + +export function isPostambleInsertion(type: string): boolean { + switch (type) { + case 'body': + case 'html': { + return true; + } + } + return false; +} + export type SuspenseBoundaryID = null | PrecomputedChunk; export const UNINITIALIZED_SUSPENSE_BOUNDARY_ID: SuspenseBoundaryID = null; @@ -1405,11 +1425,13 @@ const DOCTYPE: PrecomputedChunk = stringToPrecomputedChunk('<!DOCTYPE html>'); export function pushStartInstance( target: Array<Chunk | PrecomputedChunk>, + preamble: Array<Chunk | PrecomputedChunk>, type: string, props: Object, responseState: ResponseState, formatContext: FormatContext, ): ReactNodeList { + target = isPreambleInsertion(type) ? preamble : target; if (__DEV__) { validateARIAProperties(type, props); validateInputProperties(type, props); @@ -1521,9 +1543,11 @@ const endTag2 = stringToPrecomputedChunk('>'); export function pushEndInstance( target: Array<Chunk | PrecomputedChunk>, + postamble: Array<Chunk | PrecomputedChunk>, type: string, props: Object, ): void { + target = isPostambleInsertion(type) ? postamble : target; switch (type) { // Omitted close tags // TODO: Instead of repeating this switch we could try to pass a flag from above. diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index 14003b8291b37..e70de39ab3868 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -113,6 +113,7 @@ const ReactNoopServer = ReactFizzServer({ }, pushStartInstance( target: Array<Uint8Array>, + preamble: Array<Uint8Array>, type: string, props: Object, ): ReactNodeList { @@ -128,6 +129,7 @@ const ReactNoopServer = ReactFizzServer({ pushEndInstance( target: Array<Uint8Array>, + postamble: Array<Uint8Array>, type: string, props: Object, ): void { diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index f2974e0507e1d..8b768601ddef0 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -200,6 +200,8 @@ export opaque type Request = { clientRenderedBoundaries: Array<SuspenseBoundary>, // Errored or client rendered but not yet flushed. completedBoundaries: Array<SuspenseBoundary>, // Completed but not yet fully flushed boundaries to show. partialBoundaries: Array<SuspenseBoundary>, // Partially completed boundaries that can flush its segments early. + +preamble: Array<Chunk | PrecomputedChunk>, // Chunks that need to be emitted before any segment chunks. + +postamble: Array<Chunk | PrecomputedChunk>, // Chunks that need to be emitted after segments, waiting for all pending root tasks to finish // onError is called when an error happens anywhere in the tree. It might recover. // The return string is used in production primarily to avoid leaking internals, secondarily to save bytes. // Returning null/undefined will cause a defualt error message in production @@ -272,6 +274,8 @@ export function createRequest( clientRenderedBoundaries: [], completedBoundaries: [], partialBoundaries: [], + preamble: [], + postamble: [], onError: onError === undefined ? defaultErrorHandler : onError, onAllReady: onAllReady === undefined ? noop : onAllReady, onShellReady: onShellReady === undefined ? noop : onShellReady, @@ -632,6 +636,7 @@ function renderHostElement( const segment = task.blockedSegment; const children = pushStartInstance( segment.chunks, + request.preamble, type, props, request.responseState, @@ -647,7 +652,7 @@ function renderHostElement( // We expect that errors will fatal the whole task and that we don't need // the correct context. Therefore this is not in a finally. segment.formatContext = prevContext; - pushEndInstance(segment.chunks, type, props); + pushEndInstance(segment.chunks, request.postamble, type, props); segment.lastPushedText = false; popComponentStackInDEV(task); } @@ -2054,6 +2059,7 @@ function flushCompletedQueues( request: Request, destination: Destination, ): void { + let allComplete = false; beginWriting(destination); try { // The structure of this is to go through each queue one by one and write @@ -2063,20 +2069,30 @@ function flushCompletedQueues( // TODO: Emit preloading. - // TODO: It's kind of unfortunate to keep checking this array after we've already - // emitted the root. + let i; const completedRootSegment = request.completedRootSegment; - if (completedRootSegment !== null && request.pendingRootTasks === 0) { - flushSegment(request, destination, completedRootSegment); - request.completedRootSegment = null; - writeCompletedRoot(destination, request.responseState); + if (completedRootSegment !== null) { + if (request.pendingRootTasks === 0) { + const preamble = request.preamble; + for (i = 0; i < preamble.length; i++) { + // we expect the preamble to be tiny and will ignore backpressure + writeChunk(destination, preamble[i]); + } + preamble.length = 0; + + flushSegment(request, destination, completedRootSegment); + request.completedRootSegment = null; + writeCompletedRoot(destination, request.responseState); + } else { + // We haven't flushed the root yet so we don't need to check boundaries further down + return; + } } // We emit client rendering instructions for already emitted boundaries first. // This is so that we can signal to the client to start client rendering them as // soon as possible. const clientRenderedBoundaries = request.clientRenderedBoundaries; - let i; for (i = 0; i < clientRenderedBoundaries.length; i++) { const boundary = clientRenderedBoundaries[i]; if (!flushClientRenderedBoundary(request, destination, boundary)) { @@ -2138,9 +2154,7 @@ function flushCompletedQueues( } } largeBoundaries.splice(0, i); - } finally { - completeWriting(destination); - flushBuffered(destination); + if ( request.allPendingTasks === 0 && request.pingedTasks.length === 0 && @@ -2149,6 +2163,17 @@ function flushCompletedQueues( // We don't need to check any partially completed segments because // either they have pending task or they're complete. ) { + allComplete = true; + const postamble = request.postamble; + for (i = 0; i < postamble.length; i++) { + writeChunk(destination, postamble[i]); + } + postamble.length = 0; + } + } finally { + completeWriting(destination); + flushBuffered(destination); + if (allComplete) { if (__DEV__) { if (request.abortableTasks.size !== 0) { console.error( From eb34ba299ce3e12ad6e2cd97665d56e93d27d683 Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Mon, 8 Aug 2022 13:37:57 -0700 Subject: [PATCH 02/12] support hydrating resource stylesheets outside of normal flow --- .../src/__tests__/ReactDOMFizzServer-test.js | 121 +++++++++++++++++- .../react-dom/src/client/ReactDOMComponent.js | 14 +- .../src/client/ReactDOMHostConfig.js | 69 +++++++++- .../src/server/ReactDOMServerFormatConfig.js | 61 ++++++++- .../server/ReactNativeServerFormatConfig.js | 2 + .../ReactFiberHostConfigWithNoHydration.js | 2 + .../src/ReactFiberHydrationContext.new.js | 23 ++++ .../src/ReactFiberHydrationContext.old.js | 23 ++++ .../src/forks/ReactFiberHostConfig.custom.js | 3 + packages/react-server/src/ReactFizzServer.js | 35 +++-- packages/shared/ReactFeatureFlags.js | 2 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.native.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.testing.js | 1 + .../forks/ReactFeatureFlags.testing.www.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + scripts/error-codes/codes.json | 4 +- 21 files changed, 344 insertions(+), 24 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index c18125a7b542a..8ba29d37c5f2f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -4220,6 +4220,7 @@ describe('ReactDOMFizzServer', () => { ); }); + // @gate enableFloat it('emits html and head start tags (the preamble) before other content if rendered in the shell', async () => { await actIntoEmptyDocument(() => { const {pipe} = ReactDOMFizzServer.renderToPipeableStream( @@ -4245,7 +4246,7 @@ describe('ReactDOMFizzServer', () => { // Hydrate the same thing on the client. We expect this to still fail because <title> is not a Resource // and is unmatched on hydration const errors = []; - const root = ReactDOMClient.hydrateRoot( + ReactDOMClient.hydrateRoot( document, <> <title data-baz="baz">a title @@ -4280,8 +4281,13 @@ describe('ReactDOMFizzServer', () => { 'Hydration failed because the initial UI does not match what was rendered on the server.', 'There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering.', ]); + expect(getVisibleChildren(document)).toEqual(); + expect(() => { + expect(Scheduler).toFlushWithoutYielding(); + }).toThrow('The node to be removed is not a child of this node.'); }); + // @gate enableFloat it('holds back body and html closing tags (the postamble) until all pending tasks are completed', async () => { const chunks = []; writable.on('data', chunk => { @@ -4327,6 +4333,119 @@ describe('ReactDOMFizzServer', () => { expect(chunks.pop()).toEqual(''); }); + // @gate enableFloat + it('recognizes stylesheet links as attributes during hydration', async () => { + await actIntoEmptyDocument(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + <> + + + + + + a body + + , + ); + pipe(writable); + }); + // precedence for stylesheets is mapped to a valid data attribute that is recognized on the client + // as opting this node into resource semantics. the use of precedence on the author link is just a + // non standard attribute which React allows but is not given any special treatment. + expect(getVisibleChildren(document)).toEqual( + + + + + + a body + , + ); + + // It hydrates successfully + ReactDOMClient.hydrateRoot( + document, + <> + + + + + + a body + + , + ); + expect(Scheduler).toFlushWithoutYielding(); + expect(getVisibleChildren(document)).toEqual( + + + + + + a body + , + ); + }); + + // @gate __DEV__ && enableFloat + it('should error in dev when rendering more than one resource for a given location (href)', async () => { + await actIntoEmptyDocument(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + <> + + + + + a body + + , + ); + pipe(writable); + }); + expect(getVisibleChildren(document)).toEqual( + + + + + + a body + , + ); + + const errors = []; + ReactDOMClient.hydrateRoot( + document, + <> + + + + + + a body + + , + { + onRecoverableError(err, errInfo) { + errors.push(err.message); + }, + }, + ); + expect(() => { + expect(Scheduler).toFlushWithoutYielding(); + }).toErrorDev( + [ + 'An error occurred during hydration. The server HTML was replaced with client content in <#document>.', + ], + {withoutStack: true}, + ); + expect(errors).toEqual([ + 'Stylesheet resources need a unique representation in the DOM while hydrating and more than one matching DOM Node was found. To fix, ensure you are only rendering one stylesheet link with an href attribute of "foo".', + 'Stylesheet resources need a unique representation in the DOM while hydrating and more than one matching DOM Node was found. To fix, ensure you are only rendering one stylesheet link with an href attribute of "foo".', + 'Hydration failed because the initial UI does not match what was rendered on the server.', + 'There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering.', + ]); + }); + describe('text separators', () => { // To force performWork to start before resolving AsyncText but before piping we need to wait until // after scheduleWork which currently uses setImmediate to delay performWork diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index 93c1534c235b4..8b48bae61a497 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -73,6 +73,7 @@ import { enableTrustedTypesIntegration, enableCustomElementPropertySupport, enableClientRenderFallbackOnTextMismatch, + enableFloat, } from 'shared/ReactFeatureFlags'; import { mediaEventTypes, @@ -257,7 +258,7 @@ export function checkForUnmatchedText( } } -function getOwnerDocumentFromRootContainer( +export function getOwnerDocumentFromRootContainer( rootContainerElement: Element | Document | DocumentFragment, ): Document { return rootContainerElement.nodeType === DOCUMENT_NODE @@ -1018,6 +1019,17 @@ export function diffHydratedProperties( : getPropertyInfo(propKey); if (rawProps[SUPPRESS_HYDRATION_WARNING] === true) { // Don't bother comparing. We're ignoring all these warnings. + } else if ( + enableFloat && + tag === 'link' && + rawProps.rel === 'stylesheet' && + propKey === 'precedence' + ) { + // @TODO this is a temporary rule while we haven't implemented HostResources yet. This is used to allow + // for hydrating Resources (at the moment, stylesheets with a precedence prop) by using a data attribute. + // When we implement HostResources there will be no hydration directly so this code can be deleted + // $FlowFixMe - Should be inferred as not undefined. + extraAttributeNames.delete('data-rprec'); } else if ( propKey === SUPPRESS_CONTENT_EDITABLE_WARNING || propKey === SUPPRESS_HYDRATION_WARNING || diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 12a14df0f1c14..69996a88fb717 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -40,6 +40,7 @@ import { warnForDeletedHydratableText, warnForInsertedHydratedElement, warnForInsertedHydratedText, + getOwnerDocumentFromRootContainer, } from './ReactDOMComponent'; import {getSelectionInformation, restoreSelection} from './ReactInputSelection'; import setTextContent from './setTextContent'; @@ -64,6 +65,7 @@ import {retryIfBlockedOn} from '../events/ReactDOMEventReplaying'; import { enableCreateEventHandleAPI, enableScopeAPI, + enableFloat, } from 'shared/ReactFeatureFlags'; import {HostComponent, HostText} from 'react-reconciler/src/ReactWorkTags'; import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem'; @@ -675,6 +677,14 @@ export function clearContainer(container: Container): void { export const supportsHydration = true; +export function isHydratableResource(type: string, props: Props) { + return ( + type === 'link' && + typeof (props: any).precedence === 'string' && + (props: any).rel === 'stylesheet' + ); +} + export function canHydrateInstance( instance: HydratableInstance, type: string, @@ -769,10 +779,25 @@ export function registerSuspenseInstanceRetry( function getNextHydratable(node) { // Skip non-hydratable nodes. - for (; node != null; node = node.nextSibling) { + for (; node != null; node = ((node: any): Node).nextSibling) { const nodeType = node.nodeType; - if (nodeType === ELEMENT_NODE || nodeType === TEXT_NODE) { - break; + if (enableFloat) { + if (nodeType === ELEMENT_NODE) { + if ( + ((node: any): Element).tagName === 'LINK' && + ((node: any): Element).hasAttribute('data-rprec') + ) { + continue; + } + break; + } + if (nodeType === TEXT_NODE) { + break; + } + } else { + if (nodeType === ELEMENT_NODE || nodeType === TEXT_NODE) { + break; + } } if (nodeType === COMMENT_NODE) { const nodeData = (node: any).data; @@ -873,6 +898,44 @@ export function hydrateSuspenseInstance( precacheFiberNode(internalInstanceHandle, suspenseInstance); } +export function getMatchingResourceInstance( + type: string, + props: Props, + rootHostContainer: Container, +): ?Instance { + if (enableFloat) { + switch (type) { + case 'link': { + if (typeof (props: any).href !== 'string') { + return null; + } + const selector = `link[rel="stylesheet"][data-rprec][href="${ + (props: any).href + }"]`; + const link = getOwnerDocumentFromRootContainer( + rootHostContainer, + ).querySelector(selector); + if (__DEV__) { + const allLinks = getOwnerDocumentFromRootContainer( + rootHostContainer, + ).querySelectorAll(selector); + if (allLinks.length > 1) { + throw new Error( + 'Stylesheet resources need a unique representation in the DOM while hydrating' + + ' and more than one matching DOM Node was found. To fix, ensure you are only' + + ' rendering one stylesheet link with an href attribute of "' + + (props: any).href + + '".', + ); + } + } + return link; + } + } + } + return null; +} + export function getNextHydratableInstanceAfterSuspenseInstance( suspenseInstance: SuspenseInstance, ): null | HydratableInstance { diff --git a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js index 0dccd7ee34bd3..9f94cde76af66 100644 --- a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js @@ -20,6 +20,7 @@ import {Children} from 'react'; import { enableFilterEmptyStringAttributesDOM, enableCustomElementPropertySupport, + enableFloat, } from 'shared/ReactFeatureFlags'; import type { @@ -1076,6 +1077,52 @@ function pushStartTextArea( return null; } +function pushLink( + target: Array, + props: Object, + responseState: ResponseState, +): ReactNodeList { + const isStylesheet = props.rel === 'stylesheet'; + target.push(startChunkForTag('link')); + + for (const propKey in props) { + if (hasOwnProperty.call(props, propKey)) { + const propValue = props[propKey]; + if (propValue == null) { + continue; + } + switch (propKey) { + case 'children': + case 'dangerouslySetInnerHTML': + throw new Error( + `${'link'} is a self-closing tag and must neither have \`children\` nor ` + + 'use `dangerouslySetInnerHTML`.', + ); + case 'precedence': { + if (isStylesheet) { + if (propValue === true || typeof propValue === 'string') { + pushAttribute(target, responseState, 'data-rprec', propValue); + } else if (__DEV__) { + throw new Error( + `the "precedence" prop for links to stylehseets expects to receive a string but received something of type "${typeof propValue}" instead.`, + ); + } + break; + } + // intentionally fall through + } + // eslint-disable-next-line-no-fallthrough + default: + pushAttribute(target, responseState, propKey, propValue); + break; + } + } + } + + target.push(endOfStartTagSelfClosing); + return null; +} + function pushSelfClosing( target: Array, props: Object, @@ -1425,13 +1472,14 @@ const DOCTYPE: PrecomputedChunk = stringToPrecomputedChunk(''); export function pushStartInstance( target: Array, - preamble: Array, + preamble: ?Array, type: string, props: Object, responseState: ResponseState, formatContext: FormatContext, ): ReactNodeList { - target = isPreambleInsertion(type) ? preamble : target; + // Preamble type is nullable for feature off cases but is guaranteed when feature is on + target = enableFloat && isPreambleInsertion(type) ? (preamble: any) : target; if (__DEV__) { validateARIAProperties(type, props); validateInputProperties(type, props); @@ -1483,6 +1531,8 @@ export function pushStartInstance( return pushStartMenuItem(target, props, responseState); case 'title': return pushStartTitle(target, props, responseState); + case 'link': + return pushLink(target, props, responseState); // Newline eating tags case 'listing': case 'pre': { @@ -1497,7 +1547,6 @@ export function pushStartInstance( case 'hr': case 'img': case 'keygen': - case 'link': case 'meta': case 'param': case 'source': @@ -1543,11 +1592,13 @@ const endTag2 = stringToPrecomputedChunk('>'); export function pushEndInstance( target: Array, - postamble: Array, + postamble: ?Array, type: string, props: Object, ): void { - target = isPostambleInsertion(type) ? postamble : target; + // Preamble type is nullable for feature off cases but is guaranteed when feature is on + target = + enableFloat && isPostambleInsertion(type) ? (postamble: any) : target; switch (type) { // Omitted close tags // TODO: Instead of repeating this switch we could try to pass a flag from above. diff --git a/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js b/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js index 3c2c23c911faf..422d0ba3aed26 100644 --- a/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js +++ b/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js @@ -137,6 +137,7 @@ export function pushTextInstance( export function pushStartInstance( target: Array, + preamble: ?Array, type: string, props: Object, responseState: ResponseState, @@ -153,6 +154,7 @@ export function pushStartInstance( export function pushEndInstance( target: Array, + postamble: ?Array, type: string, props: Object, ): void { diff --git a/packages/react-reconciler/src/ReactFiberHostConfigWithNoHydration.js b/packages/react-reconciler/src/ReactFiberHostConfigWithNoHydration.js index f61c0eb7789e7..9eba0aad9dc21 100644 --- a/packages/react-reconciler/src/ReactFiberHostConfigWithNoHydration.js +++ b/packages/react-reconciler/src/ReactFiberHostConfigWithNoHydration.js @@ -24,10 +24,12 @@ export const supportsHydration = false; export const canHydrateInstance = shim; export const canHydrateTextInstance = shim; export const canHydrateSuspenseInstance = shim; +export const isHydratableResource = shim; export const isSuspenseInstancePending = shim; export const isSuspenseInstanceFallback = shim; export const getSuspenseInstanceFallbackErrorDetails = shim; export const registerSuspenseInstanceRetry = shim; +export const getMatchingResourceInstance = shim; export const getNextHydratableSibling = shim; export const getFirstHydratableChild = shim; export const getFirstHydratableChildWithinContainer = shim; diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index fd3f9b18a4921..60b653ddd1351 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -34,6 +34,7 @@ import { NoFlags, DidCapture, } from './ReactFiberFlags'; +import {enableFloat} from 'shared/ReactFeatureFlags'; import { createFiberFromHostInstanceForDeletion, @@ -45,7 +46,9 @@ import { canHydrateInstance, canHydrateTextInstance, canHydrateSuspenseInstance, + isHydratableResource, getNextHydratableSibling, + getMatchingResourceInstance, getFirstHydratableChild, getFirstHydratableChildWithinContainer, getFirstHydratableChildWithinSuspenseInstance, @@ -75,6 +78,7 @@ import { restoreSuspendedTreeContext, } from './ReactFiberTreeContext.new'; import {queueRecoverableErrors} from './ReactFiberWorkLoop.new'; +import {getRootHostContainer} from './ReactFiberHostContext.new'; // The deepest Fiber on the stack involved in a hydration context. // This may have been an insertion or a hydration. @@ -404,6 +408,22 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void { if (!isHydrating) { return; } + if (enableFloat) { + if ( + fiber.tag === HostComponent && + isHydratableResource(fiber.type, fiber.pendingProps) + ) { + const resourceInstance = getMatchingResourceInstance( + fiber.type, + fiber.pendingProps, + getRootHostContainer(), + ); + if (resourceInstance) { + fiber.stateNode = resourceInstance; + return; + } + } + } let nextInstance = nextHydratableInstance; if (!nextInstance) { if (shouldClientRenderOnMismatch(fiber)) { @@ -596,6 +616,9 @@ function popHydrationState(fiber: Fiber): boolean { if (!supportsHydration) { return false; } + if (enableFloat && isHydratableResource(fiber.type, fiber.memoizedProps)) { + return !!fiber.stateNode; + } if (fiber !== hydrationParentFiber) { // We're deeper than the current hydration context, inside an inserted // tree. diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js index 9ac9e4dc666b2..02c65dea3ef01 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js @@ -34,6 +34,7 @@ import { NoFlags, DidCapture, } from './ReactFiberFlags'; +import {enableFloat} from 'shared/ReactFeatureFlags'; import { createFiberFromHostInstanceForDeletion, @@ -45,7 +46,9 @@ import { canHydrateInstance, canHydrateTextInstance, canHydrateSuspenseInstance, + isHydratableResource, getNextHydratableSibling, + getMatchingResourceInstance, getFirstHydratableChild, getFirstHydratableChildWithinContainer, getFirstHydratableChildWithinSuspenseInstance, @@ -75,6 +78,7 @@ import { restoreSuspendedTreeContext, } from './ReactFiberTreeContext.old'; import {queueRecoverableErrors} from './ReactFiberWorkLoop.old'; +import {getRootHostContainer} from './ReactFiberHostContext.old'; // The deepest Fiber on the stack involved in a hydration context. // This may have been an insertion or a hydration. @@ -404,6 +408,22 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void { if (!isHydrating) { return; } + if (enableFloat) { + if ( + fiber.tag === HostComponent && + isHydratableResource(fiber.type, fiber.pendingProps) + ) { + const resourceInstance = getMatchingResourceInstance( + fiber.type, + fiber.pendingProps, + getRootHostContainer(), + ); + if (resourceInstance) { + fiber.stateNode = resourceInstance; + return; + } + } + } let nextInstance = nextHydratableInstance; if (!nextInstance) { if (shouldClientRenderOnMismatch(fiber)) { @@ -596,6 +616,9 @@ function popHydrationState(fiber: Fiber): boolean { if (!supportsHydration) { return false; } + if (enableFloat && isHydratableResource(fiber.type, fiber.memoizedProps)) { + return !!fiber.stateNode; + } if (fiber !== hydrationParentFiber) { // We're deeper than the current hydration context, inside an inserted // tree. diff --git a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js index 8afc9a3aa2cb9..9bb5d6e271c49 100644 --- a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js @@ -186,3 +186,6 @@ export const didNotFindHydratableTextInstance = export const didNotFindHydratableSuspenseInstance = $$$hostConfig.didNotFindHydratableSuspenseInstance; export const errorHydratingContainer = $$$hostConfig.errorHydratingContainer; +export const isHydratableResource = $$$hostConfig.isHydratableResource; +export const getMatchingResourceInstance = + $$$hostConfig.getMatchingResourceInstance; diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 8b768601ddef0..e9fc0adaae280 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -117,6 +117,7 @@ import { warnAboutDefaultPropsOnFunctionComponents, enableScopeAPI, enableSuspenseAvoidThisFallbackFizz, + enableFloat, } from 'shared/ReactFeatureFlags'; import assign from 'shared/assign'; @@ -200,8 +201,8 @@ export opaque type Request = { clientRenderedBoundaries: Array, // Errored or client rendered but not yet flushed. completedBoundaries: Array, // Completed but not yet fully flushed boundaries to show. partialBoundaries: Array, // Partially completed boundaries that can flush its segments early. - +preamble: Array, // Chunks that need to be emitted before any segment chunks. - +postamble: Array, // Chunks that need to be emitted after segments, waiting for all pending root tasks to finish + +preamble: ?Array, // Chunks that need to be emitted before any segment chunks. + +postamble: ?Array, // Chunks that need to be emitted after segments, waiting for all pending root tasks to finish // onError is called when an error happens anywhere in the tree. It might recover. // The return string is used in production primarily to avoid leaking internals, secondarily to save bytes. // Returning null/undefined will cause a defualt error message in production @@ -274,8 +275,8 @@ export function createRequest( clientRenderedBoundaries: [], completedBoundaries: [], partialBoundaries: [], - preamble: [], - postamble: [], + preamble: enableFloat ? [] : null, + postamble: enableFloat ? [] : null, onError: onError === undefined ? defaultErrorHandler : onError, onAllReady: onAllReady === undefined ? noop : onAllReady, onShellReady: onShellReady === undefined ? noop : onShellReady, @@ -2073,12 +2074,16 @@ function flushCompletedQueues( const completedRootSegment = request.completedRootSegment; if (completedRootSegment !== null) { if (request.pendingRootTasks === 0) { - const preamble = request.preamble; - for (i = 0; i < preamble.length; i++) { - // we expect the preamble to be tiny and will ignore backpressure - writeChunk(destination, preamble[i]); + if (enableFloat) { + const preamble: Array< + Chunk | PrecomputedChunk, + > = (request.preamble: any); + for (i = 0; i < preamble.length; i++) { + // we expect the preamble to be tiny and will ignore backpressure + writeChunk(destination, preamble[i]); + } + preamble.length = 0; } - preamble.length = 0; flushSegment(request, destination, completedRootSegment); request.completedRootSegment = null; @@ -2164,11 +2169,15 @@ function flushCompletedQueues( // either they have pending task or they're complete. ) { allComplete = true; - const postamble = request.postamble; - for (i = 0; i < postamble.length; i++) { - writeChunk(destination, postamble[i]); + if (enableFloat) { + const postamble: Array< + Chunk | PrecomputedChunk, + > = (request.postamble: any); + for (i = 0; i < postamble.length; i++) { + writeChunk(destination, postamble[i]); + } + postamble.length = 0; } - postamble.length = 0; } } finally { completeWriting(destination); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 33d233f92e4ef..98c9b53dcab2f 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -116,6 +116,8 @@ export const enableCPUSuspense = __EXPERIMENTAL__; // aggressiveness. export const deletedTreeCleanUpLevel = 3; +export const enableFloat = __EXPERIMENTAL__; + // ----------------------------------------------------------------------------- // Chopping Block // diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 0e07c9f67d994..3b54f76f1a525 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -81,6 +81,7 @@ export const enableUseMutableSource = true; export const enableTransitionTracing = false; export const enableSymbolFallbackForWWW = false; +export const enableFloat = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 933d914ce5a62..fc125d4110737 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -70,6 +70,7 @@ export const enableUseMutableSource = false; export const enableTransitionTracing = false; export const enableSymbolFallbackForWWW = false; +export const enableFloat = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index b218c53470bda..cbdd24ef5a901 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -70,6 +70,7 @@ export const enableUseMutableSource = false; export const enableTransitionTracing = false; export const enableSymbolFallbackForWWW = false; +export const enableFloat = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index b76a1b8506d13..ec4e9deddc726 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -68,6 +68,7 @@ export const enableUseMutableSource = false; export const enableTransitionTracing = false; export const enableSymbolFallbackForWWW = false; +export const enableFloat = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 63ba329f01a90..6fd3a292861f0 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -72,6 +72,7 @@ export const enableUseMutableSource = true; export const enableTransitionTracing = false; export const enableSymbolFallbackForWWW = false; +export const enableFloat = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 9e5a9107ab2b1..0faa78126ee5f 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -70,6 +70,7 @@ export const enableUseMutableSource = false; export const enableTransitionTracing = false; export const enableSymbolFallbackForWWW = false; +export const enableFloat = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index a4c3e1ba32678..b987da6c350b8 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -71,6 +71,7 @@ export const enableUseMutableSource = true; export const enableTransitionTracing = false; export const enableSymbolFallbackForWWW = false; +export const enableFloat = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 7a89e41ad54f2..dd6dcff47b307 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -60,5 +60,6 @@ export const disableNativeComponentFrames = false; export const createRootStrictEffectsByDefault = false; export const enableStrictEffects = false; export const allowConcurrentByDefault = true; +export const enableFloat = false; // You probably *don't* want to add more hardcoded ones. // Instead, try to add them above with the __VARIANT__ value. diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 50d078d3f0fd9..4876ac1ff30ec 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -52,6 +52,7 @@ export const enableUpdaterTracking = __PROFILE__; export const enableSuspenseAvoidThisFallback = true; export const enableSuspenseAvoidThisFallbackFizz = false; export const enableCPUSuspense = true; +export const enableFloat = false; // Logs additional User Timing API marks for use with an experimental profiling tool. export const enableSchedulingProfiler = diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 010afa06e70f3..d5f273fe1e4d5 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -420,5 +420,7 @@ "432": "The render was aborted by the server without a reason.", "433": "useId can only be used while React is rendering", "434": "`dangerouslySetInnerHTML` does not make sense on .", - "435": "Unexpected Suspense handler tag (%s). This is a bug in React." + "435": "Unexpected Suspense handler tag (%s). This is a bug in React.", + "436": "Stylesheet resources need a unique representation in the DOM while hydrating and more than one matching DOM Node was found. To fix, ensure you are only rendering one stylesheet link with an href attribute of \"%s\".", + "437": "the \"precedence\" prop for links to stylehseets expects to receive a string but received something of type \"%s\" instead." } From 52fd944511ecabd9390191b8b4989860f74c2c81 Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Tue, 9 Aug 2022 12:31:30 -0700 Subject: [PATCH 03/12] fix flakey test --- .../src/__tests__/ReactDOMFizzServer-test.js | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 8ba29d37c5f2f..1b044400e526e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -4375,16 +4375,34 @@ describe('ReactDOMFizzServer', () => { </html> </>, ); - expect(Scheduler).toFlushWithoutYielding(); - expect(getVisibleChildren(document)).toEqual( - <html> - <head> - <link rel="stylesheet" href="foo" data-rprec="default" /> - <link rel="author" precedence="this is a nonsense prop" /> - </head> - <body>a body</body> - </html>, - ); + // We manually capture uncaught errors b/c Jest does not play well with errors thrown in + // microtasks after the test completes even when it is expecting to fail (e.g. when the gate is false) + // We need to flush the scheduler at the end even if there was an earlier throw otherwise this test will + // fail even when failure is expected. This is primarily caused by invokeGuardedCallback replaying commit + // phase errors which get rethrown in a microtask + const uncaughtErrors = []; + try { + expect(Scheduler).toFlushWithoutYielding(); + expect(getVisibleChildren(document)).toEqual( + <html> + <head> + <link rel="stylesheet" href="foo" data-rprec="default" /> + <link rel="author" precedence="this is a nonsense prop" /> + </head> + <body>a body</body> + </html>, + ); + } catch (e) { + uncaughtErrors.push(e); + } + try { + expect(Scheduler).toFlushWithoutYielding(); + } catch (e) { + uncaughtErrors.push(e); + } + if (uncaughtErrors.length > 0) { + throw uncaughtErrors[0]; + } }); // @gate __DEV__ && enableFloat From 4068cd4265caa5eac79c1de9deb6efe494d0f316 Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Thu, 11 Aug 2022 09:54:32 -0700 Subject: [PATCH 04/12] refactor branching and fix allComplete bug --- .../src/__tests__/ReactDOMFizzServer-test.js | 101 +++++++++++++++--- .../src/client/ReactDOMHostConfig.js | 7 +- .../src/server/ReactDOMServerFormatConfig.js | 67 +++++++++--- .../src/ReactFiberTreeReflection.js | 22 +++- packages/react-server/src/ReactFizzServer.js | 7 +- 5 files changed, 168 insertions(+), 36 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 1b044400e526e..7785d73895685 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -4363,7 +4363,7 @@ describe('ReactDOMFizzServer', () => { ); // It hydrates successfully - ReactDOMClient.hydrateRoot( + const root = ReactDOMClient.hydrateRoot( document, <> <link rel="stylesheet" href="foo" precedence="default" /> @@ -4400,11 +4400,95 @@ describe('ReactDOMFizzServer', () => { } catch (e) { uncaughtErrors.push(e); } + + root.render( + <> + <link rel="stylesheet" href="foo" precedence="default" data-bar="bar" /> + <html> + <head /> + <body>a body</body> + </html> + </>, + ); + try { + expect(Scheduler).toFlushWithoutYielding(); + expect(getVisibleChildren(document)).toEqual( + <html> + <head> + <link + rel="stylesheet" + href="foo" + data-rprec="default" + data-bar="bar" + /> + </head> + <body>a body</body> + </html>, + ); + } catch (e) { + uncaughtErrors.push(e); + } + try { + expect(Scheduler).toFlushWithoutYielding(); + } catch (e) { + uncaughtErrors.push(e); + } + if (uncaughtErrors.length > 0) { throw uncaughtErrors[0]; } }); + // Temporarily this test is expected to fail everywhere. When we have resource hoisting + // it should start to pass and we can adjust the gate accordingly + // @gate false && enableFloat + it('should insert missing resources during hydration', async () => { + await actIntoEmptyDocument(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + <html> + <body>foo</body> + </html>, + ); + pipe(writable); + }); + + const uncaughtErrors = []; + ReactDOMClient.hydrateRoot( + document, + <> + <link rel="stylesheet" href="foo" precedence="foo" /> + <html> + <head /> + <body>foo</body> + </html> + </>, + ); + try { + expect(Scheduler).toFlushWithoutYielding(); + expect(getVisibleChildren(document)).toEqual( + <html> + <head> + <link rel="stylesheet" href="foo" precedence="foo" /> + </head> + <body>foo</body> + </html>, + ); + } catch (e) { + uncaughtErrors.push(e); + } + + // need to flush again to get the invoke guarded callback error to throw in microtask + try { + expect(Scheduler).toFlushWithoutYielding(); + } catch (e) { + uncaughtErrors.push(e); + } + + if (uncaughtErrors.length) { + throw uncaughtErrors[0]; + } + }); + // @gate __DEV__ && enableFloat it('should error in dev when rendering more than one resource for a given location (href)', async () => { await actIntoEmptyDocument(() => { @@ -4450,18 +4534,11 @@ describe('ReactDOMFizzServer', () => { ); expect(() => { expect(Scheduler).toFlushWithoutYielding(); - }).toErrorDev( - [ - 'An error occurred during hydration. The server HTML was replaced with client content in <#document>.', - ], - {withoutStack: true}, - ); - expect(errors).toEqual([ - 'Stylesheet resources need a unique representation in the DOM while hydrating and more than one matching DOM Node was found. To fix, ensure you are only rendering one stylesheet link with an href attribute of "foo".', - 'Stylesheet resources need a unique representation in the DOM while hydrating and more than one matching DOM Node was found. To fix, ensure you are only rendering one stylesheet link with an href attribute of "foo".', - 'Hydration failed because the initial UI does not match what was rendered on the server.', - 'There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering.', + }).toErrorDev([ + 'Warning: Stylesheet resources need a unique representation in the DOM while hydrating and more than one matching DOM Node was found. To fix, ensure you are only rendering one stylesheet link with an href attribute of "foo"', + 'Warning: Stylesheet resources need a unique representation in the DOM while hydrating and more than one matching DOM Node was found. To fix, ensure you are only rendering one stylesheet link with an href attribute of "foo"', ]); + expect(errors).toEqual([]); }); describe('text separators', () => { diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 69996a88fb717..52646306767ce 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -920,12 +920,11 @@ export function getMatchingResourceInstance( rootHostContainer, ).querySelectorAll(selector); if (allLinks.length > 1) { - throw new Error( + console.error( 'Stylesheet resources need a unique representation in the DOM while hydrating' + ' and more than one matching DOM Node was found. To fix, ensure you are only' + - ' rendering one stylesheet link with an href attribute of "' + - (props: any).href + - '".', + ' rendering one stylesheet link with an href attribute of "%s".', + (props: any).href, ); } } diff --git a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js index 9f94cde76af66..16924daf4e708 100644 --- a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js @@ -243,7 +243,7 @@ export function getChildFormatContext( return parentContext; } -export function isPreambleInsertion(type: string): boolean { +function isPreambleInsertion(type: string): boolean { switch (type) { case 'html': case 'head': { @@ -253,7 +253,7 @@ export function isPreambleInsertion(type: string): boolean { return false; } -export function isPostambleInsertion(type: string): boolean { +function isPostambleInsertion(type: string): boolean { switch (type) { case 'body': case 'html': { @@ -1256,6 +1256,39 @@ function pushStartTitle( return children; } +function pushStartHead( + target: Array<Chunk | PrecomputedChunk>, + preamble: ?Array<Chunk | PrecomputedChunk>, + props: Object, + tag: string, + responseState: ResponseState, +): ReactNodeList { + // Preamble type is nullable for feature off cases but is guaranteed when feature is on + target = enableFloat && isPreambleInsertion(tag) ? (preamble: any) : target; + + return pushStartGenericElement(target, props, tag, responseState); +} + +function pushStartHtml( + target: Array<Chunk | PrecomputedChunk>, + preamble: ?Array<Chunk | PrecomputedChunk>, + props: Object, + tag: string, + formatContext: FormatContext, + responseState: ResponseState, +): ReactNodeList { + // Preamble type is nullable for feature off cases but is guaranteed when feature is on + target = enableFloat && isPreambleInsertion(tag) ? (preamble: any) : target; + + if (formatContext.insertionMode === ROOT_HTML_MODE) { + // If we're rendering the html tag and we're at the root (i.e. not in foreignObject) + // then we also emit the DOCTYPE as part of the root content as a convenience for + // rendering the whole document. + target.push(DOCTYPE); + } + return pushStartGenericElement(target, props, tag, responseState); +} + function pushStartGenericElement( target: Array<Chunk | PrecomputedChunk>, props: Object, @@ -1478,8 +1511,6 @@ export function pushStartInstance( responseState: ResponseState, formatContext: FormatContext, ): ReactNodeList { - // Preamble type is nullable for feature off cases but is guaranteed when feature is on - target = enableFloat && isPreambleInsertion(type) ? (preamble: any) : target; if (__DEV__) { validateARIAProperties(type, props); validateInputProperties(type, props); @@ -1566,14 +1597,18 @@ export function pushStartInstance( case 'missing-glyph': { return pushStartGenericElement(target, props, type, responseState); } + // Preamble start tags + case 'head': + return pushStartHead(target, preamble, props, type, responseState); case 'html': { - if (formatContext.insertionMode === ROOT_HTML_MODE) { - // If we're rendering the html tag and we're at the root (i.e. not in foreignObject) - // then we also emit the DOCTYPE as part of the root content as a convenience for - // rendering the whole document. - target.push(DOCTYPE); - } - return pushStartGenericElement(target, props, type, responseState); + return pushStartHtml( + target, + preamble, + props, + type, + formatContext, + responseState, + ); } default: { if (type.indexOf('-') === -1 && typeof props.is !== 'string') { @@ -1596,9 +1631,6 @@ export function pushEndInstance( type: string, props: Object, ): void { - // Preamble type is nullable for feature off cases but is guaranteed when feature is on - target = - enableFloat && isPostambleInsertion(type) ? (postamble: any) : target; switch (type) { // Omitted close tags // TODO: Instead of repeating this switch we could try to pass a flag from above. @@ -1621,6 +1653,13 @@ export function pushEndInstance( // No close tag needed. break; } + // Postamble end tags + case 'body': + case 'html': + // Preamble type is nullable for feature off cases but is guaranteed when feature is on + target = + enableFloat && isPostambleInsertion(type) ? (postamble: any) : target; + // Intentional fallthrough default: { target.push(endTag1, stringToChunk(type), endTag2); } diff --git a/packages/react-reconciler/src/ReactFiberTreeReflection.js b/packages/react-reconciler/src/ReactFiberTreeReflection.js index 4aa5a203ae3a9..ca06e825a27d7 100644 --- a/packages/react-reconciler/src/ReactFiberTreeReflection.js +++ b/packages/react-reconciler/src/ReactFiberTreeReflection.js @@ -13,6 +13,7 @@ import type {SuspenseState} from './ReactFiberSuspenseComponent.old'; import {get as getInstance} from 'shared/ReactInstanceMap'; import ReactSharedInternals from 'shared/ReactSharedInternals'; +import {enableFloat} from 'shared/ReactFeatureFlags'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import { ClassComponent, @@ -23,6 +24,7 @@ import { SuspenseComponent, } from './ReactWorkTags'; import {NoFlags, Placement, Hydrating} from './ReactFiberFlags'; +import {supportsHydration, isHydratableResource} from './ReactFiberHostConfig'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; @@ -274,7 +276,15 @@ export function findCurrentHostFiber(parent: Fiber): Fiber | null { function findCurrentHostFiberImpl(node: Fiber) { // Next we'll drill down this component to find the first HostComponent/Text. if (node.tag === HostComponent || node.tag === HostText) { - return node; + if ( + enableFloat && + supportsHydration && + isHydratableResource(node.type, node.memoizedProps) + ) { + // This is a crude opt out of Resources from this search algorithm + } else { + return node; + } } let child = node.child; @@ -299,7 +309,15 @@ export function findCurrentHostFiberWithNoPortals(parent: Fiber): Fiber | null { function findCurrentHostFiberWithNoPortalsImpl(node: Fiber) { // Next we'll drill down this component to find the first HostComponent/Text. if (node.tag === HostComponent || node.tag === HostText) { - return node; + if ( + enableFloat && + supportsHydration && + isHydratableResource(node.type, node.memoizedProps) + ) { + // This is a crude opt out of Resources from this search algorithm + } else { + return node; + } } let child = node.child; diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index e9fc0adaae280..384c50a106330 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -2060,7 +2060,6 @@ function flushCompletedQueues( request: Request, destination: Destination, ): void { - let allComplete = false; beginWriting(destination); try { // The structure of this is to go through each queue one by one and write @@ -2159,7 +2158,8 @@ function flushCompletedQueues( } } largeBoundaries.splice(0, i); - + } finally { + let allComplete = false; if ( request.allPendingTasks === 0 && request.pingedTasks.length === 0 && @@ -2173,13 +2173,12 @@ function flushCompletedQueues( const postamble: Array< Chunk | PrecomputedChunk, > = (request.postamble: any); - for (i = 0; i < postamble.length; i++) { + for (let i = 0; i < postamble.length; i++) { writeChunk(destination, postamble[i]); } postamble.length = 0; } } - } finally { completeWriting(destination); flushBuffered(destination); if (allComplete) { From 64b95e5d5238f321e149eccca85da788558a7d14 Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Thu, 11 Aug 2022 15:01:06 -0700 Subject: [PATCH 05/12] throw if fail to hydrate a Resource --- .../src/__tests__/ReactDOMFizzServer-test.js | 45 ++++++++++++++++++- .../src/ReactFiberHydrationContext.old.js | 32 ++++++++++--- 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 7785d73895685..668d9192ef309 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -4489,7 +4489,50 @@ describe('ReactDOMFizzServer', () => { } }); - // @gate __DEV__ && enableFloat + // @gate enableFloat + it('fail hydration if a suitable resource cannot be found in the DOM for a given location (href)', async () => { + await actIntoEmptyDocument(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + <html> + <head /> + <body>a body</body> + </html>, + ); + pipe(writable); + }); + + const errors = []; + ReactDOMClient.hydrateRoot( + document, + <html> + <head> + <link rel="stylesheet" href="foo" precedence="low" /> + </head> + <body>a body</body> + </html>, + { + onRecoverableError(err, errInfo) { + errors.push(err.message); + }, + }, + ); + expect(() => { + expect(Scheduler).toFlushWithoutYielding(); + }).toErrorDev( + [ + 'Warning: A matching Hydratable Resource was not found in the DOM for <link rel="stylesheet" href="foo" >', + 'Warning: An error occurred during hydration. The server HTML was replaced with client content in <#document>.', + ], + {withoutStack: 1}, + ); + expect(errors).toEqual([ + 'Hydration failed because the initial UI does not match what was rendered on the server.', + 'Hydration failed because the initial UI does not match what was rendered on the server.', + 'There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering.', + ]); + }); + + // @gate enableFloat it('should error in dev when rendering more than one resource for a given location (href)', async () => { await actIntoEmptyDocument(() => { const {pipe} = ReactDOMFizzServer.renderToPipeableStream( diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js index 02c65dea3ef01..28371e823e5c8 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js @@ -413,15 +413,12 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void { fiber.tag === HostComponent && isHydratableResource(fiber.type, fiber.pendingProps) ) { - const resourceInstance = getMatchingResourceInstance( + fiber.stateNode = getMatchingResourceInstance( fiber.type, fiber.pendingProps, getRootHostContainer(), ); - if (resourceInstance) { - fiber.stateNode = resourceInstance; - return; - } + return; } } let nextInstance = nextHydratableInstance; @@ -616,8 +613,29 @@ function popHydrationState(fiber: Fiber): boolean { if (!supportsHydration) { return false; } - if (enableFloat && isHydratableResource(fiber.type, fiber.memoizedProps)) { - return !!fiber.stateNode; + if ( + enableFloat && + isHydrating && + isHydratableResource(fiber.type, fiber.memoizedProps) + ) { + if (fiber.stateNode === null) { + if (__DEV__) { + const rel = fiber.memoizedProps.rel + ? `rel="${fiber.memoizedProps.rel}" ` + : ''; + const href = fiber.memoizedProps.href + ? `href="${fiber.memoizedProps.href}"` + : ''; + console.error( + 'A matching Hydratable Resource was not found in the DOM for <%s %s%s>.', + fiber.type, + rel, + href, + ); + } + throwOnHydrationMismatch(fiber); + } + return true; } if (fiber !== hydrationParentFiber) { // We're deeper than the current hydration context, inside an inserted From 3ec9468d847c1557686af1f178d5e69884c60d3b Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Thu, 11 Aug 2022 15:13:06 -0700 Subject: [PATCH 06/12] gating --- .../src/__tests__/ReactDOMFizzServer-test.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 668d9192ef309..16438a63b3afd 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -4489,8 +4489,13 @@ describe('ReactDOMFizzServer', () => { } }); - // @gate enableFloat + // @gate experimental && enableFloat it('fail hydration if a suitable resource cannot be found in the DOM for a given location (href)', async () => { + gate(flags => { + if (!(__EXPERIMENTAL__ && flags.enableFloat)) { + throw new Error('bailing out of test'); + } + }); await actIntoEmptyDocument(() => { const {pipe} = ReactDOMFizzServer.renderToPipeableStream( <html> @@ -4532,8 +4537,13 @@ describe('ReactDOMFizzServer', () => { ]); }); - // @gate enableFloat + // @gate experimental && enableFloat it('should error in dev when rendering more than one resource for a given location (href)', async () => { + gate(flags => { + if (!(__EXPERIMENTAL__ && flags.enableFloat)) { + throw new Error('bailing out of test'); + } + }); await actIntoEmptyDocument(() => { const {pipe} = ReactDOMFizzServer.renderToPipeableStream( <> From 29c5e8c8742009d52a0619168adc2f87bf9e88f7 Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Thu, 11 Aug 2022 15:14:13 -0700 Subject: [PATCH 07/12] fix forks --- .../src/ReactFiberHydrationContext.new.js | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index 60b653ddd1351..23010daad4970 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -413,15 +413,12 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void { fiber.tag === HostComponent && isHydratableResource(fiber.type, fiber.pendingProps) ) { - const resourceInstance = getMatchingResourceInstance( + fiber.stateNode = getMatchingResourceInstance( fiber.type, fiber.pendingProps, getRootHostContainer(), ); - if (resourceInstance) { - fiber.stateNode = resourceInstance; - return; - } + return; } } let nextInstance = nextHydratableInstance; @@ -616,8 +613,29 @@ function popHydrationState(fiber: Fiber): boolean { if (!supportsHydration) { return false; } - if (enableFloat && isHydratableResource(fiber.type, fiber.memoizedProps)) { - return !!fiber.stateNode; + if ( + enableFloat && + isHydrating && + isHydratableResource(fiber.type, fiber.memoizedProps) + ) { + if (fiber.stateNode === null) { + if (__DEV__) { + const rel = fiber.memoizedProps.rel + ? `rel="${fiber.memoizedProps.rel}" ` + : ''; + const href = fiber.memoizedProps.href + ? `href="${fiber.memoizedProps.href}"` + : ''; + console.error( + 'A matching Hydratable Resource was not found in the DOM for <%s %s%s>.', + fiber.type, + rel, + href, + ); + } + throwOnHydrationMismatch(fiber); + } + return true; } if (fiber !== hydrationParentFiber) { // We're deeper than the current hydration context, inside an inserted From 7b71ebed97ace84748300c398ea971f0c1834f3e Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Thu, 11 Aug 2022 15:37:37 -0700 Subject: [PATCH 08/12] fix assertion --- packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 16438a63b3afd..10d3237bf38d0 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -4525,7 +4525,7 @@ describe('ReactDOMFizzServer', () => { expect(Scheduler).toFlushWithoutYielding(); }).toErrorDev( [ - 'Warning: A matching Hydratable Resource was not found in the DOM for <link rel="stylesheet" href="foo" >', + 'Warning: A matching Hydratable Resource was not found in the DOM for <link rel="stylesheet" href="foo">', 'Warning: An error occurred during hydration. The server HTML was replaced with client content in <#document>.', ], {withoutStack: 1}, From 6d94b154fcade0a0781e0103bddf8a33ef772451 Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Fri, 12 Aug 2022 11:21:47 -0700 Subject: [PATCH 09/12] only container manipulation when float is off --- packages/react-dom/src/client/ReactDOMRoot.js | 3 ++- .../src/ReactFiberTreeReflection.js | 22 ++----------------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index c522d91f21ff7..9fbed21bd1767 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -15,6 +15,7 @@ import type { import {queueExplicitHydrationTarget} from '../events/ReactDOMEventReplaying'; import {REACT_ELEMENT_TYPE} from 'shared/ReactSymbols'; +import {enableFloat} from 'shared/ReactFeatureFlags'; export type RootType = { render(children: ReactNodeList): void, @@ -118,7 +119,7 @@ ReactDOMHydrationRoot.prototype.render = ReactDOMRoot.prototype.render = functio const container = root.containerInfo; - if (container.nodeType !== COMMENT_NODE) { + if (!enableFloat && container.nodeType !== COMMENT_NODE) { const hostInstance = findHostInstanceWithNoPortals(root.current); if (hostInstance) { if (hostInstance.parentNode !== container) { diff --git a/packages/react-reconciler/src/ReactFiberTreeReflection.js b/packages/react-reconciler/src/ReactFiberTreeReflection.js index ca06e825a27d7..4aa5a203ae3a9 100644 --- a/packages/react-reconciler/src/ReactFiberTreeReflection.js +++ b/packages/react-reconciler/src/ReactFiberTreeReflection.js @@ -13,7 +13,6 @@ import type {SuspenseState} from './ReactFiberSuspenseComponent.old'; import {get as getInstance} from 'shared/ReactInstanceMap'; import ReactSharedInternals from 'shared/ReactSharedInternals'; -import {enableFloat} from 'shared/ReactFeatureFlags'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import { ClassComponent, @@ -24,7 +23,6 @@ import { SuspenseComponent, } from './ReactWorkTags'; import {NoFlags, Placement, Hydrating} from './ReactFiberFlags'; -import {supportsHydration, isHydratableResource} from './ReactFiberHostConfig'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; @@ -276,15 +274,7 @@ export function findCurrentHostFiber(parent: Fiber): Fiber | null { function findCurrentHostFiberImpl(node: Fiber) { // Next we'll drill down this component to find the first HostComponent/Text. if (node.tag === HostComponent || node.tag === HostText) { - if ( - enableFloat && - supportsHydration && - isHydratableResource(node.type, node.memoizedProps) - ) { - // This is a crude opt out of Resources from this search algorithm - } else { - return node; - } + return node; } let child = node.child; @@ -309,15 +299,7 @@ export function findCurrentHostFiberWithNoPortals(parent: Fiber): Fiber | null { function findCurrentHostFiberWithNoPortalsImpl(node: Fiber) { // Next we'll drill down this component to find the first HostComponent/Text. if (node.tag === HostComponent || node.tag === HostText) { - if ( - enableFloat && - supportsHydration && - isHydratableResource(node.type, node.memoizedProps) - ) { - // This is a crude opt out of Resources from this search algorithm - } else { - return node; - } + return node; } let child = node.child; From 4f5aa8727942afb4c5c2e780899d09da95f87203 Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Fri, 12 Aug 2022 11:26:34 -0700 Subject: [PATCH 10/12] gate test --- packages/react-dom/src/__tests__/ReactDOMRoot-test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMRoot-test.js b/packages/react-dom/src/__tests__/ReactDOMRoot-test.js index 9d6a38188376d..6418c2b6adba3 100644 --- a/packages/react-dom/src/__tests__/ReactDOMRoot-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMRoot-test.js @@ -358,6 +358,7 @@ describe('ReactDOMRoot', () => { ); }); + // @gate !enableFloat it('warns if updating a root that has had its contents removed', async () => { const root = ReactDOMClient.createRoot(container); root.render(<div>Hi</div>); From 8e69ca11bf062d143b2081c1e33173b6c9989f6a Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Fri, 12 Aug 2022 11:33:44 -0700 Subject: [PATCH 11/12] fix gate again --- packages/react-dom/src/__tests__/ReactDOMRoot-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMRoot-test.js b/packages/react-dom/src/__tests__/ReactDOMRoot-test.js index 6418c2b6adba3..0245cebd0a9b8 100644 --- a/packages/react-dom/src/__tests__/ReactDOMRoot-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMRoot-test.js @@ -358,7 +358,7 @@ describe('ReactDOMRoot', () => { ); }); - // @gate !enableFloat + // @gate !__DEV__ || !enableFloat it('warns if updating a root that has had its contents removed', async () => { const root = ReactDOMClient.createRoot(container); root.render(<div>Hi</div>); From 3d56c0eebeef689db779d8da2e03f3a9fa6ba32b Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Fri, 12 Aug 2022 12:43:50 -0700 Subject: [PATCH 12/12] nits and errors --- .../src/server/ReactDOMServerFormatConfig.js | 27 +++---------------- packages/react-server/src/ReactFizzServer.js | 13 ++++----- 2 files changed, 8 insertions(+), 32 deletions(-) diff --git a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js index 16924daf4e708..e9601b38a79a8 100644 --- a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js @@ -243,26 +243,6 @@ export function getChildFormatContext( return parentContext; } -function isPreambleInsertion(type: string): boolean { - switch (type) { - case 'html': - case 'head': { - return true; - } - } - return false; -} - -function isPostambleInsertion(type: string): boolean { - switch (type) { - case 'body': - case 'html': { - return true; - } - } - return false; -} - export type SuspenseBoundaryID = null | PrecomputedChunk; export const UNINITIALIZED_SUSPENSE_BOUNDARY_ID: SuspenseBoundaryID = null; @@ -1264,7 +1244,7 @@ function pushStartHead( responseState: ResponseState, ): ReactNodeList { // Preamble type is nullable for feature off cases but is guaranteed when feature is on - target = enableFloat && isPreambleInsertion(tag) ? (preamble: any) : target; + target = enableFloat ? (preamble: any) : target; return pushStartGenericElement(target, props, tag, responseState); } @@ -1278,7 +1258,7 @@ function pushStartHtml( responseState: ResponseState, ): ReactNodeList { // Preamble type is nullable for feature off cases but is guaranteed when feature is on - target = enableFloat && isPreambleInsertion(tag) ? (preamble: any) : target; + target = enableFloat ? (preamble: any) : target; if (formatContext.insertionMode === ROOT_HTML_MODE) { // If we're rendering the html tag and we're at the root (i.e. not in foreignObject) @@ -1657,8 +1637,7 @@ export function pushEndInstance( case 'body': case 'html': // Preamble type is nullable for feature off cases but is guaranteed when feature is on - target = - enableFloat && isPostambleInsertion(type) ? (postamble: any) : target; + target = enableFloat ? (postamble: any) : target; // Intentional fallthrough default: { target.push(endTag1, stringToChunk(type), endTag2); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 384c50a106330..84db02300fb82 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -2081,7 +2081,6 @@ function flushCompletedQueues( // we expect the preamble to be tiny and will ignore backpressure writeChunk(destination, preamble[i]); } - preamble.length = 0; } flushSegment(request, destination, completedRootSegment); @@ -2159,7 +2158,6 @@ function flushCompletedQueues( } largeBoundaries.splice(0, i); } finally { - let allComplete = false; if ( request.allPendingTasks === 0 && request.pingedTasks.length === 0 && @@ -2168,7 +2166,6 @@ function flushCompletedQueues( // We don't need to check any partially completed segments because // either they have pending task or they're complete. ) { - allComplete = true; if (enableFloat) { const postamble: Array< Chunk | PrecomputedChunk, @@ -2176,12 +2173,9 @@ function flushCompletedQueues( for (let i = 0; i < postamble.length; i++) { writeChunk(destination, postamble[i]); } - postamble.length = 0; } - } - completeWriting(destination); - flushBuffered(destination); - if (allComplete) { + completeWriting(destination); + flushBuffered(destination); if (__DEV__) { if (request.abortableTasks.size !== 0) { console.error( @@ -2191,6 +2185,9 @@ function flushCompletedQueues( } // We're done. close(destination); + } else { + completeWriting(destination); + flushBuffered(destination); } } }