Skip to content

Warn When dispatching During Middleware Setup #1345

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 2 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
6 changes: 5 additions & 1 deletion docs/advanced/Middleware.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,12 +268,16 @@ The implementation of [`applyMiddleware()`](../api/applyMiddleware.md) that ship

* It only exposes a subset of the [store API](../api/Store.md) to the middleware: [`dispatch(action)`](../api/Store.md#dispatch) and [`getState()`](../api/Store.md#getState).

* It does a bit of trickery to make sure that if you call `store.dispatch(action)` from your middleware instead of `next(action)`, the action will actually travel the whole middleware chain again, including the current middleware. This is useful for asynchronous middleware, as we have seen [previously](AsyncActions.md).
* It does a bit of trickery to make sure that if you call `store.dispatch(action)` from your middleware instead of `next(action)`, the action will actually travel the whole middleware chain again, including the current middleware. This is useful for asynchronous middleware, as we have seen [previously](AsyncActions.md). There is one caveat when calling `dispatch` during setup, described below.

* To ensure that you may only apply middleware once, it operates on `createStore()` rather than on `store` itself. Instead of `(store, middlewares) => store`, its signature is `(...middlewares) => (createStore) => createStore`.

Because it is cumbersome to apply functions to `createStore()` before using it, `createStore()` accepts an optional last argument to specify such functions.

#### Caveat: Dispatching During Setup

While `applyMiddleware` sets up your middleware, the `store.dispatch` function will point to the vanilla version provided by `createStore`. None of the other middleware will be applied to this `dispatch` until after setup is complete. If you are expecting an interaction with another middleware during setup, you will probably be disappointed. Instead, you should either communicate directly with that other middleware via a common object (for an API calling middleware, this may be your API client object) or waiting until after the middleware is constructed with a callback.

### The Final Approach

Given this middleware we just wrote:
Expand Down
9 changes: 8 additions & 1 deletion src/applyMiddleware.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import compose from './compose'
import warning from './utils/warning'

/**
* Creates a store enhancer that applies middleware to the dispatch method
Expand All @@ -19,7 +20,13 @@ import compose from './compose'
export default function applyMiddleware(...middlewares) {
return (createStore) => (reducer, initialState, enhancer) => {
var store = createStore(reducer, initialState, enhancer)
var dispatch = store.dispatch
var dispatch = (...args) => {
warning(
`Calling dispatch while constructing your middleware is discouraged. ` +
`Other middleware will not be applied to this dispatch.`
)
store.dispatch(...args)
}
var chain = []

var middlewareAPI = {
Expand Down
14 changes: 14 additions & 0 deletions test/applyMiddleware.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@ import { addTodo, addTodoAsync, addTodoIfEmpty } from './helpers/actionCreators'
import { thunk } from './helpers/middleware'

describe('applyMiddleware', () => {
it('warns when dispatching during middleware setup', () => {
function dispatchingMiddleware(store) {
store.dispatch(addTodo('Dont dispatch in middleware setup'))
return next => action => next(action)
}

const spy = expect.spyOn(console, 'error')

applyMiddleware(dispatchingMiddleware)(createStore)(reducers.todos)

expect(spy.calls.length).toEqual(1)
spy.restore()
})

it('wraps dispatch method with middleware once', () => {
function test(spyOnMethods) {
return methods => {
Expand Down