Skip to content
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

Warn When dispatching During Middleware Setup #1484

Closed
wants to merge 3 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
7 changes: 6 additions & 1 deletion src/applyMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ 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 = () => {
throw new Error(
`Dispatching while constructing your middleware is not allowed. ` +
`Other middleware would not be applied to this dispatch.`
)
}
var chain = []

var middlewareAPI = {
Expand Down
11 changes: 11 additions & 0 deletions test/applyMiddleware.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@ 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)
}

expect(() =>
applyMiddleware(dispatchingMiddleware)(createStore)(reducers.todos)
).toThrow()
})

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