Fix useSyncExternalStore dropped update when state is dispatched in render phase #25578
Conversation
This file contains 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
If state is dispatched during render phase, then work in progress `hook` will hold already updated memoized state, defeating previous value comparison. Using `currentHook` instead fixes that issue.
|
Comparing: cf3932b...776bd87 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
eps1lon
changed the title
2 tasks
facebook-github-bot
pushed a commit
to facebook/react-native
that referenced
this pull request
Nov 10, 2022
Summary: This sync includes the following changes: - **[d1e35c703](facebook/react@d1e35c703 )**: Don't disappear layout effects unnecessarily ([#25660](facebook/react#25660)) //<Samuel Susla>// - **[1e3e30dae](facebook/react@1e3e30dae )**: Fix useSyncExternalStore dropped update when state is dispatched in render phase ([#25578](facebook/react#25578)) //<Aurélien Chivot-Buhler>// Changelog: [General][Changed] - React Native sync for revisions 4bd245e...d1e35c7 jest_e2e[run_all_tests] Reviewed By: GijsWeterings Differential Revision: D41187776 fbshipit-source-id: 9c82b79458487191f4a26cf643321603d30cea5a
mofeiZ
pushed a commit
to mofeiZ/react
that referenced
this pull request
Nov 17, 2022
rickhanlonii
pushed a commit
that referenced
this pull request
Dec 3, 2022
2 tasks
OlimpiaZurek
pushed a commit
to OlimpiaZurek/react-native
that referenced
this pull request
May 22, 2023
Summary: This sync includes the following changes: - **[d1e35c703](facebook/react@d1e35c703 )**: Don't disappear layout effects unnecessarily ([facebook#25660](facebook/react#25660)) //<Samuel Susla>// - **[1e3e30dae](facebook/react@1e3e30dae )**: Fix useSyncExternalStore dropped update when state is dispatched in render phase ([facebook#25578](facebook/react#25578)) //<Aurélien Chivot-Buhler>// Changelog: [General][Changed] - React Native sync for revisions 4bd245e...d1e35c7 jest_e2e[run_all_tests] Reviewed By: GijsWeterings Differential Revision: D41187776 fbshipit-source-id: 9c82b79458487191f4a26cf643321603d30cea5a
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.
Summary
Fix #25565
In
updateSyncExternalStore, If state is dispatched during render phase, then work in progresshookwill hold already updated memoized state in the second pass, defeating previous value comparison. As a consequence, the effect updating internal store value with next value will not be scheduled.This breaks re-rendering when store is set back to it's mounting value.
Using
currentHookinstead (when defined) fixes that issue.How did you test this change?
See included test