-
Notifications
You must be signed in to change notification settings - Fork 47.3k
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
React 18 - production mode only: 2 * useSyncExternalStore + useState = Explosion #23250
Comments
It's usually faster to just create a codesandbox to verify. You can deploy a codesandbox to netlify or vercel to test production. |
@eps1lon Okay here is the repro: https://codesandbox.io/s/eloquent-rosalind-f6riy?file=/src/App.js Unfortunately the bug reproduces 100% -- our actual production application and the codesandbox crash in the exactly same way, in a couple of seconds. |
@eps1lon Here is a second, slightly different repro, which results in a different error: https://codesandbox.io/s/goofy-glade-3xz29?file=/src/App.js I'm also seeing a third error in our real app (useRef returning undefined) but that one I haven't reproduced yet. Also, when trying these out, please remember that the problem is production-mode-only. |
It would help a lot to have a failing test like one of these: |
If it only affects production mode, I highly suspect it's related to this bug, which is fixed in main: #23150 Are you able to reproduce using the latest prerelease? Try updating React and React DOM to |
This evenutally crashed with https://reactjs.org/docs/error-decoder.html/?invariant=300: https://csb-2szpyh-hswzi5eco-eps1lon.vercel.app/ Using Versions can be verified in React Devtools > Components tab > click on root > "react-dom@...." |
Thanks @acdlite I can confirm that the latest main does not crash our production app. As a separate issue, I think you should consider the versioning and/or comms -- companies will be testing against the highest numbered RC.X tag (RC.0 has more downloads than all the later versions combined, so our company is clearly not the exception). But in reality, the RC.0 is not even close to the actual release candidate and the community is now largely testing the wrong / old version. PS @gaearon sorry, I had missed your earlier reply. |
There’s nothing wrong with testing RC0 per se. We try to batch up changes before cutting the next RC so that there’s a limited number of versions people are actively testing. RC1 will go out soon. |
I agree we could have done a better job communicating about this specific bugfix but RC.0 is really close to what's in main. The only reason we haven't bumped to RC.1 yet is because we wanted to also include some improvements to error reporting (#23207) in the same version. Regarding whether the RC is the right label: we were in beta for months without receiving this bug report — we only got it after the bump to RC got us an additional wave of testers. So while I understand it's frustrating that something marked "release candidate" contained a bug, the purpose of an RC is to confirm whether there are any remaining issues so we can fix them before going to stable. Thank you for reporting the bug and please let us know if you find any additional issues so we can fix them! |
I tried to migrate an app from React 18 beta and useMutableSource to RC0 and useSyncExternalStore. Everything worked fine, except:
The exact 'explosion' seems to depend on where/how useState setter is invoked and/or what other hooks are used. In some cases useRef starts returning 'undefined' and in some cases useCallback starts throwing internally "typeError: Cannot read property 'length' of undefined".
The above is in our real app - I haven't tried isolated repro. Also in our app, the two useSyncExternalStore hooks are subscribed to different stores, not sure if that is related or relevant.
Maybe you could quickly add the above case (2x useSyncExternalStore + useState) to your existing tests, and if it doesn't reproduce the bug, then I can do a minimal isolated repro?
The text was updated successfully, but these errors were encountered: