Skip to content

Commit d177272

Browse files
authored
[Fizz] Error and deopt from rel=expect for large documents without boundaries (#33454)
We want to make sure that we can block the reveal of a well designed complete shell reliably. In the Suspense model, client transitions don't have any way to implicitly resolve. This means you need to use Suspense or SuspenseList to explicitly split the document. Relying on implicit would mean you can't add a Suspense boundary later where needed. So we highly encourage the use of them around large content. However, if you have constructed a too large shell (e.g. by not adding any Suspense boundaries at all) then that might take too long to render on the client. We shouldn't punish users (or overzealous metrics tracking tools like search engines) in that scenario. This opts out of render blocking if the shell ends up too large to be intentional and too slow to load. Instead it deopts to showing the content split up in arbitrary ways (browser default). It only does this for SSR, and not client navs so it's not reliable. In fact, we issue an error to `onError`. This error is recoverable in that the document is still produced. It's up to your framework to decide if this errors the build or just surface it for action later. What should be the limit though? 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. I came up with 512kB of uncompressed shell HTML. See the comment in code for the rationale for this number. TL;DR: Data and theory indicates that having this much content inside `rel="expect"` doesn't meaningfully change metrics. Research of above-the-fold content on various websites indicate that this can comfortable fit all of them which should be enough for any intentional initial paint.
1 parent 22b9291 commit d177272

File tree

8 files changed

+199
-64
lines changed

8 files changed

+199
-64
lines changed

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5465,7 +5465,7 @@ export function writePreambleStart(
54655465
destination: Destination,
54665466
resumableState: ResumableState,
54675467
renderState: RenderState,
5468-
skipExpect?: boolean, // Used as an override by ReactFizzConfigMarkup
5468+
skipBlockingShell: boolean,
54695469
): void {
54705470
// This function must be called exactly once on every request
54715471
if (enableFizzExternalRuntime && renderState.externalRuntimeScript) {
@@ -5549,14 +5549,18 @@ export function writePreambleStart(
55495549
renderState.bulkPreloads.forEach(flushResource, destination);
55505550
renderState.bulkPreloads.clear();
55515551

5552-
if ((htmlChunks || headChunks) && !skipExpect) {
5552+
if ((htmlChunks || headChunks) && !skipBlockingShell) {
55535553
// If we have any html or head chunks we know that we're rendering a full document.
55545554
// A full document should block display until the full shell has downloaded.
55555555
// Therefore we insert a render blocking instruction referring to the last body
55565556
// element that's considered part of the shell. We do this after the important loads
55575557
// have already been emitted so we don't do anything to delay them but early so that
55585558
// the browser doesn't risk painting too early.
55595559
writeBlockingRenderInstruction(destination, resumableState, renderState);
5560+
} else {
5561+
// We don't need to add the shell id so mark it as if sent.
5562+
// Currently it might still be sent if it was already added to a bootstrap script.
5563+
resumableState.instructions |= SentCompletedShellId;
55605564
}
55615565

55625566
// Write embedding hoistableChunks

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
writeStartClientRenderedSuspenseBoundary as writeStartClientRenderedSuspenseBoundaryImpl,
2828
writeEndCompletedSuspenseBoundary as writeEndCompletedSuspenseBoundaryImpl,
2929
writeEndClientRenderedSuspenseBoundary as writeEndClientRenderedSuspenseBoundaryImpl,
30+
writePreambleStart as writePreambleStartImpl,
3031
} from './ReactFizzConfigDOM';
3132

3233
import type {
@@ -170,7 +171,6 @@ export {
170171
createResumableState,
171172
createPreambleState,
172173
createHoistableState,
173-
writePreambleStart,
174174
writePreambleEnd,
175175
writeHoistables,
176176
writePostamble,
@@ -311,5 +311,19 @@ export function writeEndClientRenderedSuspenseBoundary(
311311
return writeEndClientRenderedSuspenseBoundaryImpl(destination, renderState);
312312
}
313313

314+
export function writePreambleStart(
315+
destination: Destination,
316+
resumableState: ResumableState,
317+
renderState: RenderState,
318+
skipBlockingShell: boolean,
319+
): void {
320+
return writePreambleStartImpl(
321+
destination,
322+
resumableState,
323+
renderState,
324+
true, // skipBlockingShell
325+
);
326+
}
327+
314328
export type TransitionStatus = FormStatus;
315329
export const NotPendingTransition: TransitionStatus = NotPending;

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

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@ global.AsyncLocalStorage = require('async_hooks').AsyncLocalStorage;
1818
let React;
1919
let ReactDOM;
2020
let ReactDOMFizzServer;
21+
let Suspense;
2122

2223
describe('ReactDOMFizzServerEdge', () => {
2324
beforeEach(() => {
2425
jest.resetModules();
2526
jest.useRealTimers();
2627
React = require('react');
28+
Suspense = React.Suspense;
2729
ReactDOM = require('react-dom');
2830
ReactDOMFizzServer = require('react-dom/server.edge');
2931
});
@@ -81,4 +83,102 @@ describe('ReactDOMFizzServerEdge', () => {
8183
);
8284
}
8385
});
86+
87+
it('recoverably errors and does not add rel="expect" for large shells', async () => {
88+
function Paragraph() {
89+
return (
90+
<p>
91+
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Mauris
92+
porttitor tortor ac lectus faucibus, eget eleifend elit hendrerit.
93+
Integer porttitor nisi in leo congue rutrum. Morbi sed ante posuere,
94+
aliquam lorem ac, imperdiet orci. Duis malesuada gravida pharetra.
95+
Cras facilisis arcu diam, id dictum lorem imperdiet a. Suspendisse
96+
aliquet tempus tortor et ultricies. Aliquam libero velit, posuere
97+
tempus ante sed, pellentesque tincidunt lorem. Nullam iaculis, eros a
98+
varius aliquet, tortor felis tempor metus, nec cursus felis eros
99+
aliquam nulla. Vivamus ut orci sed mauris congue lacinia. Cras eget
100+
blandit neque. Pellentesque a massa in turpis ullamcorper volutpat vel
101+
at massa. Sed ante est, auctor non diam non, vulputate ultrices metus.
102+
Maecenas dictum fermentum quam id aliquam. Donec porta risus vitae
103+
pretium posuere. Fusce facilisis eros in lacus tincidunt congue.
104+
</p>
105+
);
106+
}
107+
108+
function App({suspense}) {
109+
const paragraphs = [];
110+
for (let i = 0; i < 600; i++) {
111+
paragraphs.push(<Paragraph key={i} />);
112+
}
113+
return (
114+
<html>
115+
<body>
116+
{suspense ? (
117+
// This is ok
118+
<Suspense fallback="Loading">{paragraphs}</Suspense>
119+
) : (
120+
// This is not
121+
paragraphs
122+
)}
123+
</body>
124+
</html>
125+
);
126+
}
127+
const errors = [];
128+
const stream = await ReactDOMFizzServer.renderToReadableStream(
129+
<App suspense={false} />,
130+
{
131+
onError(error) {
132+
errors.push(error);
133+
},
134+
},
135+
);
136+
const result = await readResult(stream);
137+
expect(result).not.toContain('rel="expect"');
138+
if (gate(flags => flags.enableFizzBlockingRender)) {
139+
expect(errors.length).toBe(1);
140+
expect(errors[0].message).toContain(
141+
'This rendered a large document (>512) without any Suspense boundaries around most of it.',
142+
);
143+
} else {
144+
expect(errors.length).toBe(0);
145+
}
146+
147+
// If we wrap in a Suspense boundary though, then it should be ok.
148+
const errors2 = [];
149+
const stream2 = await ReactDOMFizzServer.renderToReadableStream(
150+
<App suspense={true} />,
151+
{
152+
onError(error) {
153+
errors2.push(error);
154+
},
155+
},
156+
);
157+
const result2 = await readResult(stream2);
158+
if (gate(flags => flags.enableFizzBlockingRender)) {
159+
expect(result2).toContain('rel="expect"');
160+
} else {
161+
expect(result2).not.toContain('rel="expect"');
162+
}
163+
expect(errors2.length).toBe(0);
164+
165+
// Or if we increase the progressiveChunkSize.
166+
const errors3 = [];
167+
const stream3 = await ReactDOMFizzServer.renderToReadableStream(
168+
<App suspense={false} />,
169+
{
170+
progressiveChunkSize: 100000,
171+
onError(error) {
172+
errors3.push(error);
173+
},
174+
},
175+
);
176+
const result3 = await readResult(stream3);
177+
if (gate(flags => flags.enableFizzBlockingRender)) {
178+
expect(result3).toContain('rel="expect"');
179+
} else {
180+
expect(result3).not.toContain('rel="expect"');
181+
}
182+
expect(errors3.length).toBe(0);
183+
});
84184
});

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,7 @@ describe('ReactDOMFloat', () => {
3535

3636
expect(result).toEqual(
3737
'<html><head><meta charSet="utf-8"/>' +
38-
(gate(flags => flags.enableFizzBlockingRender)
39-
? '<link rel="expect" href="#_R_" blocking="render"/>'
40-
: '') +
4138
'<title>title</title><script src="foo"></script></head>' +
42-
(gate(flags => flags.enableFizzBlockingRender)
43-
? '<template id="_R_"></template>'
44-
: '') +
4539
'</html>',
4640
);
4741
});

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

Lines changed: 10 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -70,29 +70,20 @@ describe('rendering React components at document', () => {
7070

7171
const markup = ReactDOMServer.renderToString(<Root hello="world" />);
7272
expect(markup).not.toContain('DOCTYPE');
73+
expect(markup).not.toContain('rel="expect"');
7374
const testDocument = getTestDocument(markup);
7475
const body = testDocument.body;
7576

7677
let root;
7778
await act(() => {
7879
root = ReactDOMClient.hydrateRoot(testDocument, <Root hello="world" />);
7980
});
80-
expect(testDocument.body.innerHTML).toBe(
81-
'Hello world' +
82-
(gate(flags => flags.enableFizzBlockingRender)
83-
? '<template id="_R_"></template>'
84-
: ''),
85-
);
81+
expect(testDocument.body.innerHTML).toBe('Hello world');
8682

8783
await act(() => {
8884
root.render(<Root hello="moon" />);
8985
});
90-
expect(testDocument.body.innerHTML).toBe(
91-
'Hello moon' +
92-
(gate(flags => flags.enableFizzBlockingRender)
93-
? '<template id="_R_"></template>'
94-
: ''),
95-
);
86+
expect(testDocument.body.innerHTML).toBe('Hello moon');
9687

9788
expect(body === testDocument.body).toBe(true);
9889
});
@@ -117,12 +108,7 @@ describe('rendering React components at document', () => {
117108
await act(() => {
118109
root = ReactDOMClient.hydrateRoot(testDocument, <Root />);
119110
});
120-
expect(testDocument.body.innerHTML).toBe(
121-
'Hello world' +
122-
(gate(flags => flags.enableFizzBlockingRender)
123-
? '<template id="_R_"></template>'
124-
: ''),
125-
);
111+
expect(testDocument.body.innerHTML).toBe('Hello world');
126112

127113
const originalDocEl = testDocument.documentElement;
128114
const originalHead = testDocument.head;
@@ -133,16 +119,8 @@ describe('rendering React components at document', () => {
133119
expect(testDocument.firstChild).toBe(originalDocEl);
134120
expect(testDocument.head).toBe(originalHead);
135121
expect(testDocument.body).toBe(originalBody);
136-
expect(originalBody.innerHTML).toBe(
137-
gate(flags => flags.enableFizzBlockingRender)
138-
? '<template id="_R_"></template>'
139-
: '',
140-
);
141-
expect(originalHead.innerHTML).toBe(
142-
gate(flags => flags.enableFizzBlockingRender)
143-
? '<link rel="expect" href="#_R_" blocking="render">'
144-
: '',
145-
);
122+
expect(originalBody.innerHTML).toBe('');
123+
expect(originalHead.innerHTML).toBe('');
146124
});
147125

148126
it('should not be able to switch root constructors', async () => {
@@ -180,22 +158,13 @@ describe('rendering React components at document', () => {
180158
root = ReactDOMClient.hydrateRoot(testDocument, <Component />);
181159
});
182160

183-
expect(testDocument.body.innerHTML).toBe(
184-
'Hello world' +
185-
(gate(flags => flags.enableFizzBlockingRender)
186-
? '<template id="_R_"></template>'
187-
: ''),
188-
);
161+
expect(testDocument.body.innerHTML).toBe('Hello world');
189162

190163
await act(() => {
191164
root.render(<Component2 />);
192165
});
193166

194-
expect(testDocument.body.innerHTML).toBe(
195-
(gate(flags => flags.enableFizzBlockingRender)
196-
? '<template id="_R_"></template>'
197-
: '') + 'Goodbye world',
198-
);
167+
expect(testDocument.body.innerHTML).toBe('Goodbye world');
199168
});
200169

201170
it('should be able to mount into document', async () => {
@@ -224,12 +193,7 @@ describe('rendering React components at document', () => {
224193
);
225194
});
226195

227-
expect(testDocument.body.innerHTML).toBe(
228-
'Hello world' +
229-
(gate(flags => flags.enableFizzBlockingRender)
230-
? '<template id="_R_"></template>'
231-
: ''),
232-
);
196+
expect(testDocument.body.innerHTML).toBe('Hello world');
233197
});
234198

235199
it('cannot render over an existing text child at the root', async () => {
@@ -362,12 +326,7 @@ describe('rendering React components at document', () => {
362326
: [],
363327
);
364328
expect(testDocument.body.innerHTML).toBe(
365-
favorSafetyOverHydrationPerf
366-
? 'Hello world'
367-
: 'Goodbye world' +
368-
(gate(flags => flags.enableFizzBlockingRender)
369-
? '<template id="_R_"></template>'
370-
: ''),
329+
favorSafetyOverHydrationPerf ? 'Hello world' : 'Goodbye world',
371330
);
372331
});
373332

packages/react-markup/src/ReactFizzConfigMarkup.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,13 +222,13 @@ export function writePreambleStart(
222222
destination: Destination,
223223
resumableState: ResumableState,
224224
renderState: RenderState,
225-
skipExpect?: boolean, // Used as an override by ReactFizzConfigMarkup
225+
skipBlockingShell: boolean,
226226
): void {
227227
return writePreambleStartImpl(
228228
destination,
229229
resumableState,
230230
renderState,
231-
true, // skipExpect
231+
true, // skipBlockingShell
232232
);
233233
}
234234

0 commit comments

Comments
 (0)