connected-react-router updates State faster than React unmounts component

[RichardForrester]

"connected-react-router": "^6.4.0", 
"react-redux": "^7.0.3",
"react-router": "^5.0.0"

After investigating a strange bug, it has become apparent that State.router is being updated slightly faster than React.Component is unmounted causing a mismatch between the State.router.loction and ownProps.location.

When transitioning from one route to another, the State is updated a split second faster than the component is unmounted causing a re-render of the React.Component that is about to unmount.

mapStateToProps = (state, ownProps) => {
  console.log(state.router.location.pathname === ownProps.location.pathname)
  // -> true on enter, false on exit
}

If you are relying upon State.router for component props, it can wreck your app.

There is an obvious solution which is to pull from ownProps instead of State, but doesn't that defeat the purpose of using connected-react-router?

[sateffen]

Might be connected to the react-redux release 7:

Direct Component Subscriptions

In v6, we switched from individual components subscribing to the store, to having subscribe and components read the store state from React's Context API. This worked, but unfortunately the Context API isn't as optimized for frequent updates as we'd hoped, and our usage patterns led to some folks reporting performance issues in some scenarios.

In v7, we've switched back to using direct subscriptions internally, which should improve performance considerably.

(This does result in some changes that are visible to user-facing code, in that updates dispatched in React lifecycle methods are immediately reflected in later component updates. Examples of this include components dispatching while mounting in an SSR environment. This was the behavior through v5, and is not considered part of our public API.)

So using react-redux in version ^6.0.0, like the peerDependencies tell you to, should actually fix the problem. If it does, the problem is narrowed down I think.

[MartinGian]

Hi, I am facing the same issue. I've tried with react-redux v7.x and with react-redux v6.x and was the same. The components trigger a render on go back button, even if the new route does not match with the defined at the Router level.

[koyta]

same

[koyta]

This is so cancer, anyone fixed that?

[RichardForrester]

@koyta I wish I had more time to help try and fix. But unfortunately, for me, I am an attorney and I only get to program as a hobby. I am completely consumed with other work at the moment. My "fix," was sadly to remove connected-react-router entirely from my project and update my selectors that pulled params from state.router.location.pathname to pull from ownProps.location.pathname instead. Of course that fixed the issue, but any other neat tricks that relied upon router info being in the state object are also gone. In my case, that didn't end up being that big of deal.

[vslutov]

I have fixed this problem in my project by pinning dependency versions:

  "connected-react-router": "5.0.1"
  "react-redux": "5.1.1"
  "react-router": "4.3.1",
  "react-router-dom": "4.3.1"

[noobG]

yeah this blew up my app too - this is a pretty critical bug and is really hard to debug because of the multiple libraries involved

[brettjonesdev]

Looks like this is the same issue as https://github.com/supasate/connected-react-router/issues/299.

I'm experiencing this as well, using

    "connected-react-router": "6.5.2",
    "react-redux": "7.1.0",
    "react-router-dom": "5.0.1",
``

[brettjonesdev]

I think I solved the problem

[salvoravida]

@brettjonesdev @RichardForrester @koyta Use https://github.com/salvoravida/redux-first-history

No more location issue. 🎉

[momesana]

@brettjonesdev @RichardForrester @koyta Use https://github.com/salvoravida/redux-first-history

No more location issue.

Well, doesn't work for us. Same issue as with connected-router in terms of back-navigation using the browser back-button.

[salvoravida]

@brettjonesdev @RichardForrester @koyta Use https://github.com/salvoravida/redux-first-history No more location issue.

Well, doesn't work for us. Same issue as with connected-router in terms of back-navigation using the browser back-button.

hum it's quite impossible, since there is only one history, console.log(state.router.location.pathname === ownProps.location.pathname) // ->will be true ALWAYS by redux-fist-history design

can you provide a code sand box?

[momesana]

@brettjonesdev @RichardForrester @koyta Use https://github.com/salvoravida/redux-first-history No more location issue.

Well, doesn't work for us. Same issue as with connected-router in terms of back-navigation using the browser back-button.

hum it's quite impossible, since there is only one history, console.log(state.router.location.pathname === ownProps.location.pathname) // ->will be true ALWAYS by redux-fist-history design

can you provide a code sand box?

It's proprietary company code but I can try to extract the relevant pieces into a self-contained demo. In case it helps here is a screenshot of the dev-tools after the app goes haywire: As can be seen the component for a single issue (thus the need to extract the id from the location) is rerendered before the router as a result of a back-navigation event, which fails because the location has already been updated to .../issues (instead of .../issues/:id). I'm on a branch where connected-router has been completely removed and adapted the code to resemble the examples from the redux-first-history github page.

