Skip to content

Commit

Permalink
[Flight] Serialize deduped elements by direct reference even if they …
Browse files Browse the repository at this point in the history
…suspend (#28283)

In #28123 I switched these to be lazy references. However that creates a
lazy wrapper even if they're synchronously available. We try to as much
as possible preserve the original data structure in these cases.

E.g. here in the dev outlining I only use a lazy wrapper if it didn't
complete synchronously:
https://github.com/facebook/react/pull/28272/files#diff-d4c9c509922b3671d3ecce4e051df66dd5c3d38ff913c7a7fe94abc3ba2ed72eR638

Unfortunately we don't have a data structure that tracks the status of
each emitted row. We could store the task in the map but then they
couldn't be GC:ed as they complete. We could maybe store the status of
each element but seems so heavy.

For now I just went back to direct reference which might be an issue
since it can suspend something higher up when deduped.

DiffTrain build for [ba5e6a8](ba5e6a8)
  • Loading branch information
sebmarkbage committed Feb 8, 2024
1 parent 64de3b0 commit b239264
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 11 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
e41ee9ea70f8998144fdd959ac11fd7a40e4ee20
ba5e6a8329c7194a2c573c037a37f24ce45ee58f
6 changes: 3 additions & 3 deletions compiled/facebook-www/ReactDOMTesting-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -17066,7 +17066,7 @@ Internals.Events = [
var devToolsConfig$jscomp$inline_1784 = {
findFiberByHostInstance: getClosestInstanceFromNode,
bundleType: 0,
version: "18.3.0-www-modern-da44ba18",
version: "18.3.0-www-modern-be690e4d",
rendererPackageName: "react-dom"
};
var internals$jscomp$inline_2157 = {
Expand Down Expand Up @@ -17097,7 +17097,7 @@ var internals$jscomp$inline_2157 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-modern-da44ba18"
reconcilerVersion: "18.3.0-www-modern-be690e4d"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_2158 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down Expand Up @@ -17525,4 +17525,4 @@ exports.useFormStatus = function () {
return ReactCurrentDispatcher$2.current.useHostTransitionStatus();
throw Error(formatProdErrorMessage(248));
};
exports.version = "18.3.0-www-modern-da44ba18";
exports.version = "18.3.0-www-modern-be690e4d";
12 changes: 8 additions & 4 deletions compiled/facebook-www/ReactFlightDOMServer-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -1952,12 +1952,16 @@ if (__DEV__) {
// but that is able to reuse the same task if we're already in one but then that
// will be a lazy future value rather than guaranteed to exist but maybe that's good.
var newId = outlineModel(request, value);
return serializeLazyID(newId);
return serializeByValueID(newId);
} else {
// We've already emitted this as an outlined object, so we can refer to that by its
// existing ID. We use a lazy reference since, unlike plain objects, elements might
// suspend so it might not have emitted yet even if we have the ID for it.
return serializeLazyID(_existingId);
// existing ID. TODO: We should use a lazy reference since, unlike plain objects,
// elements might suspend so it might not have emitted yet even if we have the ID for
// it. However, this creates an extra wrapper when it's not needed. We should really
// detect whether this already was emitted and synchronously available. In that
// case we can refer to it synchronously and only make it lazy otherwise.
// We currently don't have a data structure that lets us see that though.
return serializeByValueID(_existingId);
}
} else {
// This is the first time we've seen this object. We may never see it again
Expand Down
5 changes: 3 additions & 2 deletions compiled/facebook-www/ReactFlightDOMServer-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -783,8 +783,9 @@ function renderModelDestructive(
if (modelRoot === value) modelRoot = null;
else
return -1 === parentPropertyName
? "$L" + outlineModel(request, value).toString(16)
: "$L" + parentPropertyName.toString(16);
? ((request = outlineModel(request, value)),
serializeByValueID(request))
: serializeByValueID(parentPropertyName);
} else parent.set(value, -1);
return renderElement(
request,
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/ReactTestRenderer-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -26053,7 +26053,7 @@ if (__DEV__) {
return root;
}

var ReactVersion = "18.3.0-www-modern-2a20f3fe";
var ReactVersion = "18.3.0-www-modern-9c82ad3d";

// Might add PROFILE later.

Expand Down

0 comments on commit b239264

Please sign in to comment.