[Flight] Allow cyclic references to be serialized when unwrapping lazy elements#312
Closed
everettbu wants to merge 2 commits into
Closed
[Flight] Allow cyclic references to be serialized when unwrapping lazy elements#312everettbu wants to merge 2 commits into
everettbu wants to merge 2 commits into
Conversation
Fails with: ``` TypeError: Converting circular structure to JSON --> starting at object with constructor 'Object' --- property 'self' closes the circle ```
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Fixed cyclic reference serialization when unwrapping lazy elements by preserving the parent connection.
- Changed
renderModelDestructivecall inREACT_LAZY_TYPEcase to passparentandparentPropertyNameinstead ofemptyRootand empty string - This enables the
writtenObjectsWeakMap to properly track object references through the parent chain for cycle detection - Added comprehensive test that verifies cyclic objects survive two rounds of Flight serialization/deserialization
- The fix specifically addresses an advanced scenario where Flight Client results are re-serialized with Flight Server (e.g., for payload slicing)
Confidence Score: 5/5
- This PR is safe to merge with no issues found
- The change is minimal, targeted, and logically correct - it preserves the parent chain needed for cycle detection when unwrapping lazy elements. The implementation aligns with how other parts of the codebase handle object tracking via
writtenObjects. The test thoroughly validates the fix by testing the exact failure scenario (cyclic references through two serialization rounds). - No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| packages/react-server/src/ReactFlightServer.js | 5/5 | Correctly preserves parent chain when unwrapping lazy elements to enable cyclic reference serialization |
| packages/react-server-dom-webpack/src/tests/ReactFlightDOMEdge-test.js | 5/5 | Comprehensive test verifying cyclic references survive two rounds of serialization/deserialization |
Author
|
Upstream PR was closed or merged. Code is synced via branch mirror. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirror of facebook/react#35471
Original author: unstubbable
When
renderModelDestructiveunwraps a lazy element and subsequently callsrenderModelDestructiveagain with the resolved model, we should preserve the parent connection so that cyclic references can be serialized properly. This can occur in an advanced scenario where the result from the Flight Client is serialized again with the Flight Server, e.g. for slicing a precomputed payload into multiple parts.Note: The added test only fails when run with
--prod. In dev mode, the component info outlining prevents the issue from occurring.