Skip to content

Commit

Permalink
[Fizz] Fix for failing id overwrites for postpone (#27684)
Browse files Browse the repository at this point in the history
When we postpone during a render we inject a new segment synchronously
which we postpone. That gets assigned an ID so we can refer to it
immediately in the postponed state.

When we do that, the parent segment may complete later even though it's
also synchronous. If that ends up not having any content in it, it'll
inline into the child and that will override the child's segment id
which is not correct since it was already assigned one.

To fix this, we simply opt-out of the optimization in that case which is
unfortunate because we'll generate many more unnecessary empty segments.
So we should come up with a new strategy for segment id assignment but
this fixes the bug.

Co-authored-by: Josh Story <[email protected]>

DiffTrain build for [0e352ea](0e352ea)
  • Loading branch information
sebmarkbage committed Nov 10, 2023
1 parent 69de556 commit c907964
Show file tree
Hide file tree
Showing 9 changed files with 20 additions and 14 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
78c71bc545bf5c0fdeedc023b69fafe05d988067
0e352ea01c0209b6c5e23f0e857af4e01c783024
2 changes: 1 addition & 1 deletion compiled/facebook-www/ReactART-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ if (__DEV__) {
return self;
}

var ReactVersion = "18.3.0-www-modern-a2bd2f78";
var ReactVersion = "18.3.0-www-modern-adf7c13c";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down
4 changes: 2 additions & 2 deletions compiled/facebook-www/ReactART-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -9904,7 +9904,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "18.3.0-www-modern-15ac94d7",
version: "18.3.0-www-modern-d15e6948",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1302 = {
Expand Down Expand Up @@ -9935,7 +9935,7 @@ var internals$jscomp$inline_1302 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-modern-15ac94d7"
reconcilerVersion: "18.3.0-www-modern-d15e6948"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1303 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
5 changes: 3 additions & 2 deletions compiled/facebook-www/ReactDOMServer-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ if (__DEV__) {
var React = require("react");
var ReactDOM = require("react-dom");

var ReactVersion = "18.3.0-www-classic-af17cc87";
var ReactVersion = "18.3.0-www-classic-deec0503";

// This refers to a WWW module.
var warningWWW = require("warning");
Expand Down Expand Up @@ -13268,7 +13268,8 @@ if (__DEV__) {
if (
segment.chunks.length === 0 &&
segment.children.length === 1 &&
segment.children[0].boundary === null
segment.children[0].boundary === null &&
segment.children[0].id === -1
) {
// 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.
Expand Down
5 changes: 3 additions & 2 deletions compiled/facebook-www/ReactDOMServer-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ if (__DEV__) {
var React = require("react");
var ReactDOM = require("react-dom");

var ReactVersion = "18.3.0-www-modern-a2bd2f78";
var ReactVersion = "18.3.0-www-modern-adf7c13c";

// This refers to a WWW module.
var warningWWW = require("warning");
Expand Down Expand Up @@ -12996,7 +12996,8 @@ if (__DEV__) {
if (
segment.chunks.length === 0 &&
segment.children.length === 1 &&
segment.children[0].boundary === null
segment.children[0].boundary === null &&
segment.children[0].id === -1
) {
// 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.
Expand Down
5 changes: 3 additions & 2 deletions compiled/facebook-www/ReactDOMServer-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -4571,7 +4571,8 @@ function queueCompletedSegment(boundary, segment) {
if (
0 === segment.chunks.length &&
1 === segment.children.length &&
null === segment.children[0].boundary
null === segment.children[0].boundary &&
-1 === segment.children[0].id
) {
var childSegment = segment.children[0];
childSegment.id = segment.id;
Expand Down Expand Up @@ -5407,4 +5408,4 @@ exports.renderToString = function (children, options) {
'The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToReadableStream" which supports Suspense on the server'
);
};
exports.version = "18.3.0-www-classic-acad9d50";
exports.version = "18.3.0-www-classic-1cac58c7";
5 changes: 3 additions & 2 deletions compiled/facebook-www/ReactDOMServer-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -4506,7 +4506,8 @@ function queueCompletedSegment(boundary, segment) {
if (
0 === segment.chunks.length &&
1 === segment.children.length &&
null === segment.children[0].boundary
null === segment.children[0].boundary &&
-1 === segment.children[0].id
) {
var childSegment = segment.children[0];
childSegment.id = segment.id;
Expand Down Expand Up @@ -5342,4 +5343,4 @@ exports.renderToString = function (children, options) {
'The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToReadableStream" which supports Suspense on the server'
);
};
exports.version = "18.3.0-www-modern-15ac94d7";
exports.version = "18.3.0-www-modern-d15e6948";
3 changes: 2 additions & 1 deletion compiled/facebook-www/ReactDOMServerStreaming-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -12875,7 +12875,8 @@ if (__DEV__) {
if (
segment.chunks.length === 0 &&
segment.children.length === 1 &&
segment.children[0].boundary === null
segment.children[0].boundary === null &&
segment.children[0].id === -1
) {
// 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.
Expand Down
3 changes: 2 additions & 1 deletion compiled/facebook-www/ReactDOMServerStreaming-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -4343,7 +4343,8 @@ function queueCompletedSegment(boundary, segment) {
if (
0 === segment.chunks.length &&
1 === segment.children.length &&
null === segment.children[0].boundary
null === segment.children[0].boundary &&
-1 === segment.children[0].id
) {
var childSegment = segment.children[0];
childSegment.id = segment.id;
Expand Down

0 comments on commit c907964

Please sign in to comment.