Switch is re-mounting the same component on route change #4578

random-stranger · 2017-02-22T13:48:07Z

Version

4.0.0-beta.6

Test Case

Same issue as described here, the test case is from it

bug exists
http://codepen.io/ReactJSTraining/pen/ZLggog

no bug, but uses non-supported array as Route path
http://codepen.io/anon/pen/qReegR

Steps to reproduce

UsersIndex component is the one, that is persistent across the routes. Change the route to reproduce the bug (this component is re-mounted).

Expected Behavior

When route is changed, UsersIndex component is not re-mounted, only passed props are changing.

Actual Behavior

When route is changed, UsersIndex component is re-mounted on every route change.

This could be solved with Route path array support or current Switch component fix.

The text was updated successfully, but these errors were encountered:

pshrmn · 2017-02-22T14:56:55Z

This might have to do with the usage of React.cloneElement in the <Switch> components render function. I tested this with my <ConfigSwitch>, which uses React.createElement, and it worked as expected (codepen).

timdorr · 2017-02-22T18:27:49Z

Sanity check: Is this actually a problem? Mounting : unmounting !:: construction : destruction.

Switch's goal should be just to juggle which Route is mounted based on the current location. The component is actually instantiated into an element by the render function of the Switch's container. cloneElement is only used as a trick to inject some props into an already instantiated component. Otherwise, we'd just return the appropriate Route, mounting it onto the render tree.

random-stranger · 2017-02-22T18:42:24Z

@timdorr it's a common pattern to fetch data in componentDidMount(), it will also trigger on every route change. Same goes for render(), no way to prevent re-rendering.

timdorr · 2017-02-22T19:35:12Z

Sorry, still not getting it (I'm trying!). What's the problem with that? If I've got some user profile page up, navigate away, the user posts some new content, and navigate back to the profile page, I would want to see that new content. So a fresh fetch from the lifecycle method would be good, no?

vladshcherbin · 2017-02-22T19:55:47Z

@timdorr in my use-case the component needs to be persistent across several routes.

When you navigate to /users, you can see UsersIndex with a list of users.
When you navigate to /users/create, you can see the same UsersIndex list of users and UsersCreate form (modal window).

So, when you navigate across this routes, UsersIndex is always visible and has to be mounted and fetched only once. When a new user is added, the users list is automatically updated, no need to re-fetch (from redux).

Makes sense now? :)

timdorr · 2017-02-22T21:15:15Z

Sort of. Why are you using a Switch for that? Just have a non-exact Route for the list:

<Route path="/users" component={UserIndex} />
<Route path="/users/create" component={UserCreate} />

That would do the same thing.

vladshcherbin · 2017-02-22T21:18:45Z

@timdorr this will render UsersIndex on all /users routes, even on /users/not/found/route.

I want to render NotFound route on all undefined routes.

vladshcherbin · 2017-02-22T21:29:35Z

As I mentioned above, Switch can be replaced with an array of paths, passed to Route, but currently it's not supported (but working).

<Route exact path={['/users', '/users/create']} component={UsersIndex} />

ryanflorence · 2017-02-23T00:29:56Z

@timdorr I consider this a bug.

@vladshcherbin Please let's not talk about array paths in here, that's a totally different conversation :)

mjackson · 2017-02-23T05:09:28Z

Hmm, maybe we don't use cloneElement? In any case, I'd love to see a failing test for this. Makes it a lot easier to fix.

ryanflorence · 2017-02-23T06:45:47Z

cloneElement shouldn't cause a remount, I was just looking at the code and I'm a bit stumped

agundermann · 2017-02-23T17:39:55Z

It's React.Children.toArray that causes this by setting the keys of the <Route>s.

Returns the children opaque data structure as a flat array with keys assigned to each child. Useful if you want to manipulate collections of children in your render methods, especially if you want to reorder or slice this.props.children before passing it down.

(docs)

Not sure exactly why this was added. If it's needed for flattening or sth, overwriting the key in cloneElement seems to work as well.

Anyway, I think there are valid arguments for both behaviors, both in terms of how you'd initially expect it to work and in terms of what you want to do with it. Though it seems that with the suggested behavior, one could force the remount by themselves, which is not possible the other way around, e.g.:

<Switch>
  <Route render={() => <Comp key="1" />} />
  <Route render={() => <Comp key="2" />} />
</Switch>

ryanflorence · 2017-02-23T18:19:44Z

Thanks @taurose your example:

<Switch>
  <Route render={() => <Comp key="1" />} />
  <Route render={() => <Comp key="2" />} />
</Switch>

feels more intuitive to me

timdorr · 2017-02-23T18:19:53Z

Here's a test case:

Hiding the bad test in the HTML...

But I must be doing something wrong, because it passes...

random-stranger · 2017-02-23T18:28:19Z

@timdorr I guess it is:

ReactDOM.render((
  <Router history={history}>
    <div>
      <Switch>
        <Route exact path="/" component={App} />
        <Route exact path="/foo" component={App} />
      </Switch>
      <Route path="/foo" component={() => <div />}/>
    </div>
  </Router>
), node)

timdorr · 2017-02-23T18:38:10Z

Oh, I'm totally missing things. The App component should not be remounted at all when switching between two component'ed Routes. I thought it was remounting too much just in general (not when going between two unique Routes). My bad...

Let me work that into the test...

timdorr · 2017-02-23T18:41:41Z

OK, here's a failing test:

  it('does not remount a route', () => {
    const node = document.createElement('div')
    const history = createMemoryHistory({ initialEntries: ['/foo'] })

    let mountCount = 0

    class App extends React.Component {
      componentWillMount() { mountCount++ }
      render() { return <div /> }
    }

    ReactDOM.render((
      <Router history={history}>
        <Switch>
          <Route path="/foo" component={App}/>
          <Route path="/bar" component={App}/>
        </Switch>
      </Router>
    ), node)

    expect(mountCount).toBe(1)
    history.push('/bar')
    expect(mountCount).toBe(1)
    history.push('/foo')
    expect(mountCount).toBe(1)
  })

And changing the top of Switch's render to this fixes it:

    const { children:routes } = this.props
    const { location } = this.state

Sorry for being dense earlier!

mjackson · 2017-02-23T21:35:09Z

It's React.Children.toArray that causes this

Nailed it, @taurose. Thanks :)

wmertens · 2017-03-03T16:39:33Z

@mjackson Can we get a release for this one?

mjackson · 2017-03-03T18:10:26Z

@wmertens Just released this morning in beta 7

wmertens · 2017-03-03T22:37:09Z

I am now running beta7 but I'm still seeing my component being remounted. Any quick tips to find the responsible parent?

vladshcherbin · 2017-03-04T08:14:20Z

@wmertens it's probably re-rendering due to props change on route change, not re-mounting.

I can confirm there is no bug in beta 7.

Thank you, guys! 👍

wmertens · 2017-03-04T12:39:27Z

No, the constructor is getting called, and I have to figure out which parent is responsible…

…

On Sat, Mar 4, 2017, 9:14 AM Vlad Shcherbin ***@***.***> wrote: I can confirm there is no bug in beta 7. @wmertens <https://github.com/wmertens> it's probably re-rendering due to props change on route change, not re-mounting. Thank you, guys! 👍 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4578 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADWllyPAsyLz-0UGy2rvcw6Hx6uRKEUks5riR1ngaJpZM4MIqcN> .

wmertens · 2017-03-04T14:59:47Z

Found it in code of my own (merge mishap). All I had to do was put a breakpoint in Reacts mountComponent and make it conditional on truthy doConstruct. The first component to hit the breakpoint has the culprit parent. Will remember that one, useful.

  mountComponent: function (transaction, hostParent, hostContainerInfo, context) {
[...]
    // Initialize the public class
    var doConstruct = shouldConstruct(Component);
/*=> here*/    var inst = this._constructComponent(doConstruct, publicProps, publicContext, updateQueue);

VaishaliJain0209 · 2017-08-11T10:28:03Z

I am a bit new to React & React-Router. I am creating a SPA where I do not want components to be unmounted when I switch b/w routes. I am using 4.1.2 version of React-Router but ComponentDidMount is being called every time when I return to a mounted component after switching the route. Could you please suggest the version I should use.

wmertens · 2017-08-11T12:34:02Z

Try looking at the unmount/remounts via the timeline as explained in https://facebook.github.io/react/blog/2016/11/16/react-v15.4.0.html - see which component stays mounted, that is the one responsible for remounting the rest

…

On Fri, Aug 11, 2017 at 12:28 PM VaishaliJain0209 ***@***.***> wrote: I am a bit new to React & React-Router. I am creating a SPA where I do not want components to be unmounted when I switch b/w routes. I am using 4.1.2 version of React-Router but ComponentDidMount is being called every time when I return to a mounted component after switching the route. Could you please suggest the version I should use. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4578 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADWlmbpq8woeyih0jKG216HY5bcUhRIks5sXCzCgaJpZM4MIqcN> .

katiepeters · 2017-08-17T18:26:11Z

This still seems to be a problem if Switch is given an array of Route components (because of the key prop?).

So this doesn't cause a re-mount:

<Switch>
	<Route
		component={App}
		exact
		path="/"
	/>
	<Route
		component={App}
		exact
		path="/other"
	/>
</Switch>

But this does:

<Switch>
	{[
		<Route
			component={App}
			exact
			key="1"
			path="/"
		/>,
		<Route
			component={App}
			exact
			key="2"
			path="/other"
		/>,
	]}
</Switch>

Is that intentional/am I missing something?

wmertens · 2017-08-17T19:24:32Z

What happens if you don't pass a key, or pass the same key twice? And what are the circumstances of the remounting?

…

On Thu, Aug 17, 2017 at 8:26 PM Katie Peters ***@***.***> wrote: This still seems to be a problem if Switch is given an array of Route components (because of the key prop?). So this doesn't cause a re-mount: <Switch> <Route component={App} exact path="/" /> <Route component={App} exact path="/other" /> </Switch> But this does: <Switch> {[ <Route component={App} exact key="1" path="/" />, <Route component={App} exact key="2" path="/other" />, ]} </Switch> Is that intentional/am I missing something? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4578 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADWlo8e0jneofN5jNBumYcZDtlf8iGuks5sZIXkgaJpZM4MIqcN> .

pshrmn · 2017-08-17T19:34:41Z

@katiepeters Yes, React's reconciler will always unmount the existing component and mount a new one when two element's keys are different.

Edit Woops, I slightly misspoke. This behavior is not actually caused by the reconciler directly. This is the function that determines whether to update the existing component or unmount/mount: https://github.com/facebook/react/blob/master/src/renderers/shared/shared/shouldUpdateReactComponent.js#L25-L43

katiepeters · 2017-08-17T19:36:52Z

Ya, I ended up passing them the same key to get it to work. Wasn't sure if that was ideal, but maybe it's ok since Switch only renders one Route at a time

artywhite · 2017-08-21T17:32:10Z

@wmertens , @pshrmn , @timdorr please take a look on this case with react-router-config here: #5430

KaroseLiu · 2017-08-30T02:26:26Z

I'm not sure, this commit fixed the bug, I upgraded the latest version v4.2.0 as your release, and try it, the component also remount.

Here is my layout:

const Layout = ({children, location}) => {
  // url like /:personId/a, /:personId/b
  const personId = location.pathname.split('/')[2]
  // b or a path
  const activeKey = location.pathname.split('/')[3]
  // The problem is every time the path changed, the Profile always componentDidMount.
  return <div>
    <Profile personId={personId}/>
    <Tabs
      type="card"
      activeKey={location.pathname.split('/')[3]}
      onTabClick={(key) => {
        const paths = location.pathname.split('/')
        history.push(`/person/${personId}/${key}`)
      }}
    >
      {tabs.map(tab =>
        (<TabPane tab={tab.title} key={tab.key}>
          {children}
        </TabPane>),
      )}
    </Tabs>
  </div>
}

The below is the Router config:

const routes = [
  {
    path: '/a',
    component: A,
  },
  {
    path: '/b',
    component: B,
  },
  {
    path: '/c',
    component: C,
  },
]
const Router = ({ match }) => (
  <Switch>
    {
      routes.map(({ path, exact, strict, component: Comp }) =>
        (<Route
          key={path}
          path={`${match.url}${path}`}
          exact={exact}
          strict={strict}
          render={
            props => (
              <Layout {...props}>
                <Comp {...props} />
              </Layout>
            )
          }
        />),
      )
    }
  </Switch>
)

Every time the path change, the component will didMount again, so maybe my usage is the incorrect way to reuse the layout?

TimoRuetten · 2017-10-05T13:21:54Z

I am causing the same problem. Any updates to this ?

    <Switch>
      <Route
        path="/:param"
        component={App}
      />
      <Route
        path="/:param/level-2"
        component={App}
      />
      <Route
        path="/:param/level-2/level-3"
        component={App}
      />
    </Switch>

This code will causes always a remount (not update of Component - I am talking about a remount) so everytime the url changed the componentWillMount get fired.

wmertens · 2017-10-05T13:37:55Z

@TimoRuetten that is very weird, can you share your whole code? This one looks too simplified…

TimoRuetten · 2017-10-05T13:46:19Z

@wmertens Yeah - its a bit simplified - but at the end not that much. This is the current code of our <AppRouter /> which is the first rendered Main-Component after all Provider we are using:

export default () => (
  <div>
    <Switch>
      <Route
        path="/:langCode"
        component={App}
      />
      <Route
        path="/:langCode/p/:nodeSlug/:nodeId"
        component={App}
      />
      <Route
        path="/:langCode/l/:regions*"
        component={App}
      />
    </Switch>
  </div>
);

Then we have in our <App /> a simple log in componentWillMount which gets always fired when clicking on a <Link /> link.

edit Using version 4.2.0

wmertens · 2017-10-05T13:54:03Z

Are you sure it's the Switch and not something higher up? You can also put ?react_perf in your url and then you can use a Performance recording to see unmounts in the user events

TimoRuetten · 2017-10-05T14:11:25Z

Hey @wmertens - You were right, I'm sorry. A HOC of my <App /> produced this problem! Sorry for the confusion and thanks for the fast response & help!

wmertens · 2017-10-05T14:51:38Z

@KaroseLiu you should give your Routes all the same key…

julienvincent · 2017-10-07T15:11:16Z

I still seem to be getting this issue (v4.2.2). I have confirmed that I have no HOC's or anything else affecting my app:

class T extends Component {
  componentWillUnmount() {
    console.log("unmounting")
  }

  render() {
    return null
  }
}

...

<BrowserRouter>
  <div>
    <Switch>
      <Route path="/a" key="1" /*or no key*/ children={() => <T />}
    </Switch>
  </div>
</BrowserRouter>

Updating the path results in unmounting getting logged

agundermann · 2017-10-08T14:22:05Z

This snippet works as expected without any unnecessary remounts, so the problem must be somewhere else.

theenoahmason · 2017-10-11T16:26:08Z

for anyone searching here, I am not getting remounts on switch, UNLESS switch is wrapped with react transition group's TransitionGroup/CSSTransition components.

correction, I believe my above comment is only an issue when passing a location to switch, which react transition group requires.

It seems animating a switch with react transition group requires a location for the switch component, which in turn is causing a remount on the route's called component.

wmertens · 2017-10-11T17:22:40Z

You can also forego the switch and make sure only one route matches at a time…

…

On Wed, Oct 11, 2017, 6:32 PM Noah Mason ***@***.***> wrote: correction, I believe my above comment is only an issue when passing a key to switch, which react transition group requires. Itb seems animating a switch with react transition group requires a location for the switch component, which intern is causing a remount on the routes called component. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4578 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADWlgvDvEf1g5Ee6rgh2phKLBzS_PPYks5srO2OgaJpZM4MIqcN> .

timdorr mentioned this issue Feb 23, 2017

Reuse components in Switch #4592

Merged

timdorr added the bug label Feb 23, 2017

timdorr added this to the v4.0.0 milestone Feb 23, 2017

timdorr closed this as completed Feb 24, 2017

ryanflorence mentioned this issue Feb 25, 2017

How do you create a Master Detail page? (v4) #3928

Closed

pshrmn mentioned this issue Mar 1, 2017

Unexpected re-rendering when back button is pressed with nested <Switch> components #4611

Closed

artywhite mentioned this issue Aug 12, 2017

Prevent remount component on routes with the same component #5430

Merged

wmertens mentioned this issue Oct 5, 2017

Switch is prone to remounts #5591

Closed

remix-run locked and limited conversation to collaborators Nov 17, 2017

remix-run deleted a comment from hainessss Nov 17, 2017

sbyps referenced this issue in KaroseLiu/planet Dec 7, 2017

Add the ant css config in webpack

fb7363f

Switch is re-mounting the same component on route change #4578

Switch is re-mounting the same component on route change #4578

Comments

random-stranger commented Feb 22, 2017 • edited Loading

Version

Test Case

Steps to reproduce

Expected Behavior

Actual Behavior

pshrmn commented Feb 22, 2017

timdorr commented Feb 22, 2017

random-stranger commented Feb 22, 2017

timdorr commented Feb 22, 2017

vladshcherbin commented Feb 22, 2017 • edited Loading

timdorr commented Feb 22, 2017

vladshcherbin commented Feb 22, 2017 • edited Loading

vladshcherbin commented Feb 22, 2017 • edited Loading

ryanflorence commented Feb 23, 2017

mjackson commented Feb 23, 2017

ryanflorence commented Feb 23, 2017

agundermann commented Feb 23, 2017 • edited Loading

ryanflorence commented Feb 23, 2017 • edited Loading

timdorr commented Feb 23, 2017 • edited Loading

random-stranger commented Feb 23, 2017 • edited Loading

timdorr commented Feb 23, 2017

timdorr commented Feb 23, 2017 • edited Loading

mjackson commented Feb 23, 2017

wmertens commented Mar 3, 2017

mjackson commented Mar 3, 2017

wmertens commented Mar 3, 2017

vladshcherbin commented Mar 4, 2017 • edited Loading

wmertens commented Mar 4, 2017 via email

wmertens commented Mar 4, 2017

VaishaliJain0209 commented Aug 11, 2017

wmertens commented Aug 11, 2017 via email

katiepeters commented Aug 17, 2017

wmertens commented Aug 17, 2017 via email

pshrmn commented Aug 17, 2017 • edited Loading

katiepeters commented Aug 17, 2017

artywhite commented Aug 21, 2017

KaroseLiu commented Aug 30, 2017

TimoRuetten commented Oct 5, 2017 • edited Loading

wmertens commented Oct 5, 2017

TimoRuetten commented Oct 5, 2017 • edited Loading

wmertens commented Oct 5, 2017

TimoRuetten commented Oct 5, 2017

wmertens commented Oct 5, 2017

julienvincent commented Oct 7, 2017 • edited Loading

agundermann commented Oct 8, 2017

theenoahmason commented Oct 11, 2017 • edited Loading

wmertens commented Oct 11, 2017 via email

random-stranger commented Feb 22, 2017 •

edited

Loading

vladshcherbin commented Feb 22, 2017 •

edited

Loading

vladshcherbin commented Feb 22, 2017 •

edited

Loading

vladshcherbin commented Feb 22, 2017 •

edited

Loading

agundermann commented Feb 23, 2017 •

edited

Loading

ryanflorence commented Feb 23, 2017 •

edited

Loading

timdorr commented Feb 23, 2017 •

edited

Loading

random-stranger commented Feb 23, 2017 •

edited

Loading

timdorr commented Feb 23, 2017 •

edited

Loading

vladshcherbin commented Mar 4, 2017 •

edited

Loading

pshrmn commented Aug 17, 2017 •

edited

Loading

TimoRuetten commented Oct 5, 2017 •

edited

Loading

TimoRuetten commented Oct 5, 2017 •

edited

Loading

julienvincent commented Oct 7, 2017 •

edited

Loading

theenoahmason commented Oct 11, 2017 •

edited

Loading