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

Sample code for PanelStack using wrong variant of setState #3242

Closed
ChristianIvicevic opened this issue Dec 14, 2018 · 2 comments
Closed

Sample code for PanelStack using wrong variant of setState #3242

ChristianIvicevic opened this issue Dec 14, 2018 · 2 comments

Comments

@ChristianIvicevic
Copy link
Contributor

While reading through the new sample code for the PanelStack component I noticed multiple calls to setState which seem to be wrong:

this.setState({ currentPanelStack: [newPanel, ...this.state.currentPanelStack] });

this.setState({ currentPanelStack: this.state.currentPanelStack.slice(1) });

If you need the current value of state while processing setState you should use the overload setState(previousState => ({/* ... */})) since the actual change of the state is not immediate.

@giladgray
Copy link
Contributor

@ChristianIvicevic ah good catch. I suppose the current impl doesn't explode because these handlers are only called in response to user events, which happen rarely (not more than once per frame), but your proposal is the best practice.

mind submitting a PR? ⭐️

@ChristianIvicevic
Copy link
Contributor Author

@giladgray Will do it later this weekend ⭐️

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

No branches or pull requests

2 participants