Skip to content

Commit

Permalink
throw if getState, subscribe, or unsubscribe called while dispatching (
Browse files Browse the repository at this point in the history
…#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
  • Loading branch information
mwilc0x authored and timdorr committed Apr 25, 2017
1 parent 103b614 commit cad6231
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 3 deletions.
24 changes: 24 additions & 0 deletions src/createStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down
35 changes: 34 additions & 1 deletion test/createStore.spec.js
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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(() =>
Expand Down
31 changes: 30 additions & 1 deletion test/helpers/actionCreators.js
Original file line number Diff line number Diff line change
@@ -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 }
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions test/helpers/actionTypes.js
Original file line number Diff line number Diff line change
@@ -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'
39 changes: 38 additions & 1 deletion test/helpers/reducers.js
Original file line number Diff line number Diff line change
@@ -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 = []) {
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit cad6231

Please sign in to comment.