Error and deopt from rel=expect for large documents without boundaries

Author: sebmarkbage

packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

@@ -5464,7 +5464,7 @@ export function writePreambleStart(
   destination: Destination,
   resumableState: ResumableState,
   renderState: RenderState,
-  skipExpect?: boolean, // Used as an override by ReactFizzConfigMarkup
+  skipBlockingShell: boolean,
 ): void {
   // This function must be called exactly once on every request
   if (enableFizzExternalRuntime && renderState.externalRuntimeScript) {
@@ -5548,14 +5548,18 @@ export function writePreambleStart(
   renderState.bulkPreloads.forEach(flushResource, destination);
   renderState.bulkPreloads.clear();
 
-  if ((htmlChunks || headChunks) && !skipExpect) {
+  if ((htmlChunks || headChunks) && !skipBlockingShell) {
     // If we have any html or head chunks we know that we're rendering a full document.
     // A full document should block display until the full shell has downloaded.
     // Therefore we insert a render blocking instruction referring to the last body
     // element that's considered part of the shell. We do this after the important loads
     // have already been emitted so we don't do anything to delay them but early so that
     // the browser doesn't risk painting too early.
     writeBlockingRenderInstruction(destination, resumableState, renderState);
+  } else {
+    // We don't need to add the shell id so mark it as if sent.
+    // Currently it might still be sent if it was already added to a bootstrap script.
+    resumableState.instructions |= SentCompletedShellId;
   }
 
   // Write embedding hoistableChunks

packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js

@@ -27,6 +27,7 @@ import {
   writeStartClientRenderedSuspenseBoundary as writeStartClientRenderedSuspenseBoundaryImpl,
   writeEndCompletedSuspenseBoundary as writeEndCompletedSuspenseBoundaryImpl,
   writeEndClientRenderedSuspenseBoundary as writeEndClientRenderedSuspenseBoundaryImpl,
+  writePreambleStart as writePreambleStartImpl,
 } from './ReactFizzConfigDOM';
 
 import type {
@@ -170,7 +171,6 @@ export {
   createResumableState,
   createPreambleState,
   createHoistableState,
-  writePreambleStart,
   writePreambleEnd,
   writeHoistables,
   writePostamble,
@@ -311,5 +311,19 @@ export function writeEndClientRenderedSuspenseBoundary(
   return writeEndClientRenderedSuspenseBoundaryImpl(destination, renderState);
 }
 
+export function writePreambleStart(
+  destination: Destination,
+  resumableState: ResumableState,
+  renderState: RenderState,
+  skipBlockingShell: boolean,
+): void {
+  return writePreambleStartImpl(
+    destination,
+    resumableState,
+    renderState,
+    true, // skipBlockingShell
+  );
+}
+
 export type TransitionStatus = FormStatus;
 export const NotPendingTransition: TransitionStatus = NotPending;

packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js

@@ -18,12 +18,14 @@ global.AsyncLocalStorage = require('async_hooks').AsyncLocalStorage;
 let React;
 let ReactDOM;
 let ReactDOMFizzServer;
+let Suspense;
 
 describe('ReactDOMFizzServerEdge', () => {
   beforeEach(() => {
     jest.resetModules();
     jest.useRealTimers();
     React = require('react');
+    Suspense = React.Suspense;
     ReactDOM = require('react-dom');
     ReactDOMFizzServer = require('react-dom/server.edge');
   });
@@ -81,4 +83,102 @@ describe('ReactDOMFizzServerEdge', () => {
       );
     }
   });
+
+  it('recoverably errors and does not add rel="expect" for large shells', async () => {
+    function Paragraph() {
+      return (
+        <p>
+          Lorem ipsum dolor sit amet, consectetur adipiscing elit. Mauris
+          porttitor tortor ac lectus faucibus, eget eleifend elit hendrerit.
+          Integer porttitor nisi in leo congue rutrum. Morbi sed ante posuere,
+          aliquam lorem ac, imperdiet orci. Duis malesuada gravida pharetra.
+          Cras facilisis arcu diam, id dictum lorem imperdiet a. Suspendisse
+          aliquet tempus tortor et ultricies. Aliquam libero velit, posuere
+          tempus ante sed, pellentesque tincidunt lorem. Nullam iaculis, eros a
+          varius aliquet, tortor felis tempor metus, nec cursus felis eros
+          aliquam nulla. Vivamus ut orci sed mauris congue lacinia. Cras eget
+          blandit neque. Pellentesque a massa in turpis ullamcorper volutpat vel
+          at massa. Sed ante est, auctor non diam non, vulputate ultrices metus.
+          Maecenas dictum fermentum quam id aliquam. Donec porta risus vitae
+          pretium posuere. Fusce facilisis eros in lacus tincidunt congue.
+        </p>
+      );
+    }
+
+    function App({suspense}) {
+      const paragraphs = [];
+      for (let i = 0; i < 600; i++) {
+        paragraphs.push(<Paragraph key={i} />);
+      }
+      return (
+        <html>
+          <body>
+            {suspense ? (
+              // This is ok
+              <Suspense fallback="Loading">{paragraphs}</Suspense>
+            ) : (
+              // This is not
+              paragraphs
+            )}
+          </body>
+        </html>
+      );
+    }
+    const errors = [];
+    const stream = await ReactDOMFizzServer.renderToReadableStream(
+      <App suspense={false} />,
+      {
+        onError(error) {
+          errors.push(error);
+        },
+      },
+    );
+    const result = await readResult(stream);
+    expect(result).not.toContain('rel="expect"');
+    if (gate(flags => flags.enableFizzBlockingRender)) {
+      expect(errors.length).toBe(1);
+      expect(errors[0].message).toContain(
+        'This rendered a large document (>512) without any Suspense boundaries around most of it.',
+      );
+    } else {
+      expect(errors.length).toBe(0);
+    }
+
+    // If we wrap in a Suspense boundary though, then it should be ok.
+    const errors2 = [];
+    const stream2 = await ReactDOMFizzServer.renderToReadableStream(
+      <App suspense={true} />,
+      {
+        onError(error) {
+          errors2.push(error);
+        },
+      },
+    );
+    const result2 = await readResult(stream2);
+    if (gate(flags => flags.enableFizzBlockingRender)) {
+      expect(result2).toContain('rel="expect"');
+    } else {
+      expect(result2).not.toContain('rel="expect"');
+    }
+    expect(errors2.length).toBe(0);
+
+    // Or if we increase the progressiveChunkSize.
+    const errors3 = [];
+    const stream3 = await ReactDOMFizzServer.renderToReadableStream(
+      <App suspense={false} />,
+      {
+        progressiveChunkSize: 100000,
+        onError(error) {
+          errors3.push(error);
+        },
+      },
+    );
+    const result3 = await readResult(stream3);
+    if (gate(flags => flags.enableFizzBlockingRender)) {
+      expect(result3).toContain('rel="expect"');
+    } else {
+      expect(result3).not.toContain('rel="expect"');
+    }
+    expect(errors3.length).toBe(0);
+  });
 });

packages/react-dom/src/__tests__/ReactRenderDocument-test.js

@@ -70,29 +70,20 @@ describe('rendering React components at document', () => {
 
       const markup = ReactDOMServer.renderToString(<Root hello="world" />);
       expect(markup).not.toContain('DOCTYPE');
+      expect(markup).not.toContain('rel="expect"');
       const testDocument = getTestDocument(markup);
       const body = testDocument.body;
 
       let root;
       await act(() => {
         root = ReactDOMClient.hydrateRoot(testDocument, <Root hello="world" />);
       });
-      expect(testDocument.body.innerHTML).toBe(
-        'Hello world' +
-          (gate(flags => flags.enableFizzBlockingRender)
-            ? '<template id="_R_"></template>'
-            : ''),
-      );
+      expect(testDocument.body.innerHTML).toBe('Hello world');
 
       await act(() => {
         root.render(<Root hello="moon" />);
       });
-      expect(testDocument.body.innerHTML).toBe(
-        'Hello moon' +
-          (gate(flags => flags.enableFizzBlockingRender)
-            ? '<template id="_R_"></template>'
-            : ''),
-      );
+      expect(testDocument.body.innerHTML).toBe('Hello moon');
 
       expect(body === testDocument.body).toBe(true);
     });
@@ -117,12 +108,7 @@ describe('rendering React components at document', () => {
       await act(() => {
         root = ReactDOMClient.hydrateRoot(testDocument, <Root />);
       });
-      expect(testDocument.body.innerHTML).toBe(
-        'Hello world' +
-          (gate(flags => flags.enableFizzBlockingRender)
-            ? '<template id="_R_"></template>'
-            : ''),
-      );
+      expect(testDocument.body.innerHTML).toBe('Hello world');
 
       const originalDocEl = testDocument.documentElement;
       const originalHead = testDocument.head;
@@ -133,16 +119,8 @@ describe('rendering React components at document', () => {
       expect(testDocument.firstChild).toBe(originalDocEl);
       expect(testDocument.head).toBe(originalHead);
       expect(testDocument.body).toBe(originalBody);
-      expect(originalBody.innerHTML).toBe(
-        gate(flags => flags.enableFizzBlockingRender)
-          ? '<template id="_R_"></template>'
-          : '',
-      );
-      expect(originalHead.innerHTML).toBe(
-        gate(flags => flags.enableFizzBlockingRender)
-          ? '<link rel="expect" href="#_R_" blocking="render">'
-          : '',
-      );
+      expect(originalBody.innerHTML).toBe('');
+      expect(originalHead.innerHTML).toBe('');
     });
 
     it('should not be able to switch root constructors', async () => {
@@ -180,22 +158,13 @@ describe('rendering React components at document', () => {
         root = ReactDOMClient.hydrateRoot(testDocument, <Component />);
       });
 
-      expect(testDocument.body.innerHTML).toBe(
-        'Hello world' +
-          (gate(flags => flags.enableFizzBlockingRender)
-            ? '<template id="_R_"></template>'
-            : ''),
-      );
+      expect(testDocument.body.innerHTML).toBe('Hello world');
 
       await act(() => {
         root.render(<Component2 />);
       });
 
-      expect(testDocument.body.innerHTML).toBe(
-        (gate(flags => flags.enableFizzBlockingRender)
-          ? '<template id="_R_"></template>'
-          : '') + 'Goodbye world',
-      );
+      expect(testDocument.body.innerHTML).toBe('Goodbye world');
     });
 
     it('should be able to mount into document', async () => {
@@ -224,12 +193,7 @@ describe('rendering React components at document', () => {
         );
       });
 
-      expect(testDocument.body.innerHTML).toBe(
-        'Hello world' +
-          (gate(flags => flags.enableFizzBlockingRender)
-            ? '<template id="_R_"></template>'
-            : ''),
-      );
+      expect(testDocument.body.innerHTML).toBe('Hello world');
     });
 
     it('cannot render over an existing text child at the root', async () => {
@@ -362,12 +326,7 @@ describe('rendering React components at document', () => {
           : [],
       );
       expect(testDocument.body.innerHTML).toBe(
-        favorSafetyOverHydrationPerf
-          ? 'Hello world'
-          : 'Goodbye world' +
-              (gate(flags => flags.enableFizzBlockingRender)
-                ? '<template id="_R_"></template>'
-                : ''),
+        favorSafetyOverHydrationPerf ? 'Hello world' : 'Goodbye world',
       );
     });
 

packages/react-markup/src/ReactFizzConfigMarkup.js

@@ -222,13 +222,13 @@ export function writePreambleStart(
   destination: Destination,
   resumableState: ResumableState,
   renderState: RenderState,
-  skipExpect?: boolean, // Used as an override by ReactFizzConfigMarkup
+  skipBlockingShell: boolean,
 ): void {
   return writePreambleStartImpl(
     destination,
     resumableState,
     renderState,
-    true, // skipExpect
+    true, // skipBlockingShell
   );
 }