Request: pass own props to areStatesEqual

[TiddoLangerak]

I want to implement areStatesEqual in such a way that it only compares the relevant state for a container. However, this relevant state might be dependent on the container's own props. E.g. suppose my state looks like this:

{
  users : {
    1 : { /* user1 */ },
    2 : { /* user2 */ }
  }
}

and I have a container <User id={userId}>. Currently, I can't access the id from the areStatesEqual function, and as such the best I can do is compare the entire users object. This gives a lot more false positives though, and is generally more expensive as well. If I had access to the properties of the container then I could make a much more efficient areStatesEqual function.

[timdorr]

I think you're looking too high up in the tree. areStatesEqual is to allow for alternative state containers, such as immutable.js. I believe you want to use areStatePropsEqual, which can utilize props.

[TiddoLangerak]

areStatePropsEqual is too low in the tree though, since it concerns the component props, not the container's own props. areStatePropsEqual is only executed after mapStateToProps is executed, so if mapStateToProps is expensive then this isn't ideal. The documentation itself recommends to override areStatesEqual when mapStateToProps is expensive, but without access to the container's own props it's not always possible to properly implement this function. To give a more complete example (with flow types for better illustration):

/*
const state = {
  users : {
    1 : { ... },
    2 : { ... }
  }
}
*/


type UserComponentProps = {
  user : User
};
function UserComponent({ user } : UserComponentProps) {
   return <div>
     {/* snip */}
  </div>
}

type UserContainerProps = {
  id : number
}
function mapStateToProps(state, ownProps : UserContainerProps) {
   // assume `getUserFromState` is expensive
   return getUserFromState(state, ownProps.id);
}

const UserContainer = connect(mapStateToProps)(UserComponent);

Now suppose I have a <UserContainer id={1}> component. I only want this component to update when the user with id=1 changes in the store. Currently these are my choices:

• areStatesEquals: (oldState, newState) => isEqual(oldState.users, newState.users). This only prevents updates when other store slices are changed, but it still updates <UserContainer id={1}> when user with id=2 is updated.
• areStatePropsEquals: (oldProps, newProps) => isEqual(oldProps, newProps). This does prevent the component from being updated, but it doesn't prevent the mapStateToProps function from being called. When mapStateToProps is expensive the just having an areStatePropsEquals might not be sufficient.

If instead I could define a function areRelevantStatesEquals : (oldState, newState, oldOwnProps, newOwnProps) => isEqual(oldState.users[oldOwnProps.id], newState.users[newOldProps.id]) then I can very cheaply prevent updates to both the container and component.

[timdorr]

I'd argue that anything in your mapStateToProps shouldn't be expensive. I would work to make that easier to query by doing some of the work upfront in your reducer. Or you can use something like reselect (react-redux uses it internally, so you already have it in your project) to make it cacheable.

[markerikson]

Nitpicking a bit: React-Redux does not depend on Reselect. All of connect's selectors are implemented internally.

[timdorr]

Oh yeah, duh. I still hang on to that because of Jim's initial implementation for some reason.

[guillaume86]

Could please someone reconsider?

This feature feels half baked without ownProps support and there is obviously a demand for it, even the code change is very minor (see #865).

@TiddoLangerak is giving the root reason, ownProps allows to reuse the same component with different state slices, it's a basic requirement for any non trivial redux app.
The main use case will probably be list items, which have tighter performance requirements than one off components for obvious reasons so it seems misguided to prevent the use of this feature when it is the most needed...

[pmualaba]

The change is less then 1 line of code and makes the areStatesEqual way more flexible if nextOwnProps and ownProps arguments are available to control and implement your custom rerender prevention strategy.

Since adding those to 2 arguments is completely backwards compatible, I also would like to see this PR merged... Can this again be reconsidered?

Thanks!

[p0wl]

Any update on this? this has many upvotes and I have a similar case where this is needed.

[markerikson]

@timdorr : thinking about this a bit, I don't see any overriding reason why we can't or shouldn't implement passing ownProps to areStatesEqual. Goodness knows we've got the variable reference in scope right there.

[timdorr]

Would we be passing nextOwnProps? I believe that's right. Seems easy enough.

[jspurlin]

@timdorr / @markerikson mind taking a look at #1947? There's definitely been interest in adding this functionality and it sounds like you aren't opposed to it. Allowing this enables the ability for much more fine-grained checks to be performed in areStatesEqual (decreasing the false negatives).