connected-react-router icon indicating copy to clipboard operation
connected-react-router copied to clipboard

Back button updates selectors with params before route change happens

Open preciselywilliam opened this issue 5 years ago • 4 comments

Use case:

  1. navigate to /items (ItemListPage)
  2. navigate to /items/:itemId (ItemPage)
  3. press browser back button

ItemPage:

ItemPage.propTypes = {
  params: PropTypes.shape({ itemId: PropTypes.string.isRequired }).isRequired,
};

const mapStateToProps = state => ({ params: getParams });

getParams selector (psuedo-ish code):

const pathname = state.router.location.pathname;
const matchObject = appRoutes.map(route => matchPath(pathname, route)).find(matchObj => !!matchObj);
return matchObject.params;

When I press back button (step 3), the selector returns empty object, since no match was found. This rerenders ItemPage before the route change happens, which causes a propType warning (itemId undef).

What I expect to happen is for connected-react-router to update the state after the route change has happened, which wouldn't result in a rerender of ItemPage.

When navigating with Links or push, there is no propType warning.

Is this standard behavior or am I doing something wrong?

preciselywilliam avatar Apr 25 '19 12:04 preciselywilliam

@preciselywilliam I imagine you've found a workaround by now, but I think I discovered the problem/solution to this one. I'll post a demo soon, but for now, it looks like the problem has to do with the fact that react-router's <Route /> component is pulling the location in from a <RouterContext.Consumer>. But for whatever reason, when you use the browser back button, the Redux state and Redux getLocation() selector fires before the updated location context is exposed to the <Route/>!

This leads any connected child components to re-render briefly before the parent <Route/> has a chance to re-render and unmount them.

The solution I found was to use the location option on the <Route> and <Switch>, passing in a location that is pulled from Redux. This puts the Route and any children on the same footing, and the behavior is as expected.

Won't work

function Routes() {
  return (
    <Switch>
      <Route path="foo" component={connectedFoo} />
      <Route path="bar" component={connectedBar} />
    </Switch>
  )
}

Will work

import { getLocation } from 'connected-react-router'

function Routes({ location }) {
  return (
    <Switch location={location}>
      <Route location={location} path="foo" component={connectedFoo} />
      <Route location={location} path="bar" component={connectedBar} />
    </Switch>
  )
const mapStateToProps = state => ({ location: getLocation(state) })
export default connect(mapStateToProps)(Routes)

}

brettjonesdev avatar Sep 14 '19 01:09 brettjonesdev

That workaround apparently works but breaks redirects in our code, so is not an option. Not being able to use the 'Go back' browser button is a show-stopper.

momesana avatar Nov 22 '19 13:11 momesana

@preciselywilliam The workaround propsed above is merely a hack and even breaks redirects. It doesn't justify closing the issue. The developer of redux-first-history fixed the same problem in version 4.2.5 and described what caused it in the first place here: #311. Maybe it helps you tackle the root of the problem and fix the issue in the library itself.

momesana avatar Nov 23 '19 17:11 momesana

@momesana I've reopened the issue.

preciselywilliam avatar Nov 25 '19 09:11 preciselywilliam