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 re-rendering the EditorRegions component on click #18776

Merged
merged 1 commit into from Nov 29, 2019

Conversation

@youknowriad
Copy link
Contributor

youknowriad commented Nov 27, 2019

This probably doesn't have a huge impact on performance but I noticed that when clicking anywhere on the canvas, the EditorRegions component re-renders for no particular reason (causing its subtree to rerender as well). This PR prevents that.

@gziolo
gziolo approved these changes Nov 29, 2019
Copy link
Member

gziolo left a comment

From https://reactjs.org/docs/react-component.html#setstate:

setState() will always lead to a re-render unless shouldComponentUpdate() returns false. If mutable objects are being used and conditional rendering logic cannot be implemented in shouldComponentUpdate(), calling setState() only when the new state differs from the previous state will avoid unnecessary re-renders.

All good then. I shared mostly for myself as a learning process 😄

@youknowriad

This comment has been minimized.

Copy link
Contributor Author

youknowriad commented Nov 29, 2019

@gziolo In useState it's handled automatically, another good point for hooks :)

@youknowriad youknowriad merged commit 5507ce8 into master Nov 29, 2019
1 of 2 checks passed
1 of 2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Canceled
Details
@youknowriad youknowriad deleted the perf/avoid-rerendering-editor-regions-on-clicck branch Nov 29, 2019
@gziolo

This comment has been minimized.

Copy link
Member

gziolo commented Nov 29, 2019

https://reactjs.org/blog/2017/09/26/react-v16.0.html#breaking-changes

The alternative way would be to return null inside the callback passed as the first param:

Calling setState with null no longer triggers an update. This allows you to decide in an updater function if you want to re-render.

and

In useState it's handled automatically, another good point for hooks :)

Totally, hooks are much nicer in 90% of cases :)

@youknowriad youknowriad added this to the Gutenberg 7.1 milestone Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.