[Fizz] Don't flush empty segments (#24054)
Before this change, we would sometimes write segments without any content in them. For example for a Suspense boundary that immediately suspends we might emit something like: <div hidden id="123"> <template id="456"></template> </div> Where the outer div is just a temporary wrapper and the inner one is a placeholder for something to be added later. This serves no purpose. We should ideally have a heuristic that holds back segments based on byte size and time. However, this is a straight forward clear win for now.
Sebastian Markbåge committed
Mar 8, 2022 at 23:12 UTC
c91892ec3c180a8f85a030aea931b58e326a55a7
2 files changed
+30
-3
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
+4
@@ -495,6 +495,10 @@ describe('ReactDOMFizzServer', () => {
495
pipe(writable);
496
});
497
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);
498
+ // Because there is no content inside the Suspense boundary that could've
499
+ // been written, we expect to not see any additional partial data flushed
500
+ // yet.
501
+ expect(container.firstChild.nextSibling).toBe(null);
502
await act(async () => {
503
resolveElement({default: <Text text="Hello" />});
504
});
packages/react-server/src/ReactFizzServer.js
+26
-3
@@ -488,7 +488,7 @@ function renderSuspenseBoundary(
488
// We use the safe form because we don't handle suspending here. Only error handling.
489
renderNode(request, task, content);
490
contentRootSegment.status = COMPLETED;
491
- newBoundary.completedSegments.push(contentRootSegment);
491
+ queueCompletedSegment(newBoundary, contentRootSegment);
492
if (newBoundary.pendingTasks === 0) {
493
// This must have been the last segment we were waiting on. This boundary is now complete.
494
// Therefore we won't need the fallback. We early return so that we don't have to create
@@ -1430,6 +1430,29 @@ function abortTask(task: Task): void {
1430
}
1431
}
1432
1433
+function queueCompletedSegment(
1434
+ boundary: SuspenseBoundary,
1435
+ segment: Segment,
1436
+): void {
1437
+ if (
1438
+ segment.chunks.length === 0 &&
1439
+ segment.children.length === 1 &&
1440
+ segment.children[0].boundary === null
1441
+ ) {
1442
+ // This is an empty segment. There's nothing to write, so we can instead transfer the ID
1443
+ // to the child. That way any existing references point to the child.
1444
+ const childSegment = segment.children[0];
1445
+ childSegment.id = segment.id;
1446
+ childSegment.parentFlushed = true;
1447
+ if (childSegment.status === COMPLETED) {
1448
+ queueCompletedSegment(boundary, childSegment);
1449
+ }
1450
+ } else {
1451
+ const completedSegments = boundary.completedSegments;
1452
+ completedSegments.push(segment);
1453
+ }
1454
+}
1455
+
1456
function finishedTask(
1457
request: Request,
1458
boundary: Root | SuspenseBoundary,
@@ -1463,7 +1486,7 @@ function finishedTask(
1486
// If it is a segment that was aborted, we'll write other content instead so we don't need
1487
// to emit it.
1488
if (segment.status === COMPLETED) {
1466
- boundary.completedSegments.push(segment);
1489
+ queueCompletedSegment(boundary, segment);
1490
}
1491
}
1492
if (boundary.parentFlushed) {
@@ -1482,8 +1505,8 @@ function finishedTask(
1505
// If it is a segment that was aborted, we'll write other content instead so we don't need
1506
// to emit it.
1507
if (segment.status === COMPLETED) {
1508
+ queueCompletedSegment(boundary, segment);
1509
const completedSegments = boundary.completedSegments;
1486
- completedSegments.push(segment);
1510
if (completedSegments.length === 1) {
1511
// This is the first time since we last flushed that we completed anything.
1512
// We can schedule this boundary to emit its partially completed segments early