From fca60c8f5c4e9aef06e5460e68e44fe61c506c0b Mon Sep 17 00:00:00 2001 From: Lee Bannard Date: Wed, 22 Jul 2015 22:45:30 +0100 Subject: [PATCH] add verifyStateShape function and tests --- package.json | 3 +- src/utils/combineReducers.js | 54 +++++++++++++++++++++-- test/utils/combineReducers.spec.js | 71 +++++++++++++++++++++++++++++- 3 files changed, 123 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index c9c8325172..10e77d595d 100644 --- a/package.json +++ b/package.json @@ -58,7 +58,8 @@ "webpack-dev-server": "^1.8.2" }, "dependencies": { - "invariant": "^2.0.0" + "invariant": "^2.0.0", + "warning": "^2.0.0" }, "npmName": "redux", "npmFileMap": [ diff --git a/src/utils/combineReducers.js b/src/utils/combineReducers.js index e59e4d0976..8ba298991a 100644 --- a/src/utils/combineReducers.js +++ b/src/utils/combineReducers.js @@ -1,7 +1,9 @@ +import { ActionTypes } from '../createStore'; +import isPlainObject from '../utils/isPlainObject'; import mapValues from '../utils/mapValues'; import pick from '../utils/pick'; import invariant from 'invariant'; -import { ActionTypes } from '../createStore'; +import warning from 'warning'; function getErrorMessage(key, action) { var actionType = action && action.type; @@ -13,6 +15,41 @@ function getErrorMessage(key, action) { ); } +function verifyStateShape(initialState, currentState) { + var reducerKeys = Object.keys(currentState); + + if (reducerKeys.length === 0) { + warning( + false, + 'Store does not have a valid reducer. Make sure the argument passed ' + + 'to combineReducers is an object whose values are reducers.' + ); + return; + } + + if (!isPlainObject(initialState)) { + warning( + false, + 'initialState has unexpected type of "' + + ({}).toString.call(initialState).match(/\s([a-z|A-Z]+)/)[1] + + '". Expected initialState to be an object with the following ' + + `keys: "${reducerKeys.join('", "')}"` + ); + return; + } + + var unexpectedKeys = Object.keys(initialState).filter( + key => reducerKeys.indexOf(key) < 0 + ); + + warning( + unexpectedKeys.length === 0, + `Unexpected ${unexpectedKeys.length > 1 ? 'keys' : 'key'} ` + + `"${unexpectedKeys.join('", "')}" in initialState will be ignored. ` + + `Expected to find one of the known reducer keys instead: "${reducerKeys.join('", "')}"` + ); +} + /** * Turns an object whose values are different reducer functions, into a single * reducer function. It will call every child reducer, and gather their results @@ -29,6 +66,7 @@ function getErrorMessage(key, action) { * @returns {Function} A reducer function that invokes every reducer inside the * passed object, and builds a state object with the same shape. */ + export default function combineReducers(reducers) { var finalReducers = pick(reducers, (val) => typeof val === 'function'); @@ -54,8 +92,11 @@ export default function combineReducers(reducers) { ); }); - return function combination(state = {}, action) { - return mapValues(finalReducers, (reducer, key) => { + var defaultState = mapValues(finalReducers, () => undefined); + var stateShapeVerified; + + return function combination(state = defaultState, action) { + var finalState = mapValues(finalReducers, (reducer, key) => { var newState = reducer(state[key], action); invariant( typeof newState !== 'undefined', @@ -63,5 +104,12 @@ export default function combineReducers(reducers) { ); return newState; }); + + if (process.env.NODE_ENV !== 'production' && !stateShapeVerified) { + verifyStateShape(state, finalState); + stateShapeVerified = true; + } + + return finalState; }; } diff --git a/test/utils/combineReducers.spec.js b/test/utils/combineReducers.spec.js index dba2069dcf..d6ae96cba4 100644 --- a/test/utils/combineReducers.spec.js +++ b/test/utils/combineReducers.spec.js @@ -97,7 +97,7 @@ describe('Utils', () => { } }); - expect(reducer(0, { type: increment }).counter).toEqual(1); + expect(reducer({counter: 0}, { type: increment }).counter).toEqual(1); }); it('should throw an error if a reducer attempts to handle a private action', () => { @@ -119,5 +119,74 @@ describe('Utils', () => { /"counter".*private/ ); }); + + it('should warn if no reducers are passed to combineReducers', () => { + const spy = expect.spyOn(console, 'error'); + const reducer = combineReducers({}); + reducer({}); + expect(spy.calls[0].arguments[0]).toMatch( + /Store does not have a valid reducer/ + ); + spy.restore(); + }); + + it('should warn if initial state object does not match state object returned by reducer', () => { + const spy = expect.spyOn(console, 'error'); + const reducerCreator = () => { + return combineReducers({ + foo(state = {bar: 1}) { + return state; + }, + baz(state = {qux: 3}) { + return state; + } + }); + }; + + reducerCreator()({foo: {bar: 2}}); + expect(spy.calls.length).toBe(0); + + reducerCreator()({ + foo: {bar: 2}, + baz: {qux: 4} + }); + expect(spy.calls.length).toBe(0); + + reducerCreator()({bar: 2}); + expect(spy.calls[0].arguments[0]).toMatch( + /Unexpected key "bar".*instead: "foo", "baz"/ + ); + + reducerCreator()({bar: 2, qux: 4}); + expect(spy.calls[1].arguments[0]).toMatch( + /Unexpected keys "bar", "qux".*instead: "foo", "baz"/ + ); + + reducerCreator()(1); + expect(spy.calls[2].arguments[0]).toMatch( + /unexpected type of "Number".*keys: "foo", "baz"/ + ); + + spy.restore(); + }); + + it('should only check state shape on init', () => { + const spy = expect.spyOn(console, 'error'); + const reducer = combineReducers({ + foo(state = {bar: 1}) { + return state; + } + }); + + reducer({bar: 1}); + expect(spy.calls[0].arguments[0]).toMatch( + /Unexpected key "bar".*instead: "foo"/ + ); + + reducer({bar: 1}); + expect(spy.calls.length).toBe(1); + + spy.restore(); + }); }); });