-
Notifications
You must be signed in to change notification settings - Fork 535
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
Prevent automatic batching updates in ThemeProvider to ensure client use right colorMode #2732
Prevent automatic batching updates in ThemeProvider to ensure client use right colorMode #2732
Conversation
…use right colorMode
🦋 Changeset detectedLatest commit: 83a95e7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
f4cde90
to
8e90fb6
Compare
Thanks for taking the time to make Pull Request @renbaoshuo! 🙌 In your opinion, would there be a good test that we could add to make sure this doesn't happen again in the future? |
setColorMode(resolvedColorModeOnClient) | ||
// use ReactDOM.flushSync to prevent automatic batching of state updates since React 18 | ||
// ref: https://github.com/reactwg/react-18/discussions/21 | ||
ReactDOM.flushSync(() => { |
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.
Just checking, would this lead to an error with React 17?
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.
facebook/react#10225
facebook/react#11527 (comment)
It seems that ReactDOM.flushSync(batch)
is already introduced before React 17.
Sorry, I have no idea. |
8e90fb6
to
7214e0b
Compare
Closes #2731.
Use
ReactDOM.flushSync
to prevent automatic batching of state updates since React 18, which causes error described in #2731.Screenshots
None.
Merge checklist
Added/updated testsAdded/updated documentationTake a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.