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

Reference Equality Check before setState() #1437

Closed
matianfu opened this issue Feb 23, 2016 · 5 comments · Fixed by reduxjs/react-redux#348
Closed

Reference Equality Check before setState() #1437

matianfu opened this issue Feb 23, 2016 · 5 comments · Fixed by reduxjs/react-redux#348
Labels

Comments

@matianfu
Copy link

Correct me if I'm wrong.

I noticed in Connect component's handleChange() method, each time store state changes, the setState() method is triggered. So each time the store state changes, all mounted container component generated by Connect() will fire setState(), and in turn, shouldComponentUpdate() would be called, and as long as store state changes, render() will be called.

I wonder if we can do a "reference equality check" before setState() is called. Would this eliminate many unnecessary calls to render() and improve performance?

My understanding is the setState() is designed for UI's local state, not the global state change irrelevent to this component. So that any state change will trigger all container component's method should be avoided.

@gaearon
Copy link
Contributor

gaearon commented Feb 23, 2016

This would be more relevant for https://github.com/reactjs/react-redux but I’ll answer here. We can’t check it reliably before because we don’t want to call mapStateToProps(state, ownProps) until the rendering. The reason here is complicated (see reduxjs/react-redux#99 and reduxjs/react-redux#225 if you’d like) but the problem is related to the ownProps parameter which can be outdated by the time handleChange() runs if parent and nested components are updated at the same time.

Since we can’t bail out early, we do some easy bail outs in shouldComponentUpdate() but the more essential bailouts are in render() method itself where we return the cached element if the computed props are shallowly equal. For React, returning a constant element is as good as implementing as bailout in shouldComponentUpdate().

I wonder if we can do a "reference equality check" before setState() is called. Would this eliminate many unnecessary calls to render() and improve performance?

Now that I think of it we can probably do this as an optimization when neither mapStateToProps nor mapDispatchToProps depend on the parent props. We calculate this here and here. Maybe we can add a separate optimization code path that calculates this.stateProps and this.dispatchProps early and bails out immediately when these checks are false. If you’d like to work on this, please let me know.

@gaearon
Copy link
Contributor

gaearon commented Feb 23, 2016

We can discuss in reduxjs/react-redux#300.

@gaearon
Copy link
Contributor

gaearon commented Apr 12, 2016

This is now fixed in the common case in react-redux@4.4.4.
For other cases, #348 contains a valuable tip.

@c10b10
Copy link

c10b10 commented Apr 20, 2016

For React, returning a constant element is as good as implementing as bailout in shouldComponentUpdate()

Can you please detail what you mean by a "constant element"?

@gaearon
Copy link
Contributor

gaearon commented Apr 20, 2016

We save the previous return value of render and return it again if everything else is equal. In most cases this gives the same performance optimizations as shouldComponentUpdate. React sees that the element is referentially equal and bails out.

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

Successfully merging a pull request may close this issue.

3 participants