Skip to content

passing an invalid reducer to combineReducers() silently fails #1398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nabn opened this issue Feb 14, 2016 · 7 comments
Closed

passing an invalid reducer to combineReducers() silently fails #1398

nabn opened this issue Feb 14, 2016 · 7 comments

Comments

@nabn
Copy link

nabn commented Feb 14, 2016

If I pass something to combineReducers() that doesn't really return a valid reducer, it silently fails. But if it's the only reducer passed to combineReducers(), it throws an error saying valid reducers were not passed. I'm not sure if it's a bug, but sure does look like one to me.

Isn't it better if it warned me if one of the arguments passed to combineReducers() wasn't a valid reducer?

I ran into this issue when I forgot to do export default myReducer in one of my reducer files.

@nabn nabn changed the title passing a improper reducer to combineReducers() silently fails passing a invalid reducer to combineReducers() silently fails Feb 14, 2016
@aakashsigdel
Copy link

yes, I think the combineReducer just checks if the object's provided value is a function or not, and doesn't warn if the the function passed is undefined.

if (typeof reducers[key] === 'function')  {
  finalReducers[key] = reducers[key];
}

When pushing the value to finalReducer array, we could make it throw an error or give a warning if the value of the reducer passed is not a function

we could have something like

if (typeof reducers[key] === 'function')  {
  finalReducers[key] = reducers[key];
} else {
  throw 'The reducer' + reducers[key] + 'is not a valid type of reducer'
}

@gaearon
Copy link
Contributor

gaearon commented Feb 14, 2016

The reason we have this check is that it used to be a popular pattern to import * from './reducers' which will give you an object with some potentially unrelated fields, e.g. __esModule: true.

Maybe we should give up on this behavior since we don’t encourage import * for reducers anymore but that would be a breaking change.

@pke
Copy link

pke commented Mar 7, 2016

Also I might add that a reducer that throws also does so silently. You only see that its supposed to be reduced state is missing in the state.

I had a wrong import for action types and my switch statement then basically said:

switch (action.type) {
  case undefined.ACTION_NAME
   ....
}

A sanityError is saved in combineReducers, though its message does not contain which reducer was insane. That happens because the call to the reducer inside assertReducerSanity is not wrapped in try...catch

I should be able to make a PR fixing the error reporting.
Edit: Actually, https://github.com/reactjs/redux/blob/master/test/combineReducers.spec.js#L87 should already cover this. But in my app the exception is never shown in the console. Intentional?

@hoangnm
Copy link

hoangnm commented Jun 16, 2017

Is there any news for this ? I see that it's silently failed because in createStore.js (line 170), we just skip the error. Can we catch the error and log to the console, it helps easier to debug when something wrong...

@Huanzhang89
Copy link

Agree with @gaearon on this one but maybe in the next major release since this will be a breaking change for a lot of production apps using import * for reducers?

@timdorr
Copy link
Member

timdorr commented Nov 6, 2017

@Huanzhang89 Until there's a stable workaround for that case, we're not able to implement it in the library. That should continue to work, as it's a perfectly valid use case and should be expected to work.

@nabn nabn changed the title passing a invalid reducer to combineReducers() silently fails passing an invalid reducer to combineReducers() silently fails Nov 6, 2017
@Huanzhang89
Copy link

@timdorr Okay, thanks for the clarification Tim

@nabn nabn closed this as completed Jul 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants