@samitouri / QOS-React / commits / ba5e6a8329

[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.

Sebastian Markbåge committed Feb 8, 2024 at 15:45 UTC ba5e6a8329c7194a2c573c037a37f24ce45ee58f

1 file changed +8 -4

packages/react-server/src/ReactFlightServer.js

+8 -4

@@ -1192,12 +1192,16 @@ function renderModelDestructive(

1192 // but that is able to reuse the same task if we're already in one but then that

1193 // will be a lazy future value rather than guaranteed to exist but maybe that's good.

1194 const newId = outlineModel(request, (value: any));

1195 - return serializeLazyID(newId);

1195 + return serializeByValueID(newId);

1196 } else {

1197 // We've already emitted this as an outlined object, so we can refer to that by its

1198 - // existing ID. We use a lazy reference since, unlike plain objects, elements might

1199 - // suspend so it might not have emitted yet even if we have the ID for it.

1200 - return serializeLazyID(existingId);

1198 + // existing ID. TODO: We should use a lazy reference since, unlike plain objects,

1199 + // elements might suspend so it might not have emitted yet even if we have the ID for

1200 + // it. However, this creates an extra wrapper when it's not needed. We should really

1201 + // detect whether this already was emitted and synchronously available. In that

1202 + // case we can refer to it synchronously and only make it lazy otherwise.

1203 + // We currently don't have a data structure that lets us see that though.

1204 + return serializeByValueID(existingId);

1205 }

1206 } else {

1207 // This is the first time we've seen this object. We may never see it again