determine preamble while flushing and make postamble resilient to asy…

…nc render order
facebook · gnoff · Sep 30, 2022 · Jun 10, 2022 · Sep 28, 2022 · Sep 28, 2022
commit 589485d0bacf889c970094a89d2bf8d5be9cf6c8
diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js
@@ -7,6 +7,7 @@
  * @flow
  */
 
+import type {ArrayWithPreamble} from 'react-server/src/ReactFizzServer';
 import type {ReactNodeList} from 'shared/ReactTypes';
 import type {Resources, BoundaryResources} from './ReactDOMFloatServer';
 export type {Resources, BoundaryResources};
@@ -93,6 +94,8 @@ export type ResponseState = {
   ...
 };
 
+export const emptyChunk = stringToPrecomputedChunk('');
+
 const startInlineScript = stringToPrecomputedChunk('<script>');
 const endInlineScript = stringToPrecomputedChunk('</script>');
 
@@ -283,25 +286,6 @@ export function getChildFormatContext(
     return createFormatContext(HTML_MODE, null);
   }
   if (parentContext.insertionMode === ROOT_HTML_MODE) {
-    switch (type) {
-      case 'html': {
-        return parentContext;
-      }
-      case 'head':
-      case 'title':
-      case 'base':
-      case 'link':
-      case 'style':
-      case 'meta':
-      case 'script':
-      case 'noscript':
-      case 'template': {
-        break;
-      }
-      default: {
-        parentContext.preambleOpen = false;
-      }
-    }
     // We've emitted the root and is now in plain HTML mode.
     return createFormatContext(HTML_MODE, null);
   }
