add state to mapDispatchToProps

[tcoopman]

This PR is for discussion.
Recently I have found the need to have the state in mapDispatchToProps.

I have an actionCreator like this:

const addFeature = (some, state) => {
  // do something with state
}

const someActionCreator = (myFunction) => (options) => {
  //
};

And I want to write

const selectActions = (dispatch, state) => ({
  someActionCreator: bindActionCreator(actionCreator(addFeature(state.data1, state.data2), dispatch)
});
export default connect(selectState, selectActions)(MyComponent);

I know there are other options to inject addFeature(state.data1, state.data2) to someActionCreator but I would like to do it without MyComponent knowing about it and this looks like a nice solution.

Any reason why this is a bad idea? Is there a better way?

[tcoopman]

I've been thinking about this some more and I'm wondering if the API of connect can't be simplified.

If we pass state, dispatch and ownProps into mapToProps then you don't need 3 separate methods for connect like there are now. The new API would be connect(mapToProps(state, dispatch, ownProps))

The disadvantage is that it's a bit more work for the user then at the moment but it should be possible to expose the current API on top of this new one.

Thoughts?

[gaearon]

If we pass state, dispatch and ownProps into mapToProps then you don't need 3 separate methods for connect like there are now. The new API would be connect(mapToProps(state, dispatch, ownProps))

This was discussed in #1. The only reason current API isn't that simple is because it essentially makes worst perf case ("rebind everything on every prop change") the default.

[tcoopman]

So what's your opinion about this merge request.
Is this something that can be considered as a low level API with the current api on top?

Maybe my usecase for having access to the state in the actionCreators is not valid, but it looks reasonable to bind some state to the actoinCreators so that your components don't have to know about it.

[gaearon]

I'm inclined not to merge this. This is a very bad case perf-wise (re-binding on every state change) that we won't ever encourage. It also makes API asymmetric and harder to remember.

If you want to do this, you can always introduce an intermediate component and call bindActionCreators from it in render() method, as it will have comparable performance.