Skip to content

Commit

Permalink
Handling private actions is an anti-pattern. Enforce it. (Fixes #186)
Browse files Browse the repository at this point in the history
  • Loading branch information
gaearon committed Jul 13, 2015
1 parent afd4260 commit 9045a0c
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 29 deletions.
9 changes: 8 additions & 1 deletion src/Store.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import invariant from 'invariant';
import isPlainObject from './utils/isPlainObject';

// Don't ever try to handle these action types in your code. They are private.
// For any unknown actions, you must return the current state.
// If the current state is undefined, you must return the initial state.
export const ActionTypes = {
INIT: '@@redux/INIT'
};

export default class Store {
constructor(reducer, initialState) {
invariant(
Expand All @@ -19,7 +26,7 @@ export default class Store {

replaceReducer(nextReducer) {
this.reducer = nextReducer;
this.dispatch({ type: '@@INIT' });
this.dispatch({ type: ActionTypes.INIT });
}

dispatch(action) {
Expand Down
32 changes: 23 additions & 9 deletions src/utils/combineReducers.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,11 @@
import mapValues from '../utils/mapValues';
import pick from '../utils/pick';
import invariant from 'invariant';
import { ActionTypes } from '../Store';

function getErrorMessage(key, action) {
const actionType = action && action.type;
const actionName = actionType && `"${actionType}"` || 'an action';
const reducerName = `Reducer "${key}"`;

if (actionType === '@@INIT') {
return (
`${reducerName} returned undefined during initialization. ` +
`If the state passed to the reducer is undefined, ` +
`you must explicitly return the initial state.`
);
}

return (
`Reducer "${key}" returned undefined handling ${actionName}. ` +
Expand All @@ -24,6 +16,28 @@ function getErrorMessage(key, action) {
export default function combineReducers(reducers) {
const finalReducers = pick(reducers, (val) => typeof val === 'function');

Object.keys(finalReducers).forEach(key => {
const reducer = finalReducers[key];
invariant(
typeof reducer(undefined, { type: ActionTypes.INIT }) !== 'undefined',
`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.`
);

const type = Math.random().toString(36).substring(7).split('').join('.');
invariant(
typeof reducer(undefined, { type }) !== 'undefined',
`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, ` +
`in which case you must return the initial state, regardless of the ` +
`action type. The initial state may not be undefined.`
);
});

return function composition(state = {}, action) {
return mapValues(finalReducers, (reducer, key) => {
const newState = reducer(state[key], action);
Expand Down
56 changes: 37 additions & 19 deletions test/utils/combineReducers.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import expect from 'expect';
import { combineReducers } from '../../src';
import { ActionTypes } from '../../src/Store';

describe('Utils', () => {
describe('combineReducers', () => {
Expand Down Expand Up @@ -30,43 +31,44 @@ describe('Utils', () => {
).toEqual(['stack']);
});

it('should throw an error if a reducer returns undefined', () => {
it('should throw an error if a reducer returns undefined handling an action', () => {
const reducer = combineReducers({
undefinedByDefault(state = 0, action) {
counter(state = 0, action) {
switch (action && action.type) {
case 'increment':
return state + 1;
case 'decrement':
return state - 1;
case '@@INIT':
return state;
default:
case 'whatever':
case null:
case undefined:
return undefined;
default:
return state;
}
}
});

const initialState = reducer(undefined, { type: '@@INIT' });
expect(
() => reducer(initialState, { type: 'whatever' })
() => reducer({ counter: 0 }, { type: 'whatever' })
).toThrow(
/"undefinedByDefault".*"whatever"/
/"counter".*"whatever"/
);
expect(
() => reducer(initialState, null)
() => reducer({ counter: 0 }, null)
).toThrow(
/"undefinedByDefault".*an action/
/"counter".*an action/
);
expect(
() => reducer(initialState, {})
() => reducer({ counter: 0 }, {})
).toThrow(
/"undefinedByDefault".*an action/
/"counter".*an action/
);
});

it('should throw an error if a reducer returns undefined initializing', () => {
const reducer = combineReducers({
undefinedInitially(state, action) {
expect(() => combineReducers({
counter(state, action) {
switch (action.type) {
case 'increment':
return state + 1;
Expand All @@ -76,12 +78,28 @@ describe('Utils', () => {
return state;
}
}
});
})).toThrow(
/"counter".*initialization/
);
});

expect(
() => reducer(undefined, { type: '@@INIT' })
).toThrow(
/"undefinedInitially".*initialization/
it('should throw an error if a reducer attempts to handle a private action', () => {
expect(() => combineReducers({
counter(state, action) {
switch (action.type) {
case 'increment':
return state + 1;
case 'decrement':
return state - 1;
// Never do this in your code:
case ActionTypes.INIT:
return 0;
default:
return undefined;
}
}
})).toThrow(
/"counter".*private/
);
});
});
Expand Down

0 comments on commit 9045a0c

Please sign in to comment.