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

Avoid duplicate listeners when using useReactiveVar with React.StrictMode #7581

Merged
merged 2 commits into from
Jan 14, 2021

Conversation

jcreighton
Copy link
Contributor

Fixes #7509. Because StrictMode renders functional components twice, duplicate listeners were added to onNextChange. The trickier part of this is that it doesn't cause inconsistent results until the priority of the update is affected (in the reproduction, returning an unresolved Promise from the button's onClick handler resulted in a different priority level in React so the associated duplicate listeners were interleaved with other work resulting in a strange result). We now set onNextChange only on the initial mount and when the reactive var's value has changed.

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh yes, I definitely did not know about the <StrictMode/> double-rendering behavior when I was working on #6867. This makes sense!

Comment on lines 16 to 20
// equivalent to the following, but shorter.:
// useEffect(() => {
// const mute = rv.onNextChange(setValue);
// return () => mute();
// }, [value])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea to make this alternative code even more verbose, to emphasize the shortness of our version. 😂

benjamn added a commit that referenced this pull request Jan 14, 2021
@benjamn benjamn force-pushed the 7509-usereactivevar-strict-mode branch from 1458973 to f1d088f Compare January 14, 2021 20:13
@benjamn benjamn force-pushed the 7509-usereactivevar-strict-mode branch from f1d088f to 2e13d46 Compare January 14, 2021 20:15
@benjamn benjamn merged commit 3501f90 into main Jan 14, 2021
@jcreighton jcreighton deleted the 7509-usereactivevar-strict-mode branch April 2, 2021 21:06
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants