Skip to content

Fix odd state on load using React DOM batchedUpdates #42

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

Merged
merged 4 commits into from
Mar 25, 2020

Conversation

Grsmto
Copy link
Contributor

@Grsmto Grsmto commented Jan 28, 2020

Hey!

This fixes #38 .

I spent some time investigating this and I figured out, as you suggested it, that it was the two consequent calls to setState that were not being batched by React. These 2 setState are triggering 2 separate renders, creating an odd state.

This is a known "issue" in React, see facebook/react#17048

The workaround is to use the ReactDOM.unstable_batchedUpdates function that forces updates to be batched.

There are numbers of scenarios where this is needed, like to a 404 page or a "no result" state on a search feature.

Another solution would be to use useReducer instead of multiple setState to do multiple state changes in a single render.

One cons of this solution is that this relies on a React-DOM API which makes the code unusable in a React-Native environment.

Let me know what you think, thanks!

@jaydenseric
Copy link
Owner

Good job looking into it 🙌

I found a great article on the topic:

https://blog.logrocket.com/simplifying-state-management-in-react-apps-with-batched-updates

This issue will go away in React 17, but for now unstable_batchedUpdates seems an acceptable solution. Are there any other locations in the code that would benefit from using it?

React Native is not documented as supported, but any React Native graphql-react users please pipe up and we can look at catering for it.

I'll probably consider this for a bit, and maybe take a stab at a useReducer refactor first and see which approach is better. If you’re keen to speed things up, feel free to have a go at useReducer yourself :)

@jaydenseric jaydenseric merged commit 79249bb into jaydenseric:master Mar 25, 2020
@jaydenseric
Copy link
Owner

Thanks @Grsmto 🙌

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

Successfully merging this pull request may close these issues.

Weird query lifecycle transitions
2 participants