throw if getState, subscribe, or unsubscribe called while dispatching (#1569)

* throw error if getState, subscribe, or unsubscribe called while dispatching

* prevent throwing if not subscribed

* update getState error message

* fix space after period

* update subscribe/unsubscribe error messages

Author: mwilc0x

src/createStore.js

@@ -64,6 +64,14 @@ export default function createStore(reducer, preloadedState, enhancer) {
    * @returns {any} The current state tree of your application.
    */
   function getState() {
+    if (isDispatching) {
+      throw new Error(
+        'You may not call store.getState() while the reducer is executing. ' +
+        'The reducer has already received the state as an argument. ' +
+        'Pass it down from the top reducer instead of reading it from the store.'
+      )
+    }
+
     return currentState
   }
 
@@ -95,6 +103,15 @@ export default function createStore(reducer, preloadedState, enhancer) {
       throw new Error('Expected listener to be a function.')
     }
 
+    if (isDispatching) {
+      throw new Error(
+        'You may not call store.subscribe() while the reducer is executing. ' +
+        'If you would like to be notified after the store has been updated, subscribe from a ' +
+        'component and invoke store.getState() in the callback to access the latest state. ' +
+        'See http://redux.js.org/docs/api/Store.html#subscribe for more details.'
+      )
+    }
+
     let isSubscribed = true
 
     ensureCanMutateNextListeners()
@@ -105,6 +122,13 @@ export default function createStore(reducer, preloadedState, enhancer) {
         return
       }
 
+      if (isDispatching) {
+        throw new Error(
+          'You may not unsubscribe from a store listener while the reducer is executing. ' +
+          'See http://redux.js.org/docs/api/Store.html#subscribe for more details.'
+        )
+      }
+
       isSubscribed = false
 
       ensureCanMutateNextListeners()

test/createStore.spec.js

@@ -1,5 +1,13 @@
 import { createStore, combineReducers } from '../src/index'
-import { addTodo, dispatchInMiddle, throwError, unknownAction } from './helpers/actionCreators'
+import { 
+  addTodo, 
+  dispatchInMiddle, 
+  getStateInMiddle,
+  subscribeInMiddle,
+  unsubscribeInMiddle,
+  throwError, 
+  unknownAction 
+} from './helpers/actionCreators'
 import * as reducers from './helpers/reducers'
 import * as Rx from 'rxjs'
 import $$observable from 'symbol-observable'
@@ -461,6 +469,31 @@ describe('createStore', () => {
     ).toThrow(/may not dispatch/)
   })
 
+  it('does not allow getState() from within a reducer', () => {
+    const store = createStore(reducers.getStateInTheMiddleOfReducer)
+
+    expect(() =>
+      store.dispatch(getStateInMiddle(store.getState.bind(store)))
+    ).toThrow(/You may not call store.getState()/)
+  })
+
+  it('does not allow subscribe() from within a reducer', () => {
+    const store = createStore(reducers.subscribeInTheMiddleOfReducer)
+
+    expect(() =>
+      store.dispatch(subscribeInMiddle(store.subscribe.bind(store, () => {})))
+    ).toThrow(/You may not call store.subscribe()/)
+  })
+
+  it('does not allow unsubscribe from subscribe() from within a reducer', () => {
+    const store = createStore(reducers.unsubscribeInTheMiddleOfReducer)
+    const unsubscribe = store.subscribe(() => {})
+
+    expect(() =>
+      store.dispatch(unsubscribeInMiddle(unsubscribe.bind(store)))
+    ).toThrow(/You may not unsubscribe from a store/)
+  })
+
   it('recovers from an error within a reducer', () => {
     const store = createStore(reducers.errorThrowingReducer)
     expect(() =>

test/helpers/actionCreators.js

@@ -1,4 +1,12 @@
-import { ADD_TODO, DISPATCH_IN_MIDDLE, THROW_ERROR, UNKNOWN_ACTION } from './actionTypes'
+import { 
+  ADD_TODO, 
+  DISPATCH_IN_MIDDLE, 
+  GET_STATE_IN_MIDDLE, 
+  SUBSCRIBE_IN_MIDDLE,
+  UNSUBSCRIBE_IN_MIDDLE, 
+  THROW_ERROR, 
+  UNKNOWN_ACTION 
+} from './actionTypes'
 
 export function addTodo(text) {
   return { type: ADD_TODO, text }
@@ -26,6 +34,27 @@ export function dispatchInMiddle(boundDispatchFn) {
   }
 }
 
+export function getStateInMiddle(boundGetStateFn) {
+  return {
+    type: GET_STATE_IN_MIDDLE,
+    boundGetStateFn
+  }
+}
+
+export function subscribeInMiddle(boundSubscribeFn) {
+  return {
+    type: SUBSCRIBE_IN_MIDDLE,
+    boundSubscribeFn
+  }
+}
+
+export function unsubscribeInMiddle(boundUnsubscribeFn) {
+  return {
+    type: UNSUBSCRIBE_IN_MIDDLE,
+    boundUnsubscribeFn
+  }
+}
+
 export function throwError() {
   return {
     type: THROW_ERROR

test/helpers/actionTypes.js

@@ -1,4 +1,7 @@
 export const ADD_TODO = 'ADD_TODO'
 export const DISPATCH_IN_MIDDLE = 'DISPATCH_IN_MIDDLE'
+export const GET_STATE_IN_MIDDLE = 'GET_STATE_IN_MIDDLE'
+export const SUBSCRIBE_IN_MIDDLE = 'SUBSCRIBE_IN_MIDDLE'
+export const UNSUBSCRIBE_IN_MIDDLE = 'UNSUBSCRIBE_IN_MIDDLE'
 export const THROW_ERROR = 'THROW_ERROR'
 export const UNKNOWN_ACTION = 'UNKNOWN_ACTION'

test/helpers/reducers.js

@@ -1,4 +1,11 @@
-import { ADD_TODO, DISPATCH_IN_MIDDLE, THROW_ERROR } from './actionTypes'
+import { 
+  ADD_TODO, 
+  DISPATCH_IN_MIDDLE, 
+  GET_STATE_IN_MIDDLE, 
+  SUBSCRIBE_IN_MIDDLE,
+  UNSUBSCRIBE_IN_MIDDLE,
+  THROW_ERROR 
+} from './actionTypes'
 
 
 function id(state = []) {
@@ -46,6 +53,36 @@ export function dispatchInTheMiddleOfReducer(state = [], action) {
   }
 }
 
+export function getStateInTheMiddleOfReducer(state = [], action) {
+  switch (action.type) {
+    case GET_STATE_IN_MIDDLE:
+      action.boundGetStateFn()
+      return state
+    default:
+      return state
+  }
+}
+
+export function subscribeInTheMiddleOfReducer(state = [], action) {
+  switch (action.type) {
+    case SUBSCRIBE_IN_MIDDLE:
+      action.boundSubscribeFn()
+      return state
+    default:
+      return state
+  }
+}
+
+export function unsubscribeInTheMiddleOfReducer(state = [], action) {
+  switch (action.type) {
+    case UNSUBSCRIBE_IN_MIDDLE:
+      action.boundUnsubscribeFn()
+      return state
+    default:
+      return state
+  }
+}
+
 export function errorThrowingReducer(state = [], action) {
   switch (action.type) {
     case THROW_ERROR: