Enhancers are added after middleware

[KaRkY]

Hello,

Could the order of enhancers be switched? Now they are added after the middleware and that is why no middleware can see actions that are generated by enhancers.

Combination of redux-first-router and redux-saga is an example of this.

[markerikson]

Out of curiosity, what other enhancers do you have that are generating actions?

[markerikson]

Hmm. Okay, so yeah, looking at the redux-first-router docs, they really do want their enhancer to be in front of the middleware.

My first instinct is that the point of redux-starter-kit is to be reasonably opinionated, and the point of configureStore() is to keep the end user from having to fiddle around with the store setup process. In almost every Redux app I've seen, applyMiddleware comes first. I'd say that if you need to care about the order of enhancers that way, you're probably better off writing the store setup logic by hand.

[KaRkY]

This is the only one for me it is not a problem to write few lines of code that never change. I just wanted to point that out for consideration.

[icopp]

My company's also using redux-first-router, so we're not able to use configureStore because of that.

Do any other projects break if enhancers come first?

[denisw]

If we wanted to support this, here is an idea:

Export a special configureMiddleware value from redux-starter-kit (e.g., a symbol). If configureMiddleware is found in the enhancers array, apply middleware at that point; otherwise, apply middleware first.

import { configureStore, configureMiddleware } from 'redux-starter-kit'

configureStore({
  reducer,
  // enhancer comes after middleware, as before
  store: [enhancer]
})

configureStore({
  reducer,
  // middleware comes after enhancer
  store: [enhancer, configureMiddleware]
})

configureStore({
  reducer,
  // middleware comes after enhancer1, before enhancer2
  store: [enhancer1, configureMiddleware, enhancer2]
})

[markerikson]

I suppose that's a possibility. Could accept a PR that adds that.

[primozs]

Just a thought. This looks worse then original config. Why not let the possibility to just configure store as you would in redux normally. For "advanced" uses.

[markerikson]

@primozs : if you want to configure the store "normally", there's nothing stopping you from using createStore() yourself directly. That's kind of the point. This is meant for the 80% use case

[primozs]

@markerikson I got confused when I saw only some parts of redux re exported.
Cheers

[stvmachine]

The same problem happens with the enhancer of the redux-offline. Thumbs up for something like this!.

I created a ticker for that one: #184

[markerikson]

I'm not particularly sold on the "magic applyMiddleware symbol" API design idea. Does anyone have any other alternatives?

[markerikson]

Hmm. We currently have https://redux-starter-kit.js.org/api/getDefaultMiddleware . Maybe we need a getDefaultEnhancers() function too?

[phryneas]

What about accepting a callback as enhancers, with the applyMiddlewares enhancer as argument?

function configureStore({
    // A single reducer function that will be used as the root reducer,
    // or an object of slice reducers that will be passed to combineReducers()
    reducer: Object<string, ReducerFunction> | ReducerFunction,
    // An array of Redux middlewares.  If not supplied, uses getDefaultMiddleware()
    middleware?: MiddlewareFunction[],
    // Enable support for the Redux DevTools Extension. Defaults to true.
    devTools?: boolean | EnhancerOptions,
    // Same as current createStore.
    preloadedState?: State,
    // An optional array of Redux store enhancers
    enhancers?: ReduxStoreEnhancer[] | (defaultEnhancer: ReduxStoreEnhancer) => ReduxStoreEnhancer[],
})

[markerikson]

Mmm... not sure I like the callback idea either. I'm definitely leaning towards getDefaultEnhancers() at this point.

[phryneas]

But wouldn't the getDefaultEnhancers() result contain the middlewareEnhancer and thus not be available without knowing of the middlewares?

So you'd have to pass in the middlewares and end up with something like this:

const middlewares = [ ..., getDefaultMiddlewares(), ... ];
const store = configureStore({
  middlewares,
  enhancers: [ ..., getDefaultEnhancers(middlewares), ... ]
})

Having to pass in the middlewares at two different places would make it impossible to write the middlewares inline.

[markerikson]

Hmm. Excellent point.

Okay, I see what you're getting at with the callback thing. In that case, would it make sense to do that for both middleware and enhancers, and drop getDefaultMiddleware ?

We're still 0.x, so we're semantically allowed to make whatever breaking changes we want, but I'd obviously like to minimize churn overall.

FWIW, I see a number of folks calling getDefaultMiddleware already:

https://github.com/search?l=JavaScript&q=getdefaultmiddleware&type=Code

[phryneas]

Either that, or enable both the current behaviour and the callback option.

That might have not been clear from my initial post:
My initial idea was to allow either passing in an array (which would put the enhancers behind the middleware, so the current behaviour) or optionally passing the callback for more control.

Similarly, middlewares could either receive an array (which would replace all middlewares, but be used with a - potentially deprecated - getDefaultMiddleware call) or passed a callback function.

[markerikson]

Resolved this by adding the callback idea for enhancers suggested by @phryneas .