React-redux: RFC: `mapStateToProps` or `mapStoreToProps` naming convention?

[samcorcos]

I’ve taught about 10 people how to use react-redux at this point and a common sticking point is the use of mapStateToProps. This seems to be due to confusion in terminology between local state and the state of the store.

In teaching these React-newbies, I’ve found that when I describe the mapStateToProps in terms of mapStoreToProps, they understand it very quickly, since it's not immediately clear to them based on the name that the values they're mapping in mapStateToProps come from the store.

I propose changing mapStateToProps to mapStoreToProps, as I think it more accurately reflects what the function is doing: it's selecting values from the store and mapping them the component’s props.

const mapStoreToProps = store => {
  return {
    user: store.user
  }
}

I realize this is only a superficial change, since the name of the function doesn’t actually affect how connect uses it, but I think it might make it easier for React-newbies to understand what the function is actually doing.

Thoughts?

[gaearon]

This will however be wrong because store is a different thing. It’s an object with getState() and dispatch() methods. We can’t refer to store state as store because this is even more confusing. And we can’t pass store itself because we want to hide dipspatch by design.

[jimbolla]

I agree with @gaearon that mapStoreToProps would be incorrect. I suppose mapStoreStateToProps would be correct and would be slightly more clear, but ugh... getting too verbose for my liking.

I will say that the method signature for connect has been an issue for several of my team, who I'll admit probably just need to read the docs a little more carefully. I've thought a lot about alternate signatures but so far haven't come up with anything that's clearly better.

[samcorcos]

Yeah, I agree regarding mapStoreStateToProps.

What if we turned the name around? mapStateFromStore?

[gaearon]

Then it’s not clear what you map it to, and some people will try to return arrays or primitive values from the method which won’t work.

[samcorcos]

Ok, fair enough. Thanks for considering.

[jchook]

Hey sorry to dig up this old topic. It gave me a partial answer to a question I've had for some time.

@gaearon wrote:

And we can’t pass store itself because we want to hide dipspatch by design.

I'm curious why we want to hide dispatch instead of mapping state and dispatch together. Why wouldn't we have a single mapStoreToProps(state, dispatch, ownProps)?

If it is for performance gains, have these gains been measured in a benchmark?

[markerikson]

@jchook : We discourage tying together state updates and method binding to dispatch so that users avoid re-creating functions every time an action is dispatched or the store state changes. Now, Ryan Florence did write a recent article discussing perf aspects of creating new functions inside of render methods, and some of his points are relevant here. Creating new functions is not necessarily an end-of-the-world performance killer. However, it's also not going to be _good_ for perf, either. So, the core parts of the connect() API prevent that pattern by default. If you _really_ want to do so, you can implement that behavior using the lesser-known third option to connect: the mergeProps argument.

Also, as a general rule, most connected components are going to need state updates far more frequently than they will updates to bound action creators. It makes sense to separate those out functionally.

[jchook]

Thanks for the explanation and the link!

Also found this in the codebase:
https://github.com/reactjs/react-redux/blob/f892ec00d7e92ff7afb21498276472f0e3b000c5/src/connect/selectorFactory.js#L45

[eltronix]

@samcorcos's issue is still very valid in 2018, I wish you had considered @jimbolla's mapStoreStateToProps. Less cognitive dissonance.

[pandigita]

This is not just a minor issue, but I find that mapStateToPropsis the first real stumbling block for people learning redux - and that's not because of them but because it's a really bad naming decision for something used in react-redux.

I'm teaching my students mapReduxStateToProps(), but that's just one of various solutions. So independent of what the solution is, can we start with all acknowledging that mapStateToProps is letting beginners down?