From ee6238c4f31567b308d14bcfcc6abc0b022b5c7f Mon Sep 17 00:00:00 2001 From: "M.K. Safi" Date: Wed, 18 Nov 2015 14:36:06 -0800 Subject: [PATCH 1/3] Add check that reducer returns current state for unknown actions And fix test case --- src/createStore.js | 2 +- src/utils/combineReducers.js | 11 +++++------ test/utils/combineReducers.spec.js | 12 +++++------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/createStore.js b/src/createStore.js index b46ff9554b..86e425fd31 100644 --- a/src/createStore.js +++ b/src/createStore.js @@ -7,7 +7,7 @@ import isPlainObject from './utils/isPlainObject' * Do not reference these action types directly in your code. */ export var ActionTypes = { - INIT: '@@redux/INIT' + INIT: '@@redux/INIT_' + Math.random().toString(36).substring(7).split('').join('.') } /** diff --git a/src/utils/combineReducers.js b/src/utils/combineReducers.js index ea66ebac68..9b6443728e 100644 --- a/src/utils/combineReducers.js +++ b/src/utils/combineReducers.js @@ -65,13 +65,12 @@ function assertReducerSanity(reducers) { ) } - var type = '@@redux/PROBE_UNKNOWN_ACTION_' + Math.random().toString(36).substring(7).split('').join('.') - if (typeof reducer(undefined, { type }) === 'undefined') { + + var randomState = Math.random().toString(36).substring(7).split('').join('.') + if (reducer(randomState, { type: ActionTypes.INIT }) !== randomState) { throw new Error( - `Reducer "${key}" returned undefined when probed with a random type. ` + - `Don't try to handle ${ActionTypes.INIT} or other actions in "redux/*" ` + - `namespace. They are considered private. Instead, you must return the ` + - `current state for any unknown actions, unless it is undefined, ` + + `Reducer "${key}" did not return the current state when probed with a random type. ` + + `You must return the current state for any unknown actions, unless it is undefined, ` + `in which case you must return the initial state, regardless of the ` + `action type. The initial state may not be undefined.` ) diff --git a/test/utils/combineReducers.spec.js b/test/utils/combineReducers.spec.js index 4696c8249b..060ac0ee48 100644 --- a/test/utils/combineReducers.spec.js +++ b/test/utils/combineReducers.spec.js @@ -1,6 +1,6 @@ import expect from 'expect' import { combineReducers } from '../../src' -import createStore, { ActionTypes } from '../../src/createStore' +import createStore from '../../src/createStore' describe('Utils', () => { describe('combineReducers', () => { @@ -151,7 +151,7 @@ describe('Utils', () => { expect(reducer(initialState, { type: 'increment' })).toNotBe(initialState) }) - it('throws an error on first call if a reducer attempts to handle a private action', () => { + it('throws an error if reducer does not return current state for all unknown action types', () => { const reducer = combineReducers({ counter(state, action) { switch (action.type) { @@ -159,16 +159,14 @@ describe('Utils', () => { return state + 1 case 'decrement': return state - 1 - // Never do this in your code: - case ActionTypes.INIT: - return 0 default: - return undefined + return 0 } } }) + expect(() => reducer()).toThrow( - /"counter".*private/ + /"counter".*probed/ ) }) From ae16badc400d355b12bb935ebd5aa481b0efe9b3 Mon Sep 17 00:00:00 2001 From: "M.K. Safi" Date: Fri, 20 Nov 2015 15:37:40 -0800 Subject: [PATCH 2/3] Update combineReducers.js Remove extra line break --- src/utils/combineReducers.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/utils/combineReducers.js b/src/utils/combineReducers.js index 9b6443728e..a3f8e40dd4 100644 --- a/src/utils/combineReducers.js +++ b/src/utils/combineReducers.js @@ -65,7 +65,6 @@ function assertReducerSanity(reducers) { ) } - var randomState = Math.random().toString(36).substring(7).split('').join('.') if (reducer(randomState, { type: ActionTypes.INIT }) !== randomState) { throw new Error( From e240626f0265cb3dfc55a76050499a13fec72d3f Mon Sep 17 00:00:00 2001 From: "M.K. Safi" Date: Sat, 28 Nov 2015 20:22:33 -0800 Subject: [PATCH 3/3] Add test case & improve error message wording * Add a test case to verify reducer has a catch-all clause for unknown action types * Improve error message wording --- src/utils/combineReducers.js | 6 +++--- test/utils/combineReducers.spec.js | 21 ++++++++++++++++++++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/utils/combineReducers.js b/src/utils/combineReducers.js index a3f8e40dd4..b48d4f2da6 100644 --- a/src/utils/combineReducers.js +++ b/src/utils/combineReducers.js @@ -59,9 +59,9 @@ function assertReducerSanity(reducers) { if (typeof initialState === 'undefined') { throw new Error( `Reducer "${key}" returned undefined during initialization. ` + - `If the state passed to the reducer is undefined, you must ` + - `explicitly return the initial state. The initial state may ` + - `not be undefined.` + `Reducers should never return undefined. Make sure this reducer ` + + `has a catch-all clause for unknown action types and that it returns a ` + + `default initial state if the state passed to it is undefined.` ) } diff --git a/test/utils/combineReducers.spec.js b/test/utils/combineReducers.spec.js index 060ac0ee48..bfce2e98fd 100644 --- a/test/utils/combineReducers.spec.js +++ b/test/utils/combineReducers.spec.js @@ -84,6 +84,25 @@ describe('Utils', () => { ) }) + it('throws an error if reducer does not have a catch-all clause for unknown action types', () => { + const reducer = combineReducers({ + counter(state = 0, action) { + switch (action.type) { + case 'increment': + return state + 1 + case 'decrement': + return state - 1 + case undefined: + return state + } + } + }) + + expect(() => reducer()).toThrow( + /"counter".*initialization/ + ) + }) + it('catches error thrown in reducer when initializing and re-throw', () => { const reducer = combineReducers({ throwingReducer() { @@ -151,7 +170,7 @@ describe('Utils', () => { expect(reducer(initialState, { type: 'increment' })).toNotBe(initialState) }) - it('throws an error if reducer does not return current state for all unknown action types', () => { + it('throws an error if reducer does not return current state for unknown action types', () => { const reducer = combineReducers({ counter(state, action) { switch (action.type) {