Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible bug with <Switch/> in beta.7? #4661

Closed
donaldpipowitch opened this issue Mar 8, 2017 · 7 comments
Closed

Possible bug with <Switch/> in beta.7? #4661

donaldpipowitch opened this issue Mar 8, 2017 · 7 comments

Comments

@donaldpipowitch
Copy link

Version

4.0.0-beta.7

Test Case

I use webpack and react-router for code splitting and lazy loading parts of my page. My component to lazy load looks like this.

And I use it like this:

// this is our single page application
import React from 'react';
import { render } from 'react-dom';
import { Router, Route, Switch} from 'react-router-dom';
import { Lazy } from 'react-lazy';  // package defined above
import { createHashHistory } from 'history';
//import { App } from './App'; // our app component - now loaded lazily!

const hashHistory = createHashHistory();

render(
  <Router history={hashHistory}>
    <Switch>
      <Route exact path="/" render={() => <Lazy fetch={() => _import('./App')} />} />
      {/* other routes */}
    </Switch>
  </Router>,
  document.getElementById('app')
);

This worked in beta.6 and it works in beta.7 without <Switch/> around my <Route/>'s. I thought it was a problem with blocked-updates.md, because only the first route which lazy loads a component is displayed correctly. Navigation through my app doesn't update correctly. But the render method of my <Lazy/> component is always called correctly and no matter how I force updating with passing location through props, it doesn't work (because render is called anyway...). It looks more like <Switch/> doesn't display the the newly rendered part.

Strange enough... it works when I use <Lazy/> just once:

      <Switch>
        <Route
          path="/foo"
          render={() => <Lazy fetch={() => _import('./foo')} />}
        />
        <Route
          path="/bar"
          render={() => <p>Bar</p>}
        />
      </Switch>

And it of course it works without it:

      <Switch>
        <Route
          path="/foo"
          render={() => <p>Foo</p>}
        />
        <Route
          path="/bar"
          render={() => <p>Bar</p>}
        />
      </Switch>

But this fails:

      <Switch>
        <Route
          path="/foo"
          render={() => <Lazy fetch={() => _import('./foo')} />}
        />
        <Route
          path="/bar"
          render={() => <Lazy fetch={() => _import('./bar')} />}
        />
      </Switch>

As well as this:

      <Switch>
        <Route
          path="/foo"
          render={({ location }) => <Lazy location={location} fetch={() => _import('./foo')} />}
        />
        <Route
          path="/bar"
          render={({ location }) => <Lazy location={location} fetch={() => _import('./bar')} />}
        />
      </Switch>

Or even this nightmare 😄:

      <Route render={({ location }) => <Switch>
        <Route
          path="/foo"
          render={({ location }) => <Lazy location={location} fetch={() => _import('./foo')} />}
        />
        <Route
          path="/bar"
          render={({ location }) => <Lazy location={location} fetch={() => _import('./bar')} />}
        />
      </Switch> } />
@donaldpipowitch
Copy link
Author

donaldpipowitch commented Mar 8, 2017

It looks like componentWillMount is just called once. Is this because of cloneElement in <Switch/>? It looks like my <Lazy/> component is re-used? (Probably not as it was used in beta.6, too.)

@donaldpipowitch
Copy link
Author

donaldpipowitch commented Mar 8, 2017

It looks like I do need <Route component> here (see #4658, too). This works:

      <Switch>
        <Route
          path="/foo"
          component={() => <Lazy fetch={() => _import('./foo')} />}
        />
        <Route
          path="/bar"
          component={() => <Lazy fetch={() => _import('./bar')} />}
        />
      </Switch>

If this is the canonical way to solve my problem, feel free to close this issue. I was under the impression that I suffered from blocked-updates.md, but I guess I need side-effects in mount-related lifecycle hooks.

However I'm surprised that writing my example this way results in <Lazy/> being re-used instead of having two different examples.

      <Switch>
        <Route
          path="/foo"
          render={() => <Lazy fetch={() => _import('./foo')} />}
        />
        <Route
          path="/bar"
          render={() => <Lazy fetch={() => _import('./bar')} />}
        />
      </Switch>

@pshrmn
Copy link
Contributor

pshrmn commented Mar 8, 2017

If your <Lazy> component had a cDU lifecycle method, you would see that it is called on subsequent renders. Add a key prop to your <Lazy> components and it should work as expected.

<Route path='/quux' render={() => <Lazy key='quux' fetch={...}/> }/>

@mjackson
Copy link
Member

mjackson commented Mar 8, 2017

Thanks for the response @pshrmn

@mjackson mjackson closed this as completed Mar 8, 2017
@donaldpipowitch
Copy link
Author

@pshrmn Thanks. This works. But it works a little bit unexpected for me.

I don't want to sound bad - you all do a great job - but this seems somewhat tedious to me. Inside <Switch/> I always just want to render one route. This only make sense if the path (and maybe in combination with exact) is always unique. If I would have a supported way to define custom <Route/>s inside <Switch/> (coming back to #4576). I could create a <LazyRoute/> or something like that which would generate a unique key based on path and exact automatically.

But anyway, thank you.

@pshrmn
Copy link
Contributor

pshrmn commented Mar 8, 2017

<Switch> is a very simple component that iterates over its children elements. If you need to support custom routes, then the best solution would probably be to implement your own <Switch>-like component.

For example, I created a <ConfigSwitch> component. It has the same effect as <Switch>, but I prefer to explicitly define the array of route objects.

@donaldpipowitch
Copy link
Author

Cool, thank you for the link and your support! ❤

@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants