Throw error on undefined value from reducer function #197
Conversation
|
@gaearon Is this good to merge? |
|
Yeah, can you please rebase so it merges cleanly? |
taylorhakes
force-pushed
the
compose-reducer-undefined
branch
from
673f84c to
01c53ef
Compare
|
Just did the rebase. The tests pass locally, but are failing in Travis. Have you seen this before? |
|
Something to do with linting. I'll merge soon, thanks. |
|
Sorry for only now bringing this up, but I believe the check should go inside the Store class rather than |
|
^ In fact the new invariant should go just a few lines below the highlighted portion |
But then we'll check the root value (which is unlikely to be undefined if you use |
|
I think we're actually doing the right thing here. First, we want to check the actual app's reducers—not the root one. Second, we don't want this to be a hard limitation—just something the default utilities help you with. You're free to use your own |
|
@gaearon Ah I see. It should go in both places, then, right? |
|
Okay I can agree with that. That makes sense. |
|
Why do that though? Redux doesn't really care what your root value is like. I'd say it should only be enforced at |
|
Yep, agreed. Carry on :) |
taylorhakes
force-pushed
the
compose-reducer-undefined
branch
from
01c53ef to
283c439
Compare
|
Updated code to use invariant |
|
Can you please rebase on master again? I think the linting problem is fixed there by your earlier PR. |
| @@ -29,5 +29,28 @@ describe('Utils', () => { | |||
| Object.keys(reducer({}, { type: 'push' })) | |||
| ).toEqual(['stack']); | |||
| }); | |||
| it('should throw an error if undefined return from reducer', () => { | |||
|
I am a little confused. This change was made on top of breaking-changes-1.0. Do you want want me to merge master into breaking-changes-1.0? This change can't be rebased on master because it has to do with reducers. |
taylorhakes
force-pushed
the
compose-reducer-undefined
branch
from
283c439 to
b8e7e1e
Compare
|
Oh, I didn't mean on master, sorry :-( |
|
All right, this is in. |
Accidentally created against alpha before. Sorry about that.
Updated composeReducers to throw error when reducer returns undefined. Update to #193. #191