Skip to content
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

[@xstate/react] Leverage useSyncExternalStore() now that React 18 is in beta #2821

Closed
VanTanev opened this issue Nov 16, 2021 · 2 comments · Fixed by #2939
Closed

[@xstate/react] Leverage useSyncExternalStore() now that React 18 is in beta #2821

VanTanev opened this issue Nov 16, 2021 · 2 comments · Fixed by #2939

Comments

@VanTanev
Copy link
Contributor

Description
React 18 beta announcement
useSyncExternalStore() discussion
Redux PR for React 18 compat

@Andarist
Copy link
Member

Andarist commented Nov 16, 2021

We are keeping a close eye on those advancements - the change itself should be rather easy to introduce in a draft PR but until React 18 hits its stable version we can't start using useSyncExternalStore. There is a "shim" package (use-sync-external-store/shim) for it that we plan to use but its peer deps are declared in a way that installing this would result in warnings - and even if it didn't I wouldn't be super into releasing @xstate/react that would depend on smth like use-sync-external-store@0.0.0-experimental-45898dacb2-20210828 or use-sync-external-store@1.0.0-beta-96ca8d915-20211115 😅

I'll try to find some time to prepare a draft PR with the update that will just sit in the PRs until the final React 18 version gets published.

@VanTanev
Copy link
Contributor Author

VanTanev commented Dec 4, 2021

One minor thought here: When @xstate/react switches to useSyncExternalStore(), we should update the documentation that memorizing selectors is no longer required. I think this is a pretty great change in and of itself, because useSelector() type inference fails with an inline React.useCallback() for the selector, which is very annoying.

As far as I understand the shim, this change will also apply to React 17, as the shim does a clever check that skips a rerender even when the identity of the selector changes:

https://github.com/facebook/react/blob/3f9480f0f5ceb5a32a3751066f0b8e9eae5f1b10/packages/use-sync-external-store/src/useSyncExternalStoreShimClient.js#L96-L99

https://github.com/facebook/react/blob/3f9480f0f5ceb5a32a3751066f0b8e9eae5f1b10/packages/use-sync-external-store/src/useSyncExternalStoreShimClient.js#L130-L139

Edit:
Oh, it seems like the current @xstate/react code got updated to deal with inline selector functions already! I guess then useSyncExternalStore() is not relevant to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants