Skip to content

Commit

Permalink
A few fixes
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@gnoff
gnoff committed Jan 10, 2023
1 parent e640403 commit 74de63c
Show file tree
Hide file tree
Showing 3 changed files with 286 additions and 20 deletions.
38 changes: 20 additions & 18 deletions packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2442,27 +2442,29 @@ export function writeEarlyPreamble(
// If we emitted a preamble early it will have flushed <html> and <head>.
// We check that we haven't flushed anything yet which is equivalent
// to checking whether we have not flushed an <html> or <head>
if (responseState.flushed === NONE && responseState.rendered !== NONE) {
let i = 0;
const {htmlChunks, headChunks} = responseState;
if (htmlChunks.length) {
for (i = 0; i < htmlChunks.length; i++) {
writeChunk(destination, htmlChunks[i]);
if (responseState.rendered !== NONE) {
if (responseState.flushed === NONE) {
let i = 0;
const {htmlChunks, headChunks} = responseState;
if (htmlChunks.length) {
for (i = 0; i < htmlChunks.length; i++) {
writeChunk(destination, htmlChunks[i]);
}
} else {
writeChunk(destination, DOCTYPE);
writeChunk(destination, startChunkForTag('html'));
writeChunk(destination, endOfStartTag);
}
} else {
writeChunk(destination, DOCTYPE);
writeChunk(destination, startChunkForTag('html'));
writeChunk(destination, endOfStartTag);
}
if (headChunks.length) {
for (i = 0; i < headChunks.length; i++) {
writeChunk(destination, headChunks[i]);
if (headChunks.length) {
for (i = 0; i < headChunks.length; i++) {
writeChunk(destination, headChunks[i]);
}
} else {
writeChunk(destination, startChunkForTag('head'));
writeChunk(destination, endOfStartTag);
}
} else {
writeChunk(destination, startChunkForTag('head'));
writeChunk(destination, endOfStartTag);
responseState.flushed |= HTML | HEAD;
}
responseState.flushed |= HTML | HEAD;

return writeEarlyResources(
destination,
Expand Down
258 changes: 258 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@ describe('ReactDOMFizzServer', () => {
while (true) {
const bufferedContent = buffer;

document.__headOpen =
document.__headOpen ||
((bufferedContent.includes('<head>') ||
bufferedContent.includes('<head ')) &&
!bufferedContent.includes('</head>'));

let parent;
let temporaryHostElement;
if (bufferedContent.startsWith('<!DOCTYPE html>')) {
Expand Down Expand Up @@ -255,6 +261,12 @@ describe('ReactDOMFizzServer', () => {
temporaryHostElement = document.createElement('head');
temporaryHostElement.innerHTML = headContent;
buffer = bodyContent;
document.__headOpen = false;
} else if (document.__headOpen) {
parent = document.head;
temporaryHostElement = document.createElement('head');
temporaryHostElement.innerHTML = bufferedContent;
buffer = '';
} else {
parent = document.body;
temporaryHostElement = document.createElement('body');
Expand Down Expand Up @@ -6368,6 +6380,55 @@ describe('ReactDOMFizzServer', () => {
);
});

it('can render an empty fallback', async () => {
function Throw() {
throw new Error('uh oh');
}

let content = '';
writable.on('data', chunk => (content += chunk));

let didBootstrap = false;
function bootstrap() {
didBootstrap = true;
}
window.__INIT__ = bootstrap;

const errors = [];
await act(() => {
const {pipe} = renderIntoDocumentAsPipeableStream(
<html id="html">
<link rel="stylesheet" href="foo" precedence="foo" />
<link rel="stylesheet" href="bar" precedence="bar" />
<body id="body">
<Throw />
</body>
</html>,
undefined,
{
onError(err) {
errors.push(err.message);
},
fallbackBootstrapScriptContent: '__INIT__()',
},
);
pipe(writable);
});

expect(errors).toEqual(['uh oh']);
expect(didBootstrap).toBe(true);

expect(getMeaningfulChildren(document)).toEqual(
<html>
<head>
<link rel="stylesheet" href="foo" data-precedence="foo" />
<link rel="stylesheet" href="bar" data-precedence="bar" />
</head>
<body />
</html>,
);
});

it('emits fallback bootstrap scripts if configured when rendering the fallback shell', async () => {
function Throw() {
throw new Error('uh oh');
Expand Down Expand Up @@ -6461,5 +6522,202 @@ describe('ReactDOMFizzServer', () => {

expect(didBootstrap).toBe(true);
});

it('does not work on the fallback unless the primary children error in the shell', async () => {
function Throw() {
throw new Error('uh oh');
}

const logs = [];
function BlockOn({value, children}) {
readText(value);
logs.push(value);
return children;
}

const errors = [];
await act(() => {
const {pipe} = renderIntoDocumentAsPipeableStream(
<html id="html">
<body id="body">
<BlockOn value="error">
<Throw />
</BlockOn>
<BlockOn value="resource">
<link rel="stylesheet" href="foo" precedence="foo" />
</BlockOn>
hello world
</body>
</html>,
<div>
<BlockOn value="fallback">fallback</BlockOn>
</div>,
{
onError(err) {
errors.push(err.message);
},
},
);
pipe(writable);
});

expect(logs).toEqual([]);
logs.length = 0;
expect(getMeaningfulChildren(document)).toEqual(
<html id="html">
<head />
<body />
</html>,
);

// Even though we unblock fallback since the task is not scheduled no log is observed
await act(() => {
resolveText('fallback');
});
expect(logs).toEqual([]);
logs.length = 0;
expect(getMeaningfulChildren(document)).toEqual(
<html id="html">
<head />
<body />
</html>,
);

// When we resolve the resource it is emitted in the open preamble.
await act(() => {
resolveText('resource');
});
expect(logs).toEqual(['resource']);
logs.length = 0;
expect(getMeaningfulChildren(document)).toEqual(
<html id="html">
<head>
<link rel="stylesheet" href="foo" data-precedence="foo" />
</head>
<body />
</html>,
);

// When we resolve the resource it is emitted in the open preamble.
await act(() => {
resolveText('error');
});
expect(logs).toEqual(['error', 'fallback']);
logs.length = 0;
expect(getMeaningfulChildren(document)).toEqual(
<html id="html">
<head>
<link rel="stylesheet" href="foo" data-precedence="foo" />
</head>
<body>
<div>fallback</div>
</body>
</html>,
);
});

it('only emits stylesheets up to the first precedence during the early preamble', async () => {
function BlockOn({value, children}) {
readText(value);
return children;
}

await act(() => {
const {pipe} = renderIntoDocumentAsPipeableStream(
<html id="html">
<head />
<body id="body">
<BlockOn value="two">
<link rel="stylesheet" href="baz1" precedence="baz" />
<link rel="stylesheet" href="foo2" precedence="foo" />
<script async={true} src="script2" />
</BlockOn>
<BlockOn value="one">
<link rel="stylesheet" href="foo1" precedence="foo" />
<link rel="stylesheet" href="bar1" precedence="bar" />
<script async={true} src="script1" />
</BlockOn>
<BlockOn value="three">
<link rel="stylesheet" href="baz2" precedence="baz" />
<link rel="stylesheet" href="bar2" precedence="bar" />
<link rel="stylesheet" href="foo3" precedence="foo" />
<script async={true} src="script3" />
</BlockOn>
hello world
</body>
</html>,
);
pipe(writable);
});

expect(getMeaningfulChildren(document)).toEqual(
<html id="html">
<head />
<body />
</html>,
);

// We emit all resources that are unblocked which is most types except stylesheets.
// For stylesheets we can only emit up to the first precedence since we may discover
// additional stylesheets with this precedence level after this flush and would violate
// stylesheet order. For stylesheets we cannot emit we can emit a preload instead so the
// browser can start downloading the resource as soon as possible
await act(() => {
resolveText('one');
});
expect(getMeaningfulChildren(document)).toEqual(
<html id="html">
<head>
<link rel="stylesheet" href="foo1" data-precedence="foo" />
<link rel="preload" href="bar1" as="style" />
<script async="" src="script1" />
</head>
<body />
</html>,
);

// We can continue to emit early resources according to these rules
await act(() => {
resolveText('two');
});
expect(getMeaningfulChildren(document)).toEqual(
<html id="html">
<head>
<link rel="stylesheet" href="foo1" data-precedence="foo" />
<link rel="preload" href="bar1" as="style" />
<script async="" src="script1" />
<link rel="stylesheet" href="foo2" data-precedence="foo" />
<link rel="preload" href="baz1" as="style" />
<script async="" src="script2" />
</head>
<body />
</html>,
);

// Once the Shell is unblocked we can now emit the entire preamble as well as
// the main content
await act(() => {
resolveText('three');
});
expect(getMeaningfulChildren(document)).toEqual(
<html id="html">
<head>
<link rel="stylesheet" href="foo1" data-precedence="foo" />
<link rel="preload" href="bar1" as="style" />
<script async="" src="script1" />
<link rel="stylesheet" href="foo2" data-precedence="foo" />
<link rel="preload" href="baz1" as="style" />
<script async="" src="script2" />
<link rel="stylesheet" href="foo3" data-precedence="foo" />
<link rel="stylesheet" href="bar1" data-precedence="bar" />
<link rel="stylesheet" href="bar2" data-precedence="bar" />
<link rel="stylesheet" href="baz1" data-precedence="baz" />
<link rel="stylesheet" href="baz2" data-precedence="baz" />
<script async="" src="script3" />
</head>
<body id="body">hello world</body>
</html>,
);
});
});
});
10 changes: 8 additions & 2 deletions packages/react-server/src/ReactFizzServer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,10 @@ export function createRequest(
);
pingedTasks.push(rootTask);

if (fallback) {
// null is a valid fallback so we distinguish between undefined and null here
// If you use renderIntoDocument without a fallback argument the Request still
// has a null fallback and will exhibit fallback behavior
if (fallback !== undefined) {
const fallbackRootSegment = createPendingSegment(
request,
0,
Expand Down Expand Up @@ -1845,7 +1848,10 @@ function finishedTask(
abortTaskSoft.call(request, fallbackTask);
}

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

0 comments on commit 74de63c

Please sign in to comment.