-
Notifications
You must be signed in to change notification settings - Fork 50.5k
[Flight] Fix encodeReply for JSX with temporary references
#35730
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Flight] Fix encodeReply for JSX with temporary references
#35730
Conversation
`encodeReply` throws "React Element cannot be passed to Server Functions from the Client without a temporary reference set" when a React element is the root value of a `serializeModel` call (either passed directly or resolved from a promise), even when a temporary reference set is provided. The cause is that `resolveToJSON` hits the `REACT_ELEMENT_TYPE` switch case before reaching the `existingReference`/`modelRoot` check that regular objects benefit from. The synthetic JSON root created by `JSON.stringify` is never tracked in `writtenObjects`, so `parentReference` is `undefined` and the code falls through to the throw. This adds a `modelRoot` check in the `REACT_ELEMENT_TYPE` case, following the same pattern used for promises and plain objects. The added `JSX as root model` test also uncovered a pre-existing crash in the Flight Client: when the JSX element round-trips back, it arrives as a frozen object (client-created elements are frozen in DEV), and `Object.defineProperty` for `_debugInfo` fails because frozen objects are non-configurable. The same crash can occur with JSX exported as a client reference. For now, we're adding `!Object.isFrozen()` guards in `moveDebugInfoFromChunkToInnerValue` and `addAsyncInfo` to prevent the crash, which means debug info is silently dropped for frozen elements. The proper fix would likely be to clone the element so each rendering context gets its own mutable copy with correct debug info. closes facebook#34984 closes facebook#35690
|
Comparing: 2dd9b7c...a87af0f Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
| }); | ||
|
|
||
| it('can pass JSX as root model through a round trip using temporary references', async () => { | ||
| const jsx = <div />; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this server or client JSX?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Client, which is the default in our tests, and we use ReactServer.createElement if we want server JSX.
eps1lon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's discuss a proper fix.
eps1lon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going with this since there's another bug where debug info is added multiple times for shared elements. No info is better than wrong info.
[diff facebook/react@2dd9b7cf...272441a9](facebook/react@2dd9b7c...272441a) <details> <summary>React upstream changes</summary> - facebook/react#35731 - facebook/react#35730 </details>
encodeReplythrows "React Element cannot be passed to Server Functions from the Client without a temporary reference set" when a React element is the root value of aserializeModelcall (either passed directly or resolved from a promise), even when a temporary reference set is provided.The cause is that
resolveToJSONhits theREACT_ELEMENT_TYPEswitch case before reaching theexistingReference/modelRootcheck that regular objects benefit from. The synthetic JSON root created byJSON.stringifyis never tracked inwrittenObjects, soparentReferenceisundefinedand the code falls through to the throw. This adds amodelRootcheck in theREACT_ELEMENT_TYPEcase, following the same pattern used for promises and plain objects.The added
JSX as root modeltest also uncovered a pre-existing crash in the Flight Client: when the JSX element round-trips back, it arrives as a frozen object (client-created elements are frozen in DEV), andObject.definePropertyfor_debugInfofails because frozen objects are non-configurable. The same crash can occur with JSX exported as a client reference. For now, we're adding!Object.isFrozen()guards inmoveDebugInfoFromChunkToInnerValueandaddAsyncInfoto prevent the crash, which means debug info is silently dropped for frozen elements. The proper fix would likely be to clone the element so each rendering context gets its own mutable copy with correct debug info.closes #34984
closes #35690