Skip to content
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

Always call verifyStateShape for combined reducers #720

Merged
merged 1 commit into from
Sep 14, 2015

Conversation

ellbee
Copy link
Contributor

@ellbee ellbee commented Sep 13, 2015

As discussed in #715

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2015

It's still saying “initial state” in the error message.

@ellbee
Copy link
Contributor Author

ellbee commented Sep 13, 2015

Ah, sorry. I thought you meant it could be changed because the message already made sense. Of course, it does not. I'll update.

@ellbee ellbee force-pushed the always-call-verifyStateShape branch 2 times, most recently from 48ea9cd to 5853b58 Compare September 13, 2015 19:40
@ellbee
Copy link
Contributor Author

ellbee commented Sep 13, 2015

I think it is clearer if the messages are slightly different depending on whether the warning has been triggered via the INIT action in createStore or by calling the the reducer directly. Here is how I propose the messages read for these two cases:

Passing mismatching initialState to createStore (existing case)

The initialState argument passed to createStore has unexpected type of "Number". Expected argument to be an object with the following keys: "foo", "baz"

Unexpected keys "fdfds", "dsfsdf" in initialState argument passed to createStore will be ignored. Expected to find one of the known reducer keys instead: "todos", "todosReverse",
"dispatchInTheMiddleOfReducer", "errorThrowingReducer"

Passing mismatching state to reducer

Unexpected key "bar" in state argument passed to reducer will be ignored. Expected to find one of the known reducer keys instead: "foo", "baz"

Unexpected keys "bar", "qux" in state argument passed to reducer will be ignored. Expected to find one of the known reducer keys instead: "foo", "baz"

The state argument passed to reducer has unexpected type of "Number". Expected argument to be an object with the following keys: "foo", "baz"

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2015

in state argument passed to reducer will be ignored

This might be a bit misleading because most likely the user didn't pass the argument to reducer themselves. They just returned it the last time. Any idea how to change phrasing to suggest this?

@ellbee
Copy link
Contributor Author

ellbee commented Sep 13, 2015

Yeah, I think you are right.

As reducers are shown in the docs to have signature of (previousState, action) => newState, how about something like this?

The previous state received by the reducer has unexpected type of "Number". Expected argument to be an object with the following keys: "foo", "baz"

Unexpected key "bar" in previous state received by the reducer will be ignored. Expected to find one of the known reducer keys instead: "foo", "baz"

I think this is better, but the first sentence of the second warning is starting to read a bit awkwardly.

@ellbee
Copy link
Contributor Author

ellbee commented Sep 13, 2015

Slight variations:

Unexpected keys "bar", "qux" found in previous state received by the reducer. Expected to find one of the known reducer keys instead: "foo", "baz". Unexpected keys will be ignored.

Unexpected keys "bar", "qux" found in reducer's previous state. Expected to find one of the known reducer keys instead: "foo", "baz". Unexpected keys will be ignored.

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2015

I like this:

Unexpected keys "bar", "qux" found in previous state received by the reducer. Expected to find one of the known reducer keys instead: "foo", "baz". Unexpected keys will be ignored.

@ellbee ellbee force-pushed the always-call-verifyStateShape branch from 5853b58 to 6fff5e3 Compare September 13, 2015 22:31
@ellbee
Copy link
Contributor Author

ellbee commented Sep 13, 2015

PR updated with the preferred wording

gaearon added a commit that referenced this pull request Sep 14, 2015
Always call verifyStateShape for combined reducers
@gaearon gaearon merged commit c120054 into reduxjs:master Sep 14, 2015
@gaearon
Copy link
Contributor

gaearon commented Sep 14, 2015

Great job! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants