Skip to content

Commit

Permalink
add verifyStateShape function and tests
Browse files Browse the repository at this point in the history
  • Loading branch information
ellbee committed Jul 30, 2015
1 parent 0fc5802 commit fca60c8
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 5 deletions.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
54 changes: 51 additions & 3 deletions src/utils/combineReducers.js
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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
Expand All @@ -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');

Expand All @@ -54,14 +92,24 @@ 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',
getErrorMessage(key, action)
);
return newState;
});

if (process.env.NODE_ENV !== 'production' && !stateShapeVerified) {
verifyStateShape(state, finalState);
stateShapeVerified = true;
}

return finalState;
};
}
71 changes: 70 additions & 1 deletion test/utils/combineReducers.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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();
});
});
});

0 comments on commit fca60c8

Please sign in to comment.