-
-
Notifications
You must be signed in to change notification settings - Fork 10.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
Remove useSyncExternalStore in favor of useState #10377
Conversation
🦋 Changeset detectedLatest commit: f3fd217 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
}); | ||
testWindow = dom.window as unknown as Window; | ||
testWindow.history.pushState({}, "", "/"); | ||
}); |
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.
Reusing the same window instance across these tests was. ... not smart on my part :)
package.json
Outdated
}, | ||
"packages/react-router/dist/umd/react-router.production.min.js": { | ||
"none": "15.6 kB" | ||
"none": "15.0 kB" |
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.
As a bonus we drop the use-sync-external-store/shim
and shave a few bytes
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
Sibling Remix PR: remix-run/remix#6121
Seems
useSyncExternalStore
wasn't the right solution all along, since it can cause updates to occur in unexpected orders. A minimal reproduction of the underlying issue is here: https://codesandbox.io/s/use-sync-external-store-loop-9g7b81But basically when using
useSyncExternalStore
to sync some externalstate
(as we were doing to sync the@remix-run/router
state), the following does not work as one would expect. TheexternalStore.increment()
causesstate
to update and re-run the effect prior toshouldIncrement=false
being flushed, so we end up in an infinite loop.@jacob-ebey and I's hunch is that these "sync" updates are flushed synchronously and thus they go ahead of normal
useState
updates that run asynchronously in concurrent mode.This is resolved by using local react state to "sync" the external state:
This seems to be the recommendation in the react docs as well:
Closes: remix-run/remix#6072