"dispatch" passed to middlewares only passes through first argument

[Asvarox]

Do you want to request a feature or report a bug? Bug

What is the current behavior?
Right now a "dispatch" function that's passed to middlewares passes through only the first argument.

From applyMiddleware.js:

    var middlewareAPI = {
      getState: store.getState,
      dispatch: (action) => dispatch(action)
    }

I have a custom middleware that accepts a dispatch call with 2 arguments. This works fine in, say, mapDispatchToProps (from react-redux) as it uses store.dispatch, but in dispatch calls in redux-thunk, the second argument disappears.

What is the expected behavior?
All the arguments are passed to the dispatch:

    var middlewareAPI = {
      getState: store.getState,
      dispatch: (...args) => dispatch(...args)
    }

There are ways to work around it (you can put all the arguments in an array or object), additional arguments can also be stripped by any middleware in middlewares chain, but still it would make things more consistent.

If it makes sense I can submit a PR.

[timdorr]

Yeah, go ahead and make a PR for this. I'd be interested to see if there are any objections.

[markerikson]

Erm... potentially. This is very similar to the issue discussed in #1813 . As far as I know, all existing middlewares and enhancers assume that there's only one argument being passed to dispatch. So, this would only work if applyMiddleware is the first enhancer, and your middleware is the first one in the chain.

Out of curiosity, @Asvarox , what is your current use case? What does your custom middleware do?

[Asvarox]

@markerikson sure. Given a dispatch is called with function as first argument and an array as second, it will dispatch the result of calling the function passing contents of the array as arguments:

// Written "by hand",this is the "core" of actual code
const middleware = ({ dispatch }) => next => (actionCreator, args) => dispatch(actionCreator(...args));

The reasoning is to make testing thunks easier. Ihave quite a few of these that dispatch other actions - by dispatching the result of calling another action creator. That forced me to assert what these action creators return - if one of them started to return another thunk instead of plain object, it broke the tests. More so it was then pretty hard to fix the assertion.

The example from my project. I had an async action doSomething that after completion would show a notification to the user by dispatching whatever was the result of action creator "addNotification". Since addNotification was actually returning just a plain object, in test I'd assert that the object that the dispatch was called with is "proper". Some time after I needed certain notifications to be hidden after given amount of time. addNotification started to return a thunk that'd add a setTimeout to dispatch hideNotification action creator if timeout was specified. This broke the tests of doSomething and more so, it was hard to keep having the test make sure the notification is added.

I couldn't find feasible solution online so I came up with a few. The middleware is one of the ones I wanted to experiment with (writing middlewares is fun ;) ) and then I found this little bug.

[eddyw]

@Asvarox the middlewareApi variable wraps the original dispatch function which is here:
https://github.com/reactjs/redux/blob/master/src/createStore.js
And only accepts one argument which then does:
currentState = currentReducer(currentState, action)
As you can see, the original dispatch function passes the currentState as first argument and the action as second. So...

 var middlewareAPI = {
      getState: store.getState,
      dispatch: (...args) => dispatch(...args)
}

won't have any effect, since only the first argument is taken into account.

[Asvarox]

Hey @eddyw that's not entirely true as later in applyMiddleware the dispatch variable is overwritten with "final" dispatch (the one that's in enhanced store).

Otherwise you would only be able to dispatch plain actions in middlewares (or pass the action through next-in-chain middleware) and not "start over". Dispatching thunk actions in thunks wouldn't be possible.

[eddyw]

@Asvarok yeah, you are right about it. I kind of overlooked that part.

I guess passing the arguments to the function without hitting performance would be the challenge.