From d4578783599e72cffd9befa7886c9c19087e66c0 Mon Sep 17 00:00:00 2001 From: Mike Wilcox Date: Tue, 19 Jul 2016 11:36:09 -0400 Subject: [PATCH] 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 --- src/createStore.js | 24 +++++++++++++++++++++ test/createStore.spec.js | 35 +++++++++++++++++++++++++++++- test/helpers/actionCreators.js | 31 ++++++++++++++++++++++++++- test/helpers/actionTypes.js | 3 +++ test/helpers/reducers.js | 39 +++++++++++++++++++++++++++++++++- 5 files changed, 129 insertions(+), 3 deletions(-) diff --git a/src/createStore.js b/src/createStore.js index e81e368ae71..94928acd362 100644 --- a/src/createStore.js +++ b/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() diff --git a/test/createStore.spec.js b/test/createStore.spec.js index 90e60b73b22..3ca7197e274 100644 --- a/test/createStore.spec.js +++ b/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(() => diff --git a/test/helpers/actionCreators.js b/test/helpers/actionCreators.js index 198f61be206..5c26cdcc416 100644 --- a/test/helpers/actionCreators.js +++ b/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 diff --git a/test/helpers/actionTypes.js b/test/helpers/actionTypes.js index 00092962f21..2e6104345c2 100644 --- a/test/helpers/actionTypes.js +++ b/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' diff --git a/test/helpers/reducers.js b/test/helpers/reducers.js index 8e9c7321ec8..31ce7b99e18 100644 --- a/test/helpers/reducers.js +++ b/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: