Merge pull request #259 from gaearon/forbid-handling-private-actions

Handling private actions is an anti-pattern. Enforce it. (Fixes #186)

Author: gaearon

src/Store.js

@@ -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(
@@ -19,7 +26,7 @@ export default class Store {
 
   replaceReducer(nextReducer) {
     this.reducer = nextReducer;
-    this.dispatch({ type: '@@INIT' });
+    this.dispatch({ type: ActionTypes.INIT });
   }
 
   dispatch(action) {

src/utils/combineReducers.js

@@ -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}. ` +
@@ -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);

test/utils/combineReducers.spec.js

@@ -1,5 +1,6 @@
 import expect from 'expect';
 import { combineReducers } from '../../src';
+import { ActionTypes } from '../../src/Store';
 
 describe('Utils', () => {
   describe('combineReducers', () => {
@@ -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;
@@ -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/
       );
     });
   });