diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 44d3f20a61..eb8c278615 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -5465,7 +5465,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) { @@ -5549,7 +5549,7 @@ 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 @@ -5557,6 +5557,10 @@ export function writePreambleStart( // 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 diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js index 1489c35353..6ab54af00f 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js +++ b/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; diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js index c5cbe8bb85..91f0774aa6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js +++ b/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 ( +

+ 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. +

+ ); + } + + function App({suspense}) { + const paragraphs = []; + for (let i = 0; i < 600; i++) { + paragraphs.push(); + } + return ( + + + {suspense ? ( + // This is ok + {paragraphs} + ) : ( + // This is not + paragraphs + )} + + + ); + } + const errors = []; + const stream = await ReactDOMFizzServer.renderToReadableStream( + , + { + 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( + , + { + 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( + , + { + 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); + }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js index 2c4972245c..5b22c6364f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js @@ -35,13 +35,7 @@ describe('ReactDOMFloat', () => { expect(result).toEqual( '' + - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : '') + 'title' + - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : '') + '', ); }); diff --git a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js index 9a485f8ddd..f53354073e 100644 --- a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js +++ b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js @@ -70,6 +70,7 @@ describe('rendering React components at document', () => { const markup = ReactDOMServer.renderToString(); expect(markup).not.toContain('DOCTYPE'); + expect(markup).not.toContain('rel="expect"'); const testDocument = getTestDocument(markup); const body = testDocument.body; @@ -77,22 +78,12 @@ describe('rendering React components at document', () => { await act(() => { root = ReactDOMClient.hydrateRoot(testDocument, ); }); - expect(testDocument.body.innerHTML).toBe( - 'Hello world' + - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : ''), - ); + expect(testDocument.body.innerHTML).toBe('Hello world'); await act(() => { root.render(); }); - expect(testDocument.body.innerHTML).toBe( - 'Hello moon' + - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : ''), - ); + 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, ); }); - expect(testDocument.body.innerHTML).toBe( - 'Hello world' + - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : ''), - ); + 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) - ? '' - : '', - ); - expect(originalHead.innerHTML).toBe( - gate(flags => flags.enableFizzBlockingRender) - ? '' - : '', - ); + 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, ); }); - expect(testDocument.body.innerHTML).toBe( - 'Hello world' + - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : ''), - ); + expect(testDocument.body.innerHTML).toBe('Hello world'); await act(() => { root.render(); }); - expect(testDocument.body.innerHTML).toBe( - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : '') + '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) - ? '' - : ''), - ); + 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) - ? '' - : ''), + favorSafetyOverHydrationPerf ? 'Hello world' : 'Goodbye world', ); }); diff --git a/packages/react-markup/src/ReactFizzConfigMarkup.js b/packages/react-markup/src/ReactFizzConfigMarkup.js index bbca0d4ddf..fee02f320f 100644 --- a/packages/react-markup/src/ReactFizzConfigMarkup.js +++ b/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 ); } diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 579edf25c9..19860614ad 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -182,6 +182,7 @@ import { disableDefaultPropsExceptForClasses, enableAsyncIterableChildren, enableViewTransition, + enableFizzBlockingRender, } from 'shared/ReactFeatureFlags'; import assign from 'shared/assign'; @@ -418,6 +419,41 @@ type Preamble = PreambleState; // 500 * 1024 / 8 * .8 * 0.5 / 2 const DEFAULT_PROGRESSIVE_CHUNK_SIZE = 12800; +function getBlockingRenderMaxSize(request: Request): number { + // We want to make sure that we can block the reveal of a well designed complete + // shell but if you have constructed a too large shell (e.g. by not adding any + // Suspense boundaries) then that might take too long to render. We shouldn't + // punish users (or overzealous metrics tracking) in that scenario. + // There's a trade off here. If this limit is too low then you can't fit a + // reasonably well built UI within it without getting errors. If it's too high + // then things that accidentally fall below it might take too long to load. + // Web Vitals target 1.8 seconds for first paint and our goal to have the limit + // be fast enough to hit that. For this argument we assume that most external + // resources are already cached because it's a return visit, or inline styles. + // If it's not, then it's highly unlikely that any render blocking instructions + // we add has any impact what so ever on the paint. + // Assuming a first byte of about 600ms which is kind of bad but common with a + // decent static host. If it's longer e.g. due to dynamic rendering, then you + // are going to bound by dynamic production of the content and you're better off + // with Suspense boundaries anyway. This number doesn't matter much. Then you + // have about 1.2 seconds left for bandwidth. On 3G that gives you about 112.5kb + // worth of data. That's worth about 10x in terms of uncompressed bytes. Then we + // half that just to account for longer latency, slower bandwidth and CPU processing. + // Now we're down to about 500kb. In fact, looking at metrics we've collected with + // rel="expect" examples and other documents, the impact on documents smaller than + // that is within the noise. That's because there's enough happening within that + // start up to not make HTML streaming not significantly better. + // Content above the fold tends to be about 100-200kb tops. Therefore 500kb should + // be enough head room for a good loading state. After that you should use + // Suspense or SuspenseList to improve it. + // Since this is highly related to the reason you would adjust the + // progressiveChunkSize option, and always has to be higher, we define this limit + // in terms of it. So if you want to increase the limit because you have high + // bandwidth users, then you can adjust it up. If you are concerned about even + // slower bandwidth then you can adjust it down. + return request.progressiveChunkSize * 40; // 512kb by default. +} + function isEligibleForOutlining( request: Request, boundary: SuspenseBoundary, @@ -5476,9 +5512,15 @@ function flushPreamble( destination: Destination, rootSegment: Segment, preambleSegments: Array>, + skipBlockingShell: boolean, ) { // The preamble is ready. - writePreambleStart(destination, request.resumableState, request.renderState); + writePreambleStart( + destination, + request.resumableState, + request.renderState, + skipBlockingShell, + ); for (let i = 0; i < preambleSegments.length; i++) { const segments = preambleSegments[i]; for (let j = 0; j < segments.length; j++) { @@ -5888,11 +5930,32 @@ function flushCompletedQueues( flushedByteSize = request.byteSize; // Start counting bytes // TODO: Count the size of the preamble chunks too. + let skipBlockingShell = false; + if (enableFizzBlockingRender) { + const blockingRenderMaxSize = getBlockingRenderMaxSize(request); + if (flushedByteSize > blockingRenderMaxSize) { + skipBlockingShell = true; + const maxSizeKb = Math.round(blockingRenderMaxSize / 1000); + const error = new Error( + 'This rendered a large document (>' + + maxSizeKb + + ') without any Suspense ' + + 'boundaries around most of it. That can delay initial paint longer than ' + + 'necessary. To improve load performance, add a or ' + + 'around the content you expect to be below the header or below the fold. ' + + 'In the meantime, the content will deopt to paint arbitrary incomplete ' + + 'pieces of HTML.', + ); + const errorInfo: ThrownInfo = {}; + logRecoverableError(request, error, errorInfo, null); + } + } flushPreamble( request, destination, completedRootSegment, completedPreambleSegments, + skipBlockingShell, ); flushSegment(request, destination, completedRootSegment, null); request.completedRootSegment = null; diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 3c64fcc471..7e709903a8 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -545,5 +545,6 @@ "557": "Expected to have a hydrated activity instance. This error is likely caused by a bug in React. Please file an issue.", "558": "Client rendering an Activity suspended it again. This is a bug in React.", "559": "Expected to find a host node. This is a bug in React.", - "560": "Cannot use a startGestureTransition() with a comment node root." + "560": "Cannot use a startGestureTransition() with a comment node root.", + "561": "This rendered a large document (>%s) without any Suspense boundaries around most of it. That can delay initial paint longer than necessary. To improve load performance, add a or around the content you expect to be below the header or below the fold. In the meantime, the content will deopt to paint arbitrary incomplete pieces of HTML." }