Skip to content

Add opt-out - with deprecation warnings - to the bans on getState, subscription Handling and dispatch in the reducers #3444

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/api/createStore.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ There should only be a single store in your app.

3. [`enhancer`] _(Function)_: The store enhancer. You may optionally specify it to enhance the store with third-party capabilities such as middleware, time travel, persistence, etc. The only store enhancer that ships with Redux is [`applyMiddleware()`](./applyMiddleware.md).

4. ['options'] _(object)_: Optional object with further configuration of redux. Currently allows for opt out of the ban on getState, dispatch and subscriptionhandling in the reducer via the boolean parameters `rules.allowDispatch`, `rules.allowGetState` and `rules.allowSubscriptionHandling`. Keep in mind though that this ban is there for a reason, and this opt-out is meant for compatibility with legacy-code. Using these functions in the reducer is an antipattern that makes the reducer impure, and support for this might be removed in the future.

#### Returns

([_`Store`_](Store.md)): An object that holds the complete state of your app. The only way to change its state is by [dispatching actions](Store.md#dispatch). You may also [subscribe](Store.md#subscribe) to the changes to its state to update the UI.
Expand Down
26 changes: 24 additions & 2 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,17 @@ export interface Store<S = any, A extends Action = AnyAction> {
export type DeepPartial<T> = {
[K in keyof T]?: T[K] extends object ? DeepPartial<T[K]> : T[K]
}

type ReduxOptions = {
rules?: {
/** @deprecated Allow the dispatch of actions in the reducer. Antipattern, this opt-out is for legacy-reasons */
allowDispatch?: boolean | undefined;
/** @deprecated Allow the usage of getState in the reducer. Antipattern, this opt-out is for legacy-reasons */
allowGetState?: boolean | undefined;
/** @deprecated Allow the usage of subscribe and unsibscribe in the reducer. Antipattern, this opt-out is for legacy-reasons */
allowSubscriptionHandling?: boolean | undefined;
} | undefined
}

/**
* A store creator is a function that creates a Redux store. Like with
Expand All @@ -266,12 +277,14 @@ export type DeepPartial<T> = {
export interface StoreCreator {
<S, A extends Action, Ext, StateExt>(
reducer: Reducer<S, A>,
enhancer?: StoreEnhancer<Ext, StateExt>
enhancer?: StoreEnhancer<Ext, StateExt>,
options?: ReduxOptions,
): Store<S & StateExt, A> & Ext
<S, A extends Action, Ext, StateExt>(
reducer: Reducer<S, A>,
preloadedState?: DeepPartial<S>,
enhancer?: StoreEnhancer<Ext>
enhancer?: StoreEnhancer<Ext>,
options?: ReduxOptions,
): Store<S & StateExt, A> & Ext
}

Expand Down Expand Up @@ -300,6 +313,15 @@ export interface StoreCreator {
* travel, persistence, etc. The only store enhancer that ships with Redux
* is `applyMiddleware()`.
*
* @param {Object} [options] Optional object with further configuration of redux.
* Currently allows for opt out of the ban on getState, dispatch and
* subscriptionhandling in the reducer via the boolean parameters
* `rules.allowDispatch`, `rules.allowGetState` and `rules.allowSubscriptionHandling`.
* Keep in mind though that this ban is there for a reason,
* and this opt-out is meant for legacy-reasons. Using these functions in the reducer
* is an antipattern that makes the reducer impure, and support for this
* might be removed in the future.
*
* @returns A Redux store that lets you read the state, dispatch actions and
* subscribe to changes.
*/
Expand Down
60 changes: 40 additions & 20 deletions src/createStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import $$observable from 'symbol-observable'
import ActionTypes from './utils/actionTypes'
import isPlainObject from './utils/isPlainObject'

let defaultOptions = { rules: {} }

/**
* Creates a Redux store that holds the state tree.
* The only way to change the data in the store is to call `dispatch()` on it.
Expand All @@ -25,38 +27,56 @@ import isPlainObject from './utils/isPlainObject'
* time travel, persistence, etc. The only store enhancer that ships with Redux
* is `applyMiddleware()`.
*
* @param {Object} [options] Optional object with further configuration of redux.
* Currently allows for opt out of the ban on getState, dispatch and
* subscriptionhandling in the reducer via the boolean parameters
* `rules.allowDispatch`, `rules.allowGetState` and `rules.allowSubscriptionHandling`.
* Keep in mind though that this ban is there for a reason,
* and this opt-out is meant for compatibility with legacy-code. Using these functions in the reducer
* is an antipattern that makes the reducer impure, and support for this
* might be removed in the future.
*
* @returns {Store} A Redux store that lets you read the state, dispatch actions
* and subscribe to changes.
*/
export default function createStore(reducer, preloadedState, enhancer) {
export default function createStore(reducer, preloadedState, enhancer, options) {
if (
(typeof preloadedState === 'function' && typeof enhancer === 'function') ||
(typeof enhancer === 'function' && typeof arguments[3] === 'function')
(typeof enhancer === 'function' && typeof options === 'function')
) {
throw new Error(
'It looks like you are passing several store enhancers to ' +
'createStore(). This is not supported. Instead, compose them ' +
'together to a single function.'
'createStore(). This is not supported. Instead, compose them ' +
'together to a single function.'
)
}

if (typeof preloadedState === 'function' && typeof enhancer === 'undefined') {
enhancer = preloadedState
preloadedState = undefined
if (typeof preloadedState === 'function') {
return createStore(reducer, undefined, preloadedState, enhancer)
}

if (typeof enhancer !== 'undefined') {
if (typeof enhancer !== 'function') {
throw new Error('Expected the enhancer to be a function.')
}

return enhancer(createStore)(reducer, preloadedState)
return enhancer(createStore)(reducer, preloadedState, undefined, options)
}

if (typeof reducer !== 'function') {
throw new Error('Expected the reducer to be a function.')
}

options = { ...defaultOptions, ...options }

let banDispatch = !options.rules.allowDispatch
let banGetState = !options.rules.allowGetState
let banSubscriptionHandling = !options.rules.allowSubscriptionHandling

if (!(banDispatch && banGetState && banSubscriptionHandling)) {
console.warn('The usage of dispatch, getState or subscriber-logic in the reducer is an antipattern and support might be removed in the future.')
}

let currentReducer = reducer
let currentState = preloadedState
let currentListeners = []
Expand All @@ -82,11 +102,11 @@ export default function createStore(reducer, preloadedState, enhancer) {
* @returns {any} The current state tree of your application.
*/
function getState() {
if (isDispatching) {
if (banGetState && 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.'
'The reducer has already received the state as an argument. ' +
'Pass it down from the top reducer instead of reading it from the store.'
)
}

Expand Down Expand Up @@ -121,12 +141,12 @@ export default function createStore(reducer, preloadedState, enhancer) {
throw new Error('Expected the listener to be a function.')
}

if (isDispatching) {
if (banSubscriptionHandling && 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 https://redux.js.org/api-reference/store#subscribe(listener) for more details.'
'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 https://redux.js.org/api-reference/store#subscribe(listener) for more details.'
)
}

Expand All @@ -140,10 +160,10 @@ export default function createStore(reducer, preloadedState, enhancer) {
return
}

if (isDispatching) {
if (banSubscriptionHandling && isDispatching) {
throw new Error(
'You may not unsubscribe from a store listener while the reducer is executing. ' +
'See https://redux.js.org/api-reference/store#subscribe(listener) for more details.'
'See https://redux.js.org/api-reference/store#subscribe(listener) for more details.'
)
}

Expand Down Expand Up @@ -184,18 +204,18 @@ export default function createStore(reducer, preloadedState, enhancer) {
if (!isPlainObject(action)) {
throw new Error(
'Actions must be plain objects. ' +
'Use custom middleware for async actions.'
'Use custom middleware for async actions.'
)
}

if (typeof action.type === 'undefined') {
throw new Error(
'Actions may not have an undefined "type" property. ' +
'Have you misspelled a constant?'
'Have you misspelled a constant?'
)
}

if (isDispatching) {
if (banDispatch && isDispatching) {
throw new Error('Reducers may not dispatch actions.')
}

Expand Down
40 changes: 38 additions & 2 deletions test/createStore.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
unknownAction
} from './helpers/actionCreators'
import * as reducers from './helpers/reducers'
import { identity } from './utils/others'
import { from } from 'rxjs'
import { map } from 'rxjs/operators'
import $$observable from 'symbol-observable'
Expand Down Expand Up @@ -464,6 +465,16 @@ describe('createStore', () => {
).toThrow(/may not dispatch/)
})

it('does allow dispatch() from within a reducer if configured accordingly', () => {
const store = createStore(reducers.dispatchInTheMiddleOfReducer, identity, {rules: { allowDispatch: true } })

expect(() =>
store.dispatch(
dispatchInMiddle(store.dispatch.bind(store, unknownAction()))
)
).not.toThrow()
})

it('does not allow getState() from within a reducer', () => {
const store = createStore(reducers.getStateInTheMiddleOfReducer)

Expand All @@ -472,6 +483,14 @@ describe('createStore', () => {
).toThrow(/You may not call store.getState()/)
})

it('does allow getState() from within a reducer if configured accordingly', () => {
const store = createStore(reducers.getStateInTheMiddleOfReducer, identity, {rules: { allowGetState: true } })

expect(() =>
store.dispatch(getStateInMiddle(store.getState.bind(store)))
).not.toThrow()
})

it('does not allow subscribe() from within a reducer', () => {
const store = createStore(reducers.subscribeInTheMiddleOfReducer)

Expand All @@ -480,6 +499,14 @@ describe('createStore', () => {
).toThrow(/You may not call store.subscribe()/)
})

it('does allow subscribe() from within a reducer if configured accordingly', () => {
const store = createStore(reducers.subscribeInTheMiddleOfReducer, identity, {rules: { allowSubscriptionHandling: true } })

expect(() =>
store.dispatch(subscribeInMiddle(store.subscribe.bind(store, () => {})))
).not.toThrow()
})

it('does not allow unsubscribe from subscribe() from within a reducer', () => {
const store = createStore(reducers.unsubscribeInTheMiddleOfReducer)
const unsubscribe = store.subscribe(() => {})
Expand All @@ -488,6 +515,15 @@ describe('createStore', () => {
store.dispatch(unsubscribeInMiddle(unsubscribe.bind(store)))
).toThrow(/You may not unsubscribe from a store/)
})

it('does allow unsubscribe from subscribe() from within a reducer if configured accordingly', () => {
const store = createStore(reducers.unsubscribeInTheMiddleOfReducer, identity, {rules: { allowSubscriptionHandling: true } })
const unsubscribe = store.subscribe(() => {})

expect(() =>
store.dispatch(unsubscribeInMiddle(unsubscribe.bind(store)))
).not.toThrow()
})

it('recovers from an error within a reducer', () => {
const store = createStore(reducers.errorThrowingReducer)
Expand Down Expand Up @@ -523,7 +559,7 @@ describe('createStore', () => {
const spyEnhancer = vanillaCreateStore => (...args) => {
expect(args[0]).toBe(reducers.todos)
expect(args[1]).toBe(emptyArray)
expect(args.length).toBe(2)
expect(args[2]).toBe(undefined)
const vanillaStore = vanillaCreateStore(...args)
return {
...vanillaStore,
Expand All @@ -547,7 +583,7 @@ describe('createStore', () => {
const spyEnhancer = vanillaCreateStore => (...args) => {
expect(args[0]).toBe(reducers.todos)
expect(args[1]).toBe(undefined)
expect(args.length).toBe(2)
expect(args[2]).toBe(undefined)
const vanillaStore = vanillaCreateStore(...args)
return {
...vanillaStore,
Expand Down
1 change: 1 addition & 0 deletions test/utils/others.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const identity = (arg) => arg