search

erikras / multireducer

A utility to wrap many copies of a single Redux reducer into a single key-based reducer.
MIT License
422 stars 23 forks source link

add ability to use component props in connectMultireducer #81

Closed jsdmc closed 8 years ago

jsdmc commented 8 years ago

Aloha!

It would be good to have the same api for * connectMultireducer* as react-redux connect has. https://github.com/rackt/react-redux/blob/master/docs/api.md#connectmapstatetoprops-mapdispatchtoprops-mergeprops-options

In my usecase I want to use ownProps of component to connect to some kind of key-value structure in state where key - is a property of component connected using multireducer.

For such usecase fix is pretty simple. But to support the same api as original react-redux connect - it might take a little bit more time.

What's your opinion about that feature?

Cheers.

adailey14 commented 8 years ago

@jsdmc FYI I created a fork of this project that has the same interface as react-redux connect, as well as some other changes. It is here: https://github.com/adailey14/multireducer

jsdmc commented 8 years ago

@adailey14 Let's add some tests, and help @erikras to make this lib better)

jsdmc commented 8 years ago

Created pull request https://github.com/erikras/multireducer/pull/82

adailey14 commented 8 years ago

@jsdmc ok sounds good.

What do you think of some of the other semantics I introduced in my fork? If you like them I can work them into a PR for this lib and make them backwards compatible (so the current API would still work):

Instead of: connectMultireducer(mapStateToProps(preSlicedState), actionCreatorHash),

It provides: connectMultireducer(mapStateToProps(topLevelState, multireducerKey), mapDispatchToProps(dispatch, multireducerKey))

And it provides multireducerBindActionCreators(actionCreators, multireducerKey, dispatch) to use within mapDispatchToProps.

This patches the dispatch function to catch any 'thunk' action creators so that actions they dispatch will have the multireducerKey attached. This allows 'thunk' middleware to function as expected. A limitation here is you cannot dispatch actions to any other part of the reducer tree from within these thunks, since the multireducerKey would be attached to those actions. Potentially we could pass in 2 dispatch functions in such cases, a 'global' dispatch and a 'multireducer' dispatch to allow dispatching both types of actions.

I also switched the way multireducerKey is attached. Instead of wrapping the actionCreator so it returns a wrapper object with a key and the action, instead I append the key directly to the action's type attribute. This allows the action to be passed through middleware and have the structure middleware is expecting. The appending string to type part was also useful for me for debugging in redux devtools, but maybe was a bad idea for the flexibility of the library. Maybe it should be added as a property to the action. This does limit you to using objects as actions though, rather than simpler data structures.

jsdmc commented 8 years ago

@adailey14 I played around with your fork. All looks good, but it doesn't have the same api as origin connect. Your version of connectMultireducer doesn't allow to access to ownProps of component. Supporting thunk actions is a big plus. I'd like to use this functionality also!

@erikras What's your opinion about redux-thunk support?

yesmeck commented 8 years ago

@erikras Is there any chance of merging @adailey14 's effort?