Multiple unexpected history entries when using routerMiddleware

[Chris5732]

When using routerMiddleware from connected-react-router/immutable I am getting an issue where anytime the location is changed I get multiple entries in the browser history if a middleware anywhere else inside applyMiddleware calls dispatch with any action (even fake mock actions).

For example in my code I use:

replace({
            pathname: '/overview',
            search: `?assetId=${assetId}`
        });

This only gets hit once, but puts two entries in the browser history. Also if I click the back button I get a further 2 entries appended to the history.

Please find below my unrelated custom middleware which comes after routerMiddleware in applyMiddleware, which when used in conjunction with routerMiddleware, causes the issue described above.

const routeChangeMiddleware = store => next => action => {
    if (action.type === '@@router/LOCATION_CHANGE') {
        store.dispatch(hideMessage());
    }

    next(action);
}; 

Removing store.dispatch(hideMessage()); causes the history to work as expected.

[chrstph-dvx]

Flow is:

• dispatch(push(/test)) (from /)
• routerMiddleware wait for CALL_HISTORY_METHOD and triggers history method (code)
• connectedRouter history listener is triggered and dispatch LOCATION_CHANGE action
• LOCATION_CHANGE is going through middlewares
• middleware dispatch hideMessage
• connectedRouter store subscriber is triggered from LOCATION_CHANGE
  • pathnameInHistory being next URL (/test)
  • pathnameInStore being URL before change (/)
  • it will trigger a new change on history to /
• And then a second time from hideMessage:
  • pathnameInHistory being next URL (/)
  • pathnameInStore being URL before change (/test)
  • it will trigger a new change on history to /test

History entries will be:

• /test
• /
• /test

One fix seems to be moving next(action) before store.dispatch

[LuckyLuky]

+1

[AlexReff]

I encountered this as well, and did not find any solutions posted anywhere.

I was able to fix this by moving my call to next(action) to the top of my middleware, and returning the result at the end, instead of doing a return next(action) at the end of my middleware.

Previously:

export const myMiddleware = () => {
  //do custom middleware things
  return next(action);
}

Now:

export const myMiddleware = () => {
  const result = next(action);
  //do custom middleware things
  return result;
}

[richsilv]

The reason the fixes above are necessary is as follows:

ConnectedRouter subscribes to store actions in its constructor here. The callback checks whether the location in the store matches the location in the history object, and if not pushes a new location into the history object here.

If the user clicks a link, the new location will be pushed to history straight away, but if you intercept the accompantying LOCATION_CHANGED action in your middleware and dispatch any other action before calling next on that LOCATION_CHANGED action, you'll potentially update the store via another reducer before the original action has been reduced. That means that the subscriber in ConnectedRouter will fire whilst the (old) router state hasn't yet been updated to match the history location, and the subscriber callback will call history.push with the old location. The original LOCATION_CHANGED action will then be reduced, which means the store and history states will be mismatched again once the store is updated, and this time you'll get another history.push back to the correct location.

So, the main thing is to call next on the action passed into the middleware before dispatching anything else (either via api.dispatch or next). You can easily store the return value of calling next and then return it at the end of the middleware.

Is it worth including this in the README, @supasate ?