Skip to content

Commit 6309c30

Browse files
gnoffgnoff
committed
A few fixes
1. Only enqueue rootSegment if not aborted. Previously if the fallbackTask aborted it would enqueue the aborted segment as the completedRootSegment causing the stream to error 2. Always emit early resources. Previously early resources were only emitted if you were writing the preamble open in the same flush. It should have been that the early resources are emitted if the preamble open has already been sent in this flush or a prior one
1 parent 34c6c03 commit 6309c30

File tree

3 files changed

+286
-20
lines changed

3 files changed

+286
-20
lines changed

‎packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2434,27 +2434,29 @@ export function writeEarlyPreamble(
24342434
// If we emitted a preamble early it will have flushed <html> and <head>.
24352435
// We check that we haven't flushed anything yet which is equivalent
24362436
// to checking whether we have not flushed an <html> or <head>
2437-
if (responseState.flushed === NONE && responseState.rendered !== NONE) {
2438-
let i = 0;
2439-
const {htmlChunks, headChunks} = responseState;
2440-
if (htmlChunks.length) {
2441-
for (i = 0; i < htmlChunks.length; i++) {
2442-
writeChunk(destination, htmlChunks[i]);
2437+
if (responseState.rendered !== NONE) {
2438+
if (responseState.flushed === NONE) {
2439+
let i = 0;
2440+
const {htmlChunks, headChunks} = responseState;
2441+
if (htmlChunks.length) {
2442+
for (i = 0; i < htmlChunks.length; i++) {
2443+
writeChunk(destination, htmlChunks[i]);
2444+
}
2445+
} else {
2446+
writeChunk(destination, DOCTYPE);
2447+
writeChunk(destination, startChunkForTag('html'));
2448+
writeChunk(destination, endOfStartTag);
24432449
}
2444-
} else {
2445-
writeChunk(destination, DOCTYPE);
2446-
writeChunk(destination, startChunkForTag('html'));
2447-
writeChunk(destination, endOfStartTag);
2448-
}
2449-
if (headChunks.length) {
2450-
for (i = 0; i < headChunks.length; i++) {
2451-
writeChunk(destination, headChunks[i]);
2450+
if (headChunks.length) {
2451+
for (i = 0; i < headChunks.length; i++) {
2452+
writeChunk(destination, headChunks[i]);
2453+
}
2454+
} else {
2455+
writeChunk(destination, startChunkForTag('head'));
2456+
writeChunk(destination, endOfStartTag);
24522457
}
2453-
} else {
2454-
writeChunk(destination, startChunkForTag('head'));
2455-
writeChunk(destination, endOfStartTag);
2458+
responseState.flushed |= HTML | HEAD;
24562459
}
2457-
responseState.flushed |= HTML | HEAD;
24582460

24592461
return writeEarlyResources(
24602462
destination,

‎packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,12 @@ describe('ReactDOMFizzServer', () => {
212212
while (true) {
213213
const bufferedContent = buffer;
214214

215+
document.__headOpen =
216+
document.__headOpen ||
217+
((bufferedContent.includes('<head>') ||
218+
bufferedContent.includes('<head ')) &&
219+
!bufferedContent.includes('</head>'));
220+
215221
let parent;
216222
let temporaryHostElement;
217223
if (bufferedContent.startsWith('<!DOCTYPE html>')) {
@@ -255,6 +261,12 @@ describe('ReactDOMFizzServer', () => {
255261
temporaryHostElement = document.createElement('head');
256262
temporaryHostElement.innerHTML = headContent;
257263
buffer = bodyContent;
264+
document.__headOpen = false;
265+
} else if (document.__headOpen) {
266+
parent = document.head;
267+
temporaryHostElement = document.createElement('head');
268+
temporaryHostElement.innerHTML = bufferedContent;
269+
buffer = '';
258270
} else {
259271
parent = document.body;
260272
temporaryHostElement = document.createElement('body');
@@ -6368,6 +6380,55 @@ describe('ReactDOMFizzServer', () => {
63686380
);
63696381
});
63706382

6383+
it('can render an empty fallback', async () => {
6384+
function Throw() {
6385+
throw new Error('uh oh');
6386+
}
6387+
6388+
let content = '';
6389+
writable.on('data', chunk => (content += chunk));
6390+
6391+
let didBootstrap = false;
6392+
function bootstrap() {
6393+
didBootstrap = true;
6394+
}
6395+
window.__INIT__ = bootstrap;
6396+
6397+
const errors = [];
6398+
await act(() => {
6399+
const {pipe} = renderIntoDocumentAsPipeableStream(
6400+
<html id="html">
6401+
<link rel="stylesheet" href="foo" precedence="foo" />
6402+
<link rel="stylesheet" href="bar" precedence="bar" />
6403+
<body id="body">
6404+
<Throw />
6405+
</body>
6406+
</html>,
6407+
undefined,
6408+
{
6409+
onError(err) {
6410+
errors.push(err.message);
6411+
},
6412+
fallbackBootstrapScriptContent: '__INIT__()',
6413+
},
6414+
);
6415+
pipe(writable);
6416+
});
6417+
6418+
expect(errors).toEqual(['uh oh']);
6419+
expect(didBootstrap).toBe(true);
6420+
6421+
expect(getMeaningfulChildren(document)).toEqual(
6422+
<html>
6423+
<head>
6424+
<link rel="stylesheet" href="foo" data-precedence="foo" />
6425+
<link rel="stylesheet" href="bar" data-precedence="bar" />
6426+
</head>
6427+
<body />
6428+
</html>,
6429+
);
6430+
});
6431+
63716432
it('emits fallback bootstrap scripts if configured when rendering the fallback shell', async () => {
63726433
function Throw() {
63736434
throw new Error('uh oh');
@@ -6461,5 +6522,202 @@ describe('ReactDOMFizzServer', () => {
64616522

64626523
expect(didBootstrap).toBe(true);
64636524
});
6525+
6526+
it('does not work on the fallback unless the primary children error in the shell', async () => {
6527+
function Throw() {
6528+
throw new Error('uh oh');
6529+
}
6530+
6531+
const logs = [];
6532+
function BlockOn({value, children}) {
6533+
readText(value);
6534+
logs.push(value);
6535+
return children;
6536+
}
6537+
6538+
const errors = [];
6539+
await act(() => {
6540+
const {pipe} = renderIntoDocumentAsPipeableStream(
6541+
<html id="html">
6542+
<body id="body">
6543+
<BlockOn value="error">
6544+
<Throw />
6545+
</BlockOn>
6546+
<BlockOn value="resource">
6547+
<link rel="stylesheet" href="foo" precedence="foo" />
6548+
</BlockOn>
6549+
hello world
6550+
</body>
6551+
</html>,
6552+
<div>
6553+
<BlockOn value="fallback">fallback</BlockOn>
6554+
</div>,
6555+
{
6556+
onError(err) {
6557+
errors.push(err.message);
6558+
},
6559+
},
6560+
);
6561+
pipe(writable);
6562+
});
6563+
6564+
expect(logs).toEqual([]);
6565+
logs.length = 0;
6566+
expect(getMeaningfulChildren(document)).toEqual(
6567+
<html id="html">
6568+
<head />
6569+
<body />
6570+
</html>,
6571+
);
6572+
6573+
// Even though we unblock fallback since the task is not scheduled no log is observed
6574+
await act(() => {
6575+
resolveText('fallback');
6576+
});
6577+
expect(logs).toEqual([]);
6578+
logs.length = 0;
6579+
expect(getMeaningfulChildren(document)).toEqual(
6580+
<html id="html">
6581+
<head />
6582+
<body />
6583+
</html>,
6584+
);
6585+
6586+
// When we resolve the resource it is emitted in the open preamble.
6587+
await act(() => {
6588+
resolveText('resource');
6589+
});
6590+
expect(logs).toEqual(['resource']);
6591+
logs.length = 0;
6592+
expect(getMeaningfulChildren(document)).toEqual(
6593+
<html id="html">
6594+
<head>
6595+
<link rel="stylesheet" href="foo" data-precedence="foo" />
6596+
</head>
6597+
<body />
6598+
</html>,
6599+
);
6600+
6601+
// When we resolve the resource it is emitted in the open preamble.
6602+
await act(() => {
6603+
resolveText('error');
6604+
});
6605+
expect(logs).toEqual(['error', 'fallback']);
6606+
logs.length = 0;
6607+
expect(getMeaningfulChildren(document)).toEqual(
6608+
<html id="html">
6609+
<head>
6610+
<link rel="stylesheet" href="foo" data-precedence="foo" />
6611+
</head>
6612+
<body>
6613+
<div>fallback</div>
6614+
</body>
6615+
</html>,
6616+
);
6617+
});
6618+
6619+
it('only emits stylesheets up to the first precedence during the early preamble', async () => {
6620+
function BlockOn({value, children}) {
6621+
readText(value);
6622+
return children;
6623+
}
6624+
6625+
await act(() => {
6626+
const {pipe} = renderIntoDocumentAsPipeableStream(
6627+
<html id="html">
6628+
<head />
6629+
<body id="body">
6630+
<BlockOn value="two">
6631+
<link rel="stylesheet" href="baz1" precedence="baz" />
6632+
<link rel="stylesheet" href="foo2" precedence="foo" />
6633+
<script async={true} src="script2" />
6634+
</BlockOn>
6635+
<BlockOn value="one">
6636+
<link rel="stylesheet" href="foo1" precedence="foo" />
6637+
<link rel="stylesheet" href="bar1" precedence="bar" />
6638+
<script async={true} src="script1" />
6639+
</BlockOn>
6640+
<BlockOn value="three">
6641+
<link rel="stylesheet" href="baz2" precedence="baz" />
6642+
<link rel="stylesheet" href="bar2" precedence="bar" />
6643+
<link rel="stylesheet" href="foo3" precedence="foo" />
6644+
<script async={true} src="script3" />
6645+
</BlockOn>
6646+
hello world
6647+
</body>
6648+
</html>,
6649+
);
6650+
pipe(writable);
6651+
});
6652+
6653+
expect(getMeaningfulChildren(document)).toEqual(
6654+
<html id="html">
6655+
<head />
6656+
<body />
6657+
</html>,
6658+
);
6659+
6660+
// We emit all resources that are unblocked which is most types except stylesheets.
6661+
// For stylesheets we can only emit up to the first precedence since we may discover
6662+
// additional stylesheets with this precedence level after this flush and would violate
6663+
// stylesheet order. For stylesheets we cannot emit we can emit a preload instead so the
6664+
// browser can start downloading the resource as soon as possible
6665+
await act(() => {
6666+
resolveText('one');
6667+
});
6668+
expect(getMeaningfulChildren(document)).toEqual(
6669+
<html id="html">
6670+
<head>
6671+
<link rel="stylesheet" href="foo1" data-precedence="foo" />
6672+
<link rel="preload" href="bar1" as="style" />
6673+
<script async="" src="script1" />
6674+
</head>
6675+
<body />
6676+
</html>,
6677+
);
6678+
6679+
// We can continue to emit early resources according to these rules
6680+
await act(() => {
6681+
resolveText('two');
6682+
});
6683+
expect(getMeaningfulChildren(document)).toEqual(
6684+
<html id="html">
6685+
<head>
6686+
<link rel="stylesheet" href="foo1" data-precedence="foo" />
6687+
<link rel="preload" href="bar1" as="style" />
6688+
<script async="" src="script1" />
6689+
<link rel="stylesheet" href="foo2" data-precedence="foo" />
6690+
<link rel="preload" href="baz1" as="style" />
6691+
<script async="" src="script2" />
6692+
</head>
6693+
<body />
6694+
</html>,
6695+
);
6696+
6697+
// Once the Shell is unblocked we can now emit the entire preamble as well as
6698+
// the main content
6699+
await act(() => {
6700+
resolveText('three');
6701+
});
6702+
expect(getMeaningfulChildren(document)).toEqual(
6703+
<html id="html">
6704+
<head>
6705+
<link rel="stylesheet" href="foo1" data-precedence="foo" />
6706+
<link rel="preload" href="bar1" as="style" />
6707+
<script async="" src="script1" />
6708+
<link rel="stylesheet" href="foo2" data-precedence="foo" />
6709+
<link rel="preload" href="baz1" as="style" />
6710+
<script async="" src="script2" />
6711+
<link rel="stylesheet" href="foo3" data-precedence="foo" />
6712+
<link rel="stylesheet" href="bar1" data-precedence="bar" />
6713+
<link rel="stylesheet" href="bar2" data-precedence="bar" />
6714+
<link rel="stylesheet" href="baz1" data-precedence="baz" />
6715+
<link rel="stylesheet" href="baz2" data-precedence="baz" />
6716+
<script async="" src="script3" />
6717+
</head>
6718+
<body id="body">hello world</body>
6719+
</html>,
6720+
);
6721+
});
64646722
});
64656723
});

‎packages/react-server/src/ReactFizzServer.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,10 @@ export function createRequest(
345345
);
346346
pingedTasks.push(rootTask);
347347

348-
if (fallback) {
348+
// null is a valid fallback so we distinguish between undefined and null here
349+
// If you use renderIntoDocument without a fallback argument the Request still
350+
// has a null fallback and will exhibit fallback behavior
351+
if (fallback !== undefined) {
349352
const fallbackRootSegment = createPendingSegment(
350353
request,
351354
0,
@@ -1840,7 +1843,10 @@ function finishedTask(
18401843
abortTaskSoft.call(request, fallbackTask);
18411844
}
18421845

1843-
if (segment.parentFlushed) {
1846+
// When the fallbackTask is aborted it still gets finished. We need to ensure we only
1847+
// set the completedRootSegment if the segment is not aborted to avoid having more than
1848+
// one set at a time.
1849+
if (segment.parentFlushed && segment.status !== ABORTED) {
18441850
if (request.completedRootSegment !== null) {
18451851
throw new Error(
18461852
'There can only be one root segment. This is a bug in React.',

0 commit comments

Comments
 (0)