@@ -1316,40 +1300,33 @@ function pushStartTitle(
 }
 
 function pushStartHead(
-  target: Array<Chunk | PrecomputedChunk>,
-  preamble: Array<Chunk | PrecomputedChunk>,
+  target: ArrayWithPreamble<Chunk | PrecomputedChunk>,
   props: Object,
   tag: string,
   responseState: ResponseState,
   formatContext: FormatContext,
 ): ReactNodeList {
-  // Preamble type is nullable for feature off cases but is guaranteed when feature is on
-  target =
-    enableFloat &&
-    formatContext.insertionMode === ROOT_HTML_MODE &&
-    formatContext.preambleOpen
-      ? preamble
-      : target;
-
-  return pushStartGenericElement(target, props, tag, responseState);
+  const children = pushStartGenericElement(target, props, tag, responseState);
+  target._preambleIndex = target.length;
+  return children;
 }
 
 function pushStartHtml(
-  target: Array<Chunk | PrecomputedChunk>,
-  preamble: Array<Chunk | PrecomputedChunk>,
+  target: ArrayWithPreamble<Chunk | PrecomputedChunk>,
   props: Object,
   tag: string,
   responseState: ResponseState,
   formatContext: FormatContext,
 ): ReactNodeList {
   if (formatContext.insertionMode === ROOT_HTML_MODE) {
-    target = enableFloat && formatContext.preambleOpen ? preamble : target;
     // 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);
+  const children = pushStartGenericElement(target, props, tag, responseState);
+  target._preambleIndex = target.length;
+  return children;
 }
 
 function pushStartGenericElement(
@@ -1567,8 +1544,7 @@ function startChunkForTag(tag: string): PrecomputedChunk {
 const DOCTYPE: PrecomputedChunk = stringToPrecomputedChunk('<!DOCTYPE html>');
 
 export function pushStartInstance(
-  target: Array<Chunk | PrecomputedChunk>,
-  preamble: Array<Chunk | PrecomputedChunk>,
+  target: ArrayWithPreamble<Chunk | PrecomputedChunk>,
   type: string,
   props: Object,
   responseState: ResponseState,
@@ -1663,23 +1639,9 @@ export function pushStartInstance(
     }
     // Preamble start tags
     case 'head':
-      return pushStartHead(
-        target,
-        preamble,
-        props,
-        type,
-        responseState,
-        formatContext,
-      );
+      return pushStartHead(target, props, type, responseState, formatContext);
     case 'html': {
-      return pushStartHtml(
-        target,
-        preamble,
-        props,
-        type,
-        responseState,
-        formatContext,
-      );
+      return pushStartHtml(target, props, type, responseState, formatContext);
     }
     default: {
       if (type.indexOf('-') === -1 && typeof props.is !== 'string') {
@@ -1722,17 +1684,24 @@ export function pushEndInstance(
     case 'track':
     case 'wbr': {
       // No close tag needed.
-      break;
+      return;
     }
     // Postamble end tags
-    case 'body':
-    case 'html':
-      target = enableFloat ? postamble : target;
-    // Intentional fallthrough
-    default: {
-      target.push(endTag1, stringToChunk(type), endTag2);
+    case 'body': {
+      if (enableFloat) {
+        postamble.unshift(endTag1, stringToChunk(type), endTag2);
+        return;
+      }
+      break;
     }
+    case 'html':
+      if (enableFloat) {
+        postamble.push(endTag1, stringToChunk(type), endTag2);
+        return;
+      }
+      break;
   }
+  target.push(endTag1, stringToChunk(type), endTag2);
 }
 
 export function writeCompletedRoot(

diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerLegacyFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerLegacyFormatConfig.js
@@ -110,6 +110,7 @@ export {
   setCurrentlyRenderingBoundaryResources,
   prepareToRender,
   cleanupAfterRender,
+  emptyChunk,
 } from './ReactDOMServerFormatConfig';
 
 import {stringToChunk} from 'react-server/src/ReactServerStreamConfig';

diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
@@ -4320,70 +4320,129 @@ describe('ReactDOMFizzServer', () => {
   });
 
   // @gate enableFloat
-  it('emits html and head start tags (the preamble) before other content if rendered in the shell', async () => {
+  it('can emit the preamble even if the head renders asynchronously', async () => {
+    function AsyncNoOutput() {
+      readText('nooutput');
+      return null;
+    }
+    function AsyncHead() {
+      readText('head');
+      return (
+        <head data-foo="foo">
+          <title>a title</title>
+        </head>
+      );
+    }
+    function AsyncBody() {
+      readText('body');
+      return (
+        <body data-bar="bar">
+          <link rel="preload" as="style" href="foo" />
+          hello
+        </body>
+      );
+    }
     await actIntoEmptyDocument(() => {
       const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
-        <>
-          <title data-baz="baz">a title</title>
-          <html data-foo="foo">
-            <head data-bar="bar" />
-            <body>a body</body>
-          </html>
-        </>,
+        <html data-html="html">
+          <AsyncNoOutput />
+          <AsyncHead />
+          <AsyncBody />
+        </html>,
       );
       pipe(writable);
     });
+    await actIntoEmptyDocument(() => {
+      resolveText('body');
+    });
+    await actIntoEmptyDocument(() => {
+      resolveText('nooutput');
+    });
+    // We need to use actIntoEmptyDocument because act assumes that buffered
+    // content should be fake streamed into the body which is normally true
+    // but in this test the entire shell was delayed and we need the initial
+    // construction to be done to get the parsing right
+    await actIntoEmptyDocument(() => {
+      resolveText('head');
+    });
     expect(getVisibleChildren(document)).toEqual(
-      <html data-foo="foo">
-        <head data-bar="bar">
-          <title data-baz="baz">a title</title>
+      <html data-html="html">
+        <head data-foo="foo">
+          <link rel="preload" as="style" href="foo" />
+          <title>a title</title>
         </head>
-        <body>a body</body>
+        <body data-bar="bar">hello</body>
       </html>,
     );
+  });
 
-    // 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 = [];
-    ReactDOMClient.hydrateRoot(
-      document,
-      <>
-        <title data-baz="baz">a title</title>
-        <html data-foo="foo">
-          <head data-bar="bar" />
-          <body>a body</body>
-        </html>
-      </>,
-      {
-        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 <title> 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},
+  // @gate enableFloat
+  it('does not emit as preamble after the first non-preamble chunk', async () => {
+    function AsyncNoOutput() {
+      readText('nooutput');
+      return null;
+    }
+    function AsyncHead() {
+      readText('head');
+      return (
+        <head data-foo="foo">
+          <title>a title</title>
+        </head>
+      );
+    }
+    function AsyncBody() {
+      readText('body');
+      return (
+        <body data-bar="bar">
+          <link rel="preload" as="style" href="foo" />
+          hello
+        </body>
+      );
+    }
+    await actIntoEmptyDocument(() => {
+      const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
+        <html data-html="html">
+          <AsyncNoOutput />
+          <AsyncBody />
+          <AsyncHead />
+        </html>,
+      );
+      pipe(writable);
+    });
+    await actIntoEmptyDocument(() => {
+      resolveText('body');
+    });
+    await actIntoEmptyDocument(() => {
+      resolveText('nooutput');
+    });
+    // We need to use actIntoEmptyDocument because act assumes that buffered
+    // content should be fake streamed into the body which is normally true
+    // but in this test the entire shell was delayed and we need the initial
+    // construction to be done to get the parsing right
+    await actIntoEmptyDocument(() => {
+      resolveText('head');
+    });
+    // This assertion is a little strange. The html open tag is part of the preamble
+    // but since the next chunk will be the body open tag which is not preamble it
+    // emits resources. The browser understands that the link is part of the head and
+    // constructs the head implicitly which is why it does not have the data-foo attribute.
+    // When the head finally streams in it is inside the body rather than after it because the
+    // body closing tag is part of the postamble which stays open until the entire request
+    // has flushed. This is how the browser would interpret a late head arriving after the
+    // the body closing tag so while strange it is the expected behavior. One other oddity
+    // is that <head> in body is elided by html parsers so we end up with just an inlined
+    // style tag.
+    expect(getVisibleChildren(document)).toEqual(
+      <html data-html="html">
+        <head>
+          <link rel="preload" as="style" href="foo" />
+        </head>
+        <body data-bar="bar">
+          hello
+          <title>a title</title>
+        </body>
+      </html>,
     );
-    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.',
-    ]);
-    expect(getVisibleChildren(document)).toEqual();
-    expect(() => {
-      expect(Scheduler).toFlushWithoutYielding();
-    }).toThrow('The node to be removed is not a child of this node.');
   });
 
   // @gate enableFloat

diff --git a/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js b/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js
@@ -65,6 +65,8 @@ export type ResponseState = {
   nextSuspenseID: number,
 };
 
+export const emptyChunk = stringToPrecomputedChunk('');
+
 // Allows us to keep track of what we've already written so we can refer back to it.
 export function createResponseState(): ResponseState {
   return {
@@ -140,7 +142,6 @@ export function pushTextInstance(
 
 export function pushStartInstance(
   target: Array<Chunk | PrecomputedChunk>,
-  preamble: Array<Chunk | PrecomputedChunk>,
   type: string,
   props: Object,
   responseState: ResponseState,

diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js
@@ -116,7 +116,6 @@ const ReactNoopServer = ReactFizzServer({
   },
   pushStartInstance(
     target: Array<Uint8Array>,
-    preamble: Array<Uint8Array>,
     type: string,
     props: Object,
   ): ReactNodeList {