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 (facebook#28283)

In facebook#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.
  • Loading branch information
sebmarkbage authored and AndyPengc12 committed Apr 15, 2024
1 parent 7a98d96 commit 261a0cb
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1192,12 +1192,16 @@ function renderModelDestructive(
// 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.
const newId = outlineModel(request, (value: any));
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

0 comments on commit 261a0cb

Please sign in to comment.