Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Fizz] Don't flush empty segments #24054

Merged
merged 1 commit into from
Mar 9, 2022
Merged

[Fizz] Don't flush empty segments #24054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,10 @@ describe('ReactDOMFizzServer', () => {
pipe(writable);
});
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);
// Because there is no content inside the Suspense boundary that could've
// been written, we expect to not see any additional partial data flushed
// yet.
expect(container.firstChild.nextSibling).toBe(null);
await act(async () => {
resolveElement({default: <Text text="Hello" />});
});
Expand Down
29 changes: 26 additions & 3 deletions packages/react-server/src/ReactFizzServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ function renderSuspenseBoundary(
// We use the safe form because we don't handle suspending here. Only error handling.
renderNode(request, task, content);
contentRootSegment.status = COMPLETED;
newBoundary.completedSegments.push(contentRootSegment);
queueCompletedSegment(newBoundary, contentRootSegment);
if (newBoundary.pendingTasks === 0) {
// This must have been the last segment we were waiting on. This boundary is now complete.
// Therefore we won't need the fallback. We early return so that we don't have to create
Expand Down Expand Up @@ -1430,6 +1430,29 @@ function abortTask(task: Task): void {
}
}

function queueCompletedSegment(
boundary: SuspenseBoundary,
segment: Segment,
): void {
if (
segment.chunks.length === 0 &&
segment.children.length === 1 &&
segment.children[0].boundary === null
) {
// This is an empty segment. There's nothing to write, so we can instead transfer the ID
// to the child. That way any existing references point to the child.
const childSegment = segment.children[0];
childSegment.id = segment.id;
childSegment.parentFlushed = true;
if (childSegment.status === COMPLETED) {
queueCompletedSegment(boundary, childSegment);
}
} else {
const completedSegments = boundary.completedSegments;
completedSegments.push(segment);
}
}

function finishedTask(
request: Request,
boundary: Root | SuspenseBoundary,
Expand Down Expand Up @@ -1463,7 +1486,7 @@ function finishedTask(
// 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);
queueCompletedSegment(boundary, segment);
}
}
if (boundary.parentFlushed) {
Expand All @@ -1482,8 +1505,8 @@ function finishedTask(
// 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) {
queueCompletedSegment(boundary, segment);
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
Expand Down