We have migrated to hooks (i. e. functional components) all over the place so I can't evaluate the comparison with ownProps though.

[salvoravida]

@momesana I m quite sure that there is something wrong with your code, not with redux first history. I will try to explain: back button or dispatch new url is exactly the same on redux first history, because of one way data flow design! Without looking at your code I cannot help you. What select issue.js selector? How are nested components under react router <Route? Please provide e demo on codesandbox otherwise it is not possible to understand your issue.

[momesana]

@momesana I m quite sure that there is something wrong with your code, not with redux first history.

@salvoravida that may very well be the case. Either way I am sort of stuck with that issue and it happens both on connected-react-router and on the revised version with redux-first-history and I would very much appreciate it, if you could spot the problem and point me toward a feasible solution.

I will try to explain: back button or dispatch new url is exactly the same on redux first history, because of one way data flow design! Without looking at your code I cannot help you. What select issue.js selector? How are nested components under react router <Route? Please provide e demo on codesandbox otherwise it is not possible to understand your issue.

I've just scaffolded and uploaded a fairly minimal example with CRA (to github and codesandbox respectively) which highlights the problem with about 150 trivial lines of code in total, both on connected-react-router (master branch) and redux-first-history (using-redux-first-history branch)

github:

• redux-first-history version: https://github.com/momesana/premature-router-rerender-issue/tree/using-redux-first-history
• connected-react-router version: https://github.com/momesana/premature-router-rerender-issue

codesandbox:

• redux-first-history version: https://codesandbox.io/s/github/momesana/premature-router-rerender-issue/tree/using-redux-first-history/?from-embed
• connected-react-router version: https://codesandbox.io/embed/github/momesana/premature-router-rerender-issue/tree/master/?from-embed

How to reproduce the problem:

1. load the code-sandbox example
2. click on one of the issues in the issue-list
3. once in the issue page (i. e. the page of the previously selected issue) hit the browsers back-button.

An exception will be thrown from withing the selectors.js file. This file includes two very basic selectors:

export const getIssues = state => state.issues;

const issueIdRegex = /.*\/issues\/(\d+)\/?/;

export const getIssue = state => {
  const { pathname } = state.router.location;
  const match = pathname.match(issueIdRegex);

  if (!match) {
      // we throw the exception here because the id is not specified as the location
      // was already updated but the route still briefly tries to rerender the issues
      // component instead of the issue list page
    throw Error("pathname doesn't specify an id"); 
  }

  const issueId = match[1]
  return state.issues[issueId];
};

The getIssue selector is used in the component rendered by the "/issues/:id" route: App.js:

...
function App() {
  return (
    <Switch>
      <Redirect from="/" exact to="/issues" />
      <Route exact path="/issues" component={Issues} /> // list containing the issues
      <Route path="/issues/:id" component={Issue} /> // individual issues rendered if an id is given
    </Switch>
  );
}

issue.jsx

...
export default function Issue() {
  const issue = useSelector(getIssue); // here we invoke the getIssue selector which throws the exception when we try to back-navigate.

  return (
    <>
      <Title>{issue.name}</Title>
      <div>{issue.description}</div>
    </>
  );
}

Thanx for taking the time, btw :-)

[salvoravida]

@momesana ok, the problem is the states update top-down, because of selector update before <Route /> on this particular edge case (even if i think you should return null if the selector does not match)

i have fixed it in rfh 4.2.5 check demo here: https://codesandbox.io/s/router-issue-yb275

because redux-first-history is not react only related, i expressly support react-dom batches updates on createReduxHistoryContext .

usage:

import { unstable_batchedUpdates } from "react-dom";
import { createBrowserHistory } from "history";
import { createReduxHistoryContext } from "redux-first-history";

const {
  createReduxHistory,
  routerMiddleware,
  routerReducer
} = createReduxHistoryContext({
  history: createBrowserHistory(),
  batch: unstable_batchedUpdates
});

Hope i have helped you!

[momesana]

@momesana ok, the problem is the states update top-down, because of selector update before <Route /> on this particular edge case (even if i think you should return null if the selector does not match)

Good point regarding the selector return value.

i have fixed it in rfh 4.2.5 check demo here: https://codesandbox.io/s/router-issue-yb275

Thank you very much @salvoravida. I just updated redux-first-history and applied the proposed changes and it works like a charm. If any other issues turn up I'll let you know on the redux-first-history issues page.

because redux-first-history is not react only related, i expressly support react-dom batches updates on createReduxHistoryContext .

usage:

import { unstable_batchedUpdates } from "react-dom";
import { createBrowserHistory } from "history";
import { createReduxHistoryContext } from "redux-first-history";

const {
  createReduxHistory,
  routerMiddleware,
  routerReducer
} = createReduxHistoryContext({
  history: createBrowserHistory(),
  batch: unstable_batchedUpdates
});

Hope i have helped you!

You did, indeed. Thanks again :-).

[connorgiles]

Any update on this issue? Is the only workaround currently to use redux-first-history as @momesana suggested? This library should be able to accommodate browser navigation.

[escaton]

The root of the problem are the following 3 pieces of code: 1) subscription to location change in connected-react-router + subsequent dispatch and update of all connected components https://github.com/supasate/connected-react-router/blob/a1053804b1097cda52e14b4e52b5a0ea3a421ede/src/ConnectedRouter.js#L72 2) another subscription to locations change in react-router itself to update internal state https://github.com/ReactTraining/react-router/blob/b6826980ee0d89267ebac4f066ab8ed517536f2c/packages/react-router/modules/Router.js#L32 3) listeners invocation in history module (v4): https://github.com/ReactTraining/history/blob/3f69f9e07b0a739419704cffc3b3563133281548/modules/createTransitionManager.js#L67

So while all of them are executed synchronously, 1 and 2 are interpreted as independent updates by React. This means that at first React will update all connected components (they will receive state with updated router field in mapStateToProps). And only after that react will perform next update triggered by react-router subscription (2) and change components in tree according to routing. This is why we can catch new location from redux state in obsolete react component.

To prevent such issues we need to turn these independent updates into one. To do that we can import { unstable_batchedUpdates } from "react-dom"; and patch history object like this:

const history = createBrowserHistory();

// create own subscription pool
// copypasted from https://github.com/ReactTraining/history/blob/3f69f9e07b0a739419704cffc3b3563133281548/modules/createTransitionManager.js#L49
let listeners = [];

function appendListener(fn) {
  let isActive = true;

  function listener(...args) {
    if (isActive) fn(...args);
  }

  listeners.push(listener);

  return () => {
    isActive = false;
    listeners = listeners.filter(item => item !== listener);
  };
}

function notifyListeners(...args) {
  listeners.forEach(listener => listener(...args));
}

// make only one subscription to history changes and proxy to our internal listeners
history.listen((...args) => {
  // here's the key change
  unstable_batchedUpdates(() => {
    notifyListeners(...args)
  })
});

// monkey patch to store subscriptions into our own pool
history.listen = (fn) => {
  return appendListener(fn);
}

// follow README.md

const store = createStore(
  createReducer(history), // root reducer with router state
  composeEnhancers(
    applyMiddleware(
      routerMiddleware(history) // for dispatching history actions
      // ... other middlewares ...
    )
  )
);

const App = () => {
  return (
    <Provider store={store}>
      <ConnectedRouter history={history}>
        ...
      </ConnectedRouter>
    </Provider>
  );
};

And that's it! Since history module is unaware of React specificity, this patch should be placed in connected-react-router or react-router. @supasate what do you think?

[salvoravida]

@escaton yeah you are right. Note that redux-first-history, is even better because batch all history listeners, react-router-dom, and store updates.

so again, the MAIN problem of connect-react-router, is that it depends on react-component lifecycle.

Put in the mind, that there is NO reason of a

[escaton]

@salvoravida what do you mean here

the MAIN problem of connect-react-router, is that it depends on react-component lifecycle

connected-react-router only subscribe to history in it’s constructor and that’s all. Am I missing something?

[salvoravida]

yes. you are missing that history should be in sync with store, always not on mount/unmount. Moreover, CRR will not garantee that state.router.location=== history.location because it duplicated the Source of true

https://github.com/salvoravida/redux-first-history

Over 100 issues are caused by this location ref duplication.

[agentcooper]

@supasate I've encountered this issue today using the latest connected-react-router together with react-redux@7.2.2 and react-router-dom@5.2.0. And indeed the solution above by @escaton fixes the problem. Would you accept a pull request for this?

[mdubourg001]

Had the same issue when migrating from "react-router-redux" to "connected-react-router" and struggled a lot to try to fix the issue without success.

I just switched to redux-first-history as mentioned by @salvoravida and it works like a charm 👍