From b2392647b580e60eeacea7ecdb03bd0ba53ef2f7 Mon Sep 17 00:00:00 2001 From: sebmarkbage Date: Thu, 8 Feb 2024 23:50:18 +0000 Subject: [PATCH] [Flight] Serialize deduped elements by direct reference even if they 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 [ba5e6a8329c7194a2c573c037a37f24ce45ee58f](https://github.com/facebook/react/commit/ba5e6a8329c7194a2c573c037a37f24ce45ee58f) --- compiled/facebook-www/REVISION | 2 +- compiled/facebook-www/ReactDOMTesting-prod.modern.js | 6 +++--- .../facebook-www/ReactFlightDOMServer-dev.modern.js | 12 ++++++++---- .../facebook-www/ReactFlightDOMServer-prod.modern.js | 5 +++-- .../facebook-www/ReactTestRenderer-dev.modern.js | 2 +- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/compiled/facebook-www/REVISION b/compiled/facebook-www/REVISION index f1e38b118f01f..c5df1125ed7c8 100644 --- a/compiled/facebook-www/REVISION +++ b/compiled/facebook-www/REVISION @@ -1 +1 @@ -e41ee9ea70f8998144fdd959ac11fd7a40e4ee20 +ba5e6a8329c7194a2c573c037a37f24ce45ee58f diff --git a/compiled/facebook-www/ReactDOMTesting-prod.modern.js b/compiled/facebook-www/ReactDOMTesting-prod.modern.js index 2ba8d6fea77ec..0bfd8f673a880 100644 --- a/compiled/facebook-www/ReactDOMTesting-prod.modern.js +++ b/compiled/facebook-www/ReactDOMTesting-prod.modern.js @@ -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 = { @@ -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__; @@ -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"; diff --git a/compiled/facebook-www/ReactFlightDOMServer-dev.modern.js b/compiled/facebook-www/ReactFlightDOMServer-dev.modern.js index 1bf27e60f3035..c0b2e0d3ada6b 100644 --- a/compiled/facebook-www/ReactFlightDOMServer-dev.modern.js +++ b/compiled/facebook-www/ReactFlightDOMServer-dev.modern.js @@ -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 diff --git a/compiled/facebook-www/ReactFlightDOMServer-prod.modern.js b/compiled/facebook-www/ReactFlightDOMServer-prod.modern.js index fc9b664726ad3..0f49ccc376b25 100644 --- a/compiled/facebook-www/ReactFlightDOMServer-prod.modern.js +++ b/compiled/facebook-www/ReactFlightDOMServer-prod.modern.js @@ -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, diff --git a/compiled/facebook-www/ReactTestRenderer-dev.modern.js b/compiled/facebook-www/ReactTestRenderer-dev.modern.js index 0fe8e1302548c..c7f74b86aa078 100644 --- a/compiled/facebook-www/ReactTestRenderer-dev.modern.js +++ b/compiled/facebook-www/ReactTestRenderer-dev.modern.js @@ -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.