Add check that reducer returns current state for unknown actions

[msafi]

Hey folks,

First ever open source contribution here and a Redux newb, so I have no idea what I'm doing! Alright, with that out of the way...

I was reading the test cases for combineReducers and noticed that this test doesn't really test what it intends to test because if you change the default case to return a value, the reducer will not throw and the test will fail.

I changed the way Redux inits reducers by making it do an init and a random probe at the same time. This will make it impossible for users to handle private Redux actions, so there's no need for that test anymore. However, I altered the test to make it assert that Redux checks reducers to make sure they return the current state for an unknown action.

[timdorr]

Ah, good call. I think this looks good to me. I'll let Dan chime in if he wants to.

[gaearon]

Hey, thanks for the PR!
I'm a bit busy now with the screencasts.
I'll review when I'm less busy.

Let's hold this off for now. I need to understand this change better. I think DevTools currently actually rely on INIT having a stable name although I'm not sure.

[timdorr]

FWIW, DevTools has a different INIT signature (and in the next branch), so this shouldn't cause problems with it.

[nettofarah]

@msafi is there any particular reason for using Math.random().toString(36).substring(7).split('').join('.') as opposed to a shorter random string?

[msafi]

@nettofarah combineReducers.js already had this code here. That's where I copied it from.

[gaearon]

Can we keep the test that if you forget a default case you'll get this warning? It's an important thing to remember to check.

[msafi]

Okay, I added it here.

[msafi]

I changed the error message here because if the user doesn't have a default case in their code, this is the error that will be thrown.

[timdorr]

@msafi Could you update this against master? We can get the ball rolling again and get this merged in, hopefully 😄

[msafi]

@timdorr Sorry about the delay. I updated the code against master. Please let me know if there's anything else I need to do!

[gaearon]

Sorry for the long wait.

Alas, I just remembered why we can't rely on this. We previously rejected a similar change in #174 (comment) because

If someone has runtime type checking in their reducers, this will cause an error.

Potentially this might have problems for static type checkers as well since we're calling a function with a string state argument whereas they might have annotated their reducer with a specific type.

This is how #174 got superseded by #193 which is roughly what you see now.

[msafi]

Ah, good catch!

So I guess there's no way to have a runtime check that a reducer returns given state for unknown action type.

Alright, but there's another issue that this PR tried to fix, which is that there's still a way for the user to handle ActionTypes.INIT and this check would not catch it. Like, if we change the test like this, the user can bypass the check.

I'll see if I can send another PR that only addresses this second issue.