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

Predicate function for updating component. #235

Closed
gacosta89 opened this Issue Jan 1, 2016 · 2 comments

Comments

3 participants
@gacosta89

gacosta89 commented Jan 1, 2016

It would be nice to have an optional predicate function to pass to the connect function, so we can do the usual further immutable data optimizations.

Something like this:

function connect(mapStateToProps, mapDispatchToProps, mergeProps, updateComponent, options = {})
....
const defaultUpdateComponent = (prevStoreState, storeState) =>  prevStoreState !== storeState;
const finalUpdateComponent = updateComponent || defaultUpdateComponent;

then in handleChange:

handleChange() {
....
  if (!pure || finalUpdateComponent(prevStoreState, storeState)) {
       this.hasStoreStateChanged = true
     this.setState({ storeState })
  }
}

P.D.: Happy New Year!

@DeadMG

This comment has been minimized.

Show comment
Hide comment
@DeadMG

DeadMG Jan 4, 2016

I've also noticed that for your own props, shallowEqual is used, but for the store props, it's strict object identity only. This somewhat concerns me as it seems that it would be slightly tricky to always preserve the exact object instance just to optimize the comparison a little bit which we may not even need- or in other words, data sharing between immutable instances becomes mandatory, rather than a nice optimization.

It would be nice if we could supply our own comparison or even just use shallowEqual.

DeadMG commented Jan 4, 2016

I've also noticed that for your own props, shallowEqual is used, but for the store props, it's strict object identity only. This somewhat concerns me as it seems that it would be slightly tricky to always preserve the exact object instance just to optimize the comparison a little bit which we may not even need- or in other words, data sharing between immutable instances becomes mandatory, rather than a nice optimization.

It would be nice if we could supply our own comparison or even just use shallowEqual.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jan 15, 2016

Collaborator

There is a possibility that we’ll make arePropsEqual configurable (#183) but I don't think we want to provide any more customization.

Just because that setState() call happens doesn't mean your inner component will actually get updated. We have a bunch of additional optimizations (in shouldComponentUpdate() and render()) that will try very hard to prevent renders if the slice of state you're actually interested in (as determined by mapStateToProps() you supplied) has not changed. We have a whole bunch of tests proving that those optimizations work and renders are only performed when absolutely necessary.

I've also noticed that for your own props, shallowEqual is used, but for the store props, it's strict object identity only.

This is incorrect. shallowEqual is used to compare all props, whether derived from parent props, or store. Please see here, here, and here.

This somewhat concerns me as it seems that it would be slightly tricky to always preserve the exact object instance just to optimize the comparison a little bit which we may not even need- or in other words, data sharing between immutable instances becomes mandatory, rather than a nice optimization.

Unfortunately I don't understand the problem you're describing at all. :-( Maybe some code would help?

Collaborator

gaearon commented Jan 15, 2016

There is a possibility that we’ll make arePropsEqual configurable (#183) but I don't think we want to provide any more customization.

Just because that setState() call happens doesn't mean your inner component will actually get updated. We have a bunch of additional optimizations (in shouldComponentUpdate() and render()) that will try very hard to prevent renders if the slice of state you're actually interested in (as determined by mapStateToProps() you supplied) has not changed. We have a whole bunch of tests proving that those optimizations work and renders are only performed when absolutely necessary.

I've also noticed that for your own props, shallowEqual is used, but for the store props, it's strict object identity only.

This is incorrect. shallowEqual is used to compare all props, whether derived from parent props, or store. Please see here, here, and here.

This somewhat concerns me as it seems that it would be slightly tricky to always preserve the exact object instance just to optimize the comparison a little bit which we may not even need- or in other words, data sharing between immutable instances becomes mandatory, rather than a nice optimization.

Unfortunately I don't understand the problem you're describing at all. :-( Maybe some code would help?

@gaearon gaearon closed this Jan 15, 2016

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