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

Order of other middleware #87

Open
cpsubrian opened this issue Sep 28, 2015 · 9 comments
Open

Order of other middleware #87

cpsubrian opened this issue Sep 28, 2015 · 9 comments
Labels

Comments

@cpsubrian
Copy link

Not sure if this is just a gotcha to be documented or an actual issue...

TLDR I needed to put thunk and promise middleware before the router and I had to put my logger middleware before AND after the router (and make sure I only log the same action once). This is due to the router historyMiddleware not calling next() (which is probably correct).

Before

I'm creating my store with the following code. With my middleware chain before the router, everything works fine except I don't get any logging of @@reduxReactRouter/historyAPI actions, which is totally predictable because if the historyMiddleware catches one of those it triggers a state change and the rest of the middleware does not fire. This is not a problem per-se but my preference is to log all actions during development so I can really see whats going on in the app.

store.js

import {createStore, applyMiddleware, compose} from 'redux'
import {reduxReactRouter} from 'redux-router'
import createBrowserHistory from 'history/lib/createBrowserHistory'
import transducers from './transducers'
import reducers from './reducers'
import middleware from './middleware'

let devTools = () => next => (reducer, initialState) => next(reducer, initialState) // noop
if ((process.env.NODE_ENV === 'development') && (window.localStorage.getItem('DEVTOOLS') === 'enabled')) {
  devTools = require('redux-devtools').devTools
}

const initialState = {}

const store = compose(
  applyMiddleware.apply(null, middleware),
  reduxReactRouter({createHistory: createBrowserHistory}),
  devTools()
)(createStore)(transducers(reducers), initialState)

// Enable Webpack hot module replacement for reducers.
if (module.hot) {
  module.hot.accept('./reducers', () => {
    const nextRootReducer = require('./reducers/index')
    store.replaceReducer(transducers(nextRootReducer))
  })
}

export default store

transducers/index.js

import {compose} from 'redux'
import history from './history'

export default compose(
  history
)

reducers/index.js

import {combineReducers} from 'redux'
import account from './account'
import comic from './comic'
import feed from './feed'
import notifications from './notifications'
import router from './router'

const reducers = {
  account,
  comic,
  feed,
  notifications,
  router
}
const combined = combineReducers(reducers)

middleware/index.js

import logger from './logger'
import crashReporter from './crashReporter'
import promise from './promise'
import thunk from './thunk'
import validationErrors from './validationErrors'
import {batchedUpdatesMiddleware} from 'redux-batched-updates'

const middleware = [
  crashReporter,
  promise,
  thunk,
  logger,
  validationErrors,
  batchedUpdatesMiddleware
]

export default middleware

After

To accommodate my logging needs, I split my middleware into a 'before' chain and an 'after' chain like so:

middleware/_before.js

import crashReporter from './crashReporter'
import promise from './promise'
import thunk from './thunk'
import logger from './logger'

const middleware = [
  crashReporter,
  promise,
  thunk,
  logger
]

export default middleware

middleware/_after.js

import logger from './logger'
import validationErrors from './validationErrors'
import {batchedUpdatesMiddleware} from 'redux-batched-updates'

const middleware = [
  logger,
  validationErrors,
  batchedUpdatesMiddleware
]

export default middleware

middleware/logger.js
I added a check to make sure the same action doesn't get logged twice.

store.js (changes)

const store = compose(
  applyMiddleware.apply(null, beforeMiddleware),
  reduxReactRouter({createHistory: createBrowserHistory}),
  applyMiddleware.apply(null, afterMiddleware),
  devTools()
)(createStore)(transducers(reducers), initialState)
@cpsubrian
Copy link
Author

A quick addendum: No matter where I put my original middleware chain, everything 'functions', but I get different things logged (either everything but the historyAPI actions, or only the historyAPI actions).

@mikeyamadeo
Copy link

I ran into a related issue when dispatching a pushState action within redux-thunk

export const updateQuery = (query) => {
  return (dispatch, getState) => {
    const { router } = getState()

    dispatch(pushState(null, router.location.pathname, query))
  }
}

With this configuration:

const coreMiddleware = compose(
  reduxReactRouter({ routes, createHistory }),
  applyMiddleware(
    thunkMiddleware,
    apiMiddleware
  )
)

dispatching updateQuery has @@reduxReactRouter/historyAPI action type coming out the other end and fails to change the url state

Re-ordering:

const coreMiddleware = compose(
  applyMiddleware(
    thunkMiddleware,
    apiMiddleware
  ),
  reduxReactRouter({ routes, createHistory })
)

now has updateQuery spitting out @@reduxReactRouter/routerDidChange and correctly changes state.

like @cpsubrian, not sure if this is simply a gotcha that could use documentation or an issue to be looked into. hope that helps.

@tomazzaman
Copy link

Can confirm this issue, my working order of things is now:

const createComposedStore = compose(
  reduxReactRouter( { routes, createHistory } ),
  applyMiddleware( thunkMiddleware, apiMiddleware, historyMiddleware, loggerMiddleware ),
  devTools()
)( createStore );

Noticed no problems in calling the router before applyMiddleware.

@cpsubrian
Copy link
Author

Interesting .. when I didn't have a logger before and after the router, I was missing some of the dispatches.

@thiagofelix
Copy link

I guess am experiencing the same situation.

Store:

const logger = createLogger()
const createStoreWithMiddleware = compose(
  applyMiddleware(thunk, logger),
  reduxReactRouter({ createHistory })
)

Async Thunk action

export function logoutAndRedirect() {
  return (dispatch, state) => {
    dispatch(logout());
    dispatch(pushState(null, 'login'));
  }
}

Resulting Log ( Correct )
image

Navigating without pushState ( Link component )

<Link to="/orders" className="navigate-right">
  Order List
</Link>

The Link component changes the url, the historySynchronization dispatches a routerDidChange action which does the right changes but is not executed through the middles defined on the app hence not captured by the log middleware.

Is there a way to navigate using the Link component and still get the right actions on the log without having to mess with the middlewares?

@Scarysize Scarysize added the bug label Dec 24, 2015
@yuluyi
Copy link

yuluyi commented Jul 28, 2016

Same issue here.
if store is like

const store = compose(
  applyMiddleware.apply(null, [createSagaMiddleware()]),
  reduxReactRouter({createHistory: createBrowserHistory}),
  devTools()
)(createStore)(transducers(reducers), initialState)

Then I can't see routerDidChange action in saga.

if store is like

const store = compose(
  reduxReactRouter({createHistory: createBrowserHistory}),
  applyMiddleware.apply(null, [createSagaMiddleware()]),
  devTools()
)(createStore)(transducers(reducers), initialState)

Then I can see routerDidChange action in saga, but I can't dispatch push action to change url in saga.

@luisherranz
Copy link

Is it safe to do this to get both push and routerDidChange working with redux-saga?

const store = compose(
  reduxReactRouter({createHistory: createBrowserHistory}),
  applyMiddleware.apply(null, [createSagaMiddleware()]),
  reduxReactRouter({createHistory: createBrowserHistory}),
  devTools()
)(createStore)(transducers(reducers), initialState)

@marcusstenbeck
Copy link

Any news on whether this is resolved?

@lbdremy
Copy link

lbdremy commented May 28, 2017

Any updates on this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants