-
Notifications
You must be signed in to change notification settings - Fork 49.3k
fix: React.use
inside React.lazy
-ed component on SSR
#33941
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
Conversation
/cc @acdlite |
cc @eps1lon |
Comparing: dffacc7...dc09e25 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 |
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.
Changes makes sense to me since the added check matches the one we do earlier before we call getSuspendedThenable
. Thank you!
It's unfortunate because we need the thenable state for other debug info. However, there's a general problem with "throw a Promise" and debugging that leads us to wanting to deprecate the whole feature anyway. |
) DiffTrain build for [cc01584](facebook@cc01584)
Summary
React.use
insideReact.lazy
-ed component returns otherReact.use
value on SSR #33937As indicated by the previous issue comment #27731 (comment), it looks like legacy "throw thenable"-based suspense implementation (used by React.lazy) causes an issue with
React.use
based suspense implementation, which relies onthenableState
hook state tracking.The PR fixes the issue by avoiding
getThenableStateAfterSuspending
when legacy "throw thenable" style suspension happened. I tried a different approach such as resettingtask.thenableState = null
duringrenderLazyComponent
, but it broke one test caseuseActionState can return binary state during MPA form submission
inpackages/react-server-dom-webpack/src/__tests__/ReactFlightDOMForm-test.js
.(By no means, I'm confident with the change, but I thought poking around the code provides better insight for short term workaround. See #33937 (comment). I'd appreciate any help on the issue.)
How did you test this change?
I added a test case in
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
based on the reproduction of the issue (and also #33938).