Support Additional arguments for handleAction(s) #161
Conversation
src/__tests__/handleAction-test.js
Outdated
| increment, (state, { payload }, extraArg) => ({ | ||
| counter: state.counter + payload + extraArg | ||
| }), | ||
| { counter: 3 }); |
There was a problem hiding this comment.
Can we put the closing parenthesis on a newline, and use the same number of arguments in handleActions-test.js?
src/__tests__/handleActions-test.js
Outdated
| }); | ||
|
|
||
| const counterBase = 5; | ||
| const counterFix = 1; |
There was a problem hiding this comment.
Can we use extraArg1 and extraArg2 instead, to mirror the parameter names you used above?
src/__tests__/handleActions-test.js
Outdated
| @@ -147,4 +147,21 @@ describe('handleActions', () => { | |||
| counter: 7 | |||
| }); | |||
| }); | |||
|
|
|||
| it('accepts additional arguments', () => { | |||
| // meaningless base and fix just for testing additional arguments | |||
There was a problem hiding this comment.
Is this comment necessary?
src/handleActions.js
Outdated
|
|
||
| export default function handleActions(handlers, defaultState) { | ||
| const reducers = ownKeys(handlers).map(type => handleAction(type, handlers[type])); | ||
| const reducer = reduceReducers(...reducers); | ||
| const reducer = (state, action, ...args) => reducers.reduce( | ||
| (p, r) => r(p, action, ...args), state); |
There was a problem hiding this comment.
I don't like these one-letter variable names.
Can we please use:
reducers.reduce((previousState, reducer) => reducer(previousState, action, ...args), state)Also, at this point, we should remove reducer-reducers from package.json, since we're no longer using it.
And, could you submit a pull request here for passing the extra arguments?
src/__tests__/handleAction-test.js
Outdated
| }), | ||
| { counter: 3 }); | ||
|
|
||
| const counterBase = 10; |
There was a problem hiding this comment.
Let's call this extraArg; counterBase doesn't really signify anything.
* remove reduceReducers from dependency
|
Can we update the README to indicate that extra arguments are passed? Thanks for the contribution. I'd like to wait over the weekend to see if @timche has any thoughts, and then merge. |
|
Sorry, we just merged |
Conflicts: README.md package.json src/handleAction.js src/handleActions.js
|
@timche will take a look tomorrow. |
|
Before we add this feature, I'd like to discuss this a bit deeper: #148 |
|
I think this change is good. I realize it violates reducer signature if the users use the additional arguments, but from what I can gather, this is a great way of passing global state as a read only to a slice reducer that can write to it's own slice. The pattern seems fine and the change seems harmless, since it would change literally anything, it'll just be less opinionated on how you use yours reducers which are, as we know, +1 for the change |
|
Closing because it's already open for very long time. Don't see it being merged. If you still think a necessity, feel free to comment. |
resolve #148