React-router: component wrapped by withRouter is unmounted on every re-render

[marksweston]

With regard to withRouter in v4:

I wrapped a component with withRouter in order to get access to push. My component maintained a small amount of state in order to manage user interactions with child components (e.g. currently selected tab).

As soon as I added withRouter, my component started behaving strangely, and I eventually tracked it down to the fact my component, now wrapped by withRouter, was being unmounted and re-mounted every time a user interaction caused what would previously have been an update / re-render. The result was of course that state was being lost every time.

Is this intended behaviour? It seems to me that the implied contract of an HOC is that it adds behaviour or properties to the wrapped component without changing the previous behaviour of that component, and this side-effect breaks that contract. It certainly makes it hard to use withRouter when one has to consider whether a) it's safe to use withRouter with any specific component and then b) worry about that component might need to change some time in the future, in a way that will make it retrospectively incompatible with withRouter.

I don't yet understand the internals of react-router very well, but I assume this behaviour is a result of the implementation of withRouter by rendering a Route and passing the wrapped component as a prop to be rendered. Is this essential? Presumably the problem goes away if withRouter accesses the ancestor Router's context and renders the wrapped component directly?

[Shaxmatist]

Yes, something happened or changed with withRouter in beta.7. In version beta.6 everything worked correctly.

[Shaxmatist]

I'm sorry, the previous comment is probably not related to this topic. But there's something really wrong with the beta.7. When routes changed, all components are not updated. While I do not understand what the problem is.

Now the question about withRouter v4.
If the Component wrapped by withRouter and url is /items/:itemID then match.params is always empty. Is it normal behavior? Why itemID is not available through params in Component?

[mjackson]

there's something really wrong with the beta.7

Are you using react-redux or shouldComponentUpdate in your route components?

[firasdib]

@mjackson I am using react-redux and seeing the same problems as @Shaxmatist. I suppose the props aren't being compared properly and thus the components aren't updating. What specifically changed in beta7 for this to occur? Perhaps it should be changed as the current behavior seems a bit backwards and having to manually re-implement shouldComponentUpdate to tailor to specific things in react-router feels a bit tedious.

[pshrmn]

Subscriptions were removed in beta 7.

Navigation re-renders should propagate from the<Router>, not every location aware component. If you have a component that uses sCU to prevent re-renders, React Router shouldn't have to try to get around that. InsteadsCU should recognize that the location is different and return true. When it comes to react-redux, you can do this by passing the location as a prop to the connected component.

[firasdib]

@pshrmn That would probably explain some of the issues I've been having, but I still can't get my app to update.

I have a Root component that renders the following only:

      <Provider store={this.props.store}>
        <Router>
          <MainContainer />
        </Router>
      </Provider>

And then MainContainer contains a bunch of routes in a switch. It depends on react-redux so I've overwritten shouldComponentUpdate to always return true. Regardless of this change it does not work.

[pshrmn]

Pass the { pure: false } option to connect (fourth argument) and it should work.

[firasdib]

@pshrmn That seems to have solved one of my issues, but the remounting of components (which I believe was the root of this issue) is still present.

[Shaxmatist]

Are you using react-redux or shouldComponentUpdate in your route components?

I'm using react-redux. Thanks to @pshrmn for the answer. But question about withRouter and match.params is actual.

[pshrmn]

@Shaxmatist Right now, withRouter renders a pathless <Route>, which creates a new, "empty" match object. That is why your parameters are not showing up (and this is different than what would happen if you accessed the parent match directly through context, so I would consider this a bug).

Based on my work with relative routes, I believe that a pathless <Route> should inherit the match from its parent. If that is the case, then the match prop inside of withRouter would have the correct params.

[mjackson]

@pshrmn Let's fix the bug and create a doc for people using react-redux and/or shouldComponentUpdate.

[marksweston]

@pshrmn @mjackson
This conversation feels like it's taken a slight tangent from my original question, if you're only looking at how match params are passed down to the component wrapped by withRouter.
Is the repeated unmounting of the wrapped component intended behaviour? Should it be?

(my app wasn't using react-redux and the relevant component didn't define shouldComponentUpdate).

[mjackson]

You're right, @marksweston. My apologies.

What beta version are you using? We recently fixed a re-mounting issue in #4592 (released in beta 7). Could be that it's already fixed.

[timdorr]

Also, some example code would be really helpful here.

[pshrmn]

@marksweston An example would be appreciated. I am not seeing any difference in the mount/updates between a component wrapped with withRouter and one that is not in this codepen

[marksweston]

@mjackson I was using beta.6, but updating to beta.7 gives me the same behaviour.

It's a bit tough to extract a readable example from our apps codebase, so I'll try to construct something in Codepen or an example app.

[marksweston]

Update: So far I'm failing to come up with an example that fits easily in Codepen and reproduces the unmounting behaviour.

I can still reproduce the problem in my app at will, but have worked round it by simply removing withRouter and having a component that's already router-aware pass push down the tree as a prop.

I will have another go at creating an example app that reproduces the problem tonight.

[taurose]

Some guesses:

• you call withRouter(..) during rendering which would create a new component class each time
• you pass a new function to Route.component each render, e.g. using anonymous function <Route component={() => <div>{...}</div>} />, which would create a new component as well

[marksweston]

  render() {
    const RoutedInvoicesTable = withRouter(InvoicesTable);
    ...
  }

Good guess, thank you. At the time it seemed like a good idea to put the const declaring the component class as close as possible to where it was used, for future readability. Doh.

I guess I'm ready to close this issue, but it did start a parallel conversation which might still be about a genuine bug. So should I leave it open for now?

[pshrmn]

The other issue has been resolved as well.

[mjackson]

Thanks @taurose :)