Skip to content

Commit

Permalink
[Fizz] Don't add aborted segments to the completedSegments list (#21976)
Browse files Browse the repository at this point in the history
* Don't add aborted segments to the completedSegments list

* Update error message to include aborted status
  • Loading branch information
sebmarkbage authored Jul 28, 2021
1 parent cdccdbe commit 321087d
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 11 deletions.
31 changes: 31 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1447,4 +1447,35 @@ describe('ReactDOMFizzServer', () => {

expect(loggedErrors).toEqual([theError]);
});

// @gate experimental
it('should be able to abort the fallback if the main content finishes first', async () => {
await act(async () => {
const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
<Suspense fallback={<Text text="Loading Outer" />}>
<div>
<Suspense
fallback={
<div>
<AsyncText text="Loading" />
Inner
</div>
}>
<AsyncText text="Hello" />
</Suspense>
</div>
</Suspense>,
writable,
);
startWriting();
});
expect(getVisibleChildren(container)).toEqual('Loading Outer');
// We should have received a partial segment containing the a partial of the fallback.
expect(container.innerHTML).toContain('Inner');
await act(async () => {
resolveText('Hello');
});
// We should've been able to display the content without waiting for the rest of the fallback.
expect(getVisibleChildren(container)).toEqual(<div>Hello</div>);
});
});
28 changes: 18 additions & 10 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1408,7 +1408,11 @@ function finishedTask(
// This must have been the last segment we were waiting on. This boundary is now complete.
if (segment.parentFlushed) {
// Our parent segment already flushed, so we need to schedule this segment to be emitted.
boundary.completedSegments.push(segment);
// If it is a segment that was aborted, we'll write other content instead so we don't need
// to emit it.
if (segment.status === COMPLETED) {
boundary.completedSegments.push(segment);
}
}
if (boundary.parentFlushed) {
// The segment might be part of a segment that didn't flush yet, but if the boundary's
Expand All @@ -1423,14 +1427,18 @@ function finishedTask(
} else {
if (segment.parentFlushed) {
// Our parent already flushed, so we need to schedule this segment to be emitted.
const completedSegments = boundary.completedSegments;
completedSegments.push(segment);
if (completedSegments.length === 1) {
// This is the first time since we last flushed that we completed anything.
// We can schedule this boundary to emit its partially completed segments early
// in case the parent has already been flushed.
if (boundary.parentFlushed) {
request.partialBoundaries.push(boundary);
// If it is a segment that was aborted, we'll write other content instead so we don't need
// to emit it.
if (segment.status === COMPLETED) {
const completedSegments = boundary.completedSegments;
completedSegments.push(segment);
if (completedSegments.length === 1) {
// This is the first time since we last flushed that we completed anything.
// We can schedule this boundary to emit its partially completed segments early
// in case the parent has already been flushed.
if (boundary.parentFlushed) {
request.partialBoundaries.push(boundary);
}
}
}
}
Expand Down Expand Up @@ -1570,7 +1578,7 @@ function flushSubtree(
default: {
invariant(
false,
'Errored or already flushed boundaries should not be flushed again. This is a bug in React.',
'Aborted, errored or already flushed boundaries should not be flushed again. This is a bug in React.',
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion scripts/error-codes/codes.json
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@
"387": "Should have a current fiber. This is a bug in React.",
"388": "Expected to find a bailed out fiber. This is a bug in React.",
"389": "There can only be one root segment. This is a bug in React.",
"390": "Errored or already flushed boundaries should not be flushed again. This is a bug in React.",
"390": "Aborted, errored or already flushed boundaries should not be flushed again. This is a bug in React.",
"391": "A previously unvisited boundary must have exactly one root segment. This is a bug in React.",
"392": "A root segment ID must have been assigned by now. This is a bug in React.",
"393": "Cache cannot be refreshed during server rendering.",
Expand Down

0 comments on commit 321087d

Please sign in to comment.