<Switch> does not update inside <Suspense>

[overlookmotel]

Version

5.1.2 (not tested on previous versions)

Test Case

https://codesandbox.io/s/react-router-t0ig4

Steps to reproduce

Please see above CodeSandbox.

Expected Behavior

Using React Router with Suspense/lazy.

const Home = () => <div>I am not lazy loaded</div>;
const About = React.lazy( () => import('./About.js') );

const App = () => (
  <Router>
    <div>
      <Link to="/">Home</Link>
      <Link to="/about">About</Link>
    </div>
    <Suspense fallback="Loading...">
      <Switch>
        <Route exact path="/" component={Home} />
        <Route path="/about" component={About} />
      </Switch>
    </Suspense>
  </Router>
);

Home is a normal component, About is lazy-loaded.

When you click "About" link, "Loading..." should appear (it does).

Now, while About is still loading, if you click "Home" link, the page should update immediately and show you the "Home" page again.

Actual Behavior

In fact, the route transition back to "Home" is delayed until About has finished loading.

This is hard to spot in a dev environment, as About will load quickly anyway. So in the CodeSandbox above I've substituted a fake Lazy component which never loads. The result is that the page is stuck on "Loading..." forever - navigation ceases to work.

Two things solve the problem:

1. Add a useLocation() somewhere below the <Switch> element to force <Switch> to re-render on a route transition.
2. Remove the <Switch> entirely. If it's just a series of <Route> elements, it works as expected.

I have played around with this a lot, and can't figure out if it's a bug in React Router, or in React's handling of Context within Suspense. But I figured I'd post this here first.

[MeiKatz]

Actually, this is the expected behaviour of <Suspense />. You have to wrap each route inside of an own suspense component.

[overlookmotel]

@MeiKatz

Thanks for swift reply.

Sorry to contradict you, but I'm not clear that this is intended behavior.

Reimplementing Switch using a useContext hook rather than Context.Consumer does not exhibit the same behavior. Please see here:

https://codesandbox.io/s/react-router-yp42p

I wouldn't have expected that change should make any difference, if this was the intended behavior. But have you seen something to the contrary in React's docs somewhere? Am I missing the point?

Either way, having to wrap every route in its own Suspense rules out some nice patterns e.g.:

<Suspense fallback="Loading">
  <div>Page header</div>
  <Switch>
    <Route ... />
  </Switch>
</Suspense>

If you want to keep "Page header" hidden until the content of the route has loaded, that is not possible by wrapping each route in its own Suspense. You'd have to add "Page header" to every route.

[MeiKatz]

The point is, that as long as there is any child component of a <Suspense /> component that throws a Promise anything inside the Suspense component will be replaced by the component defined in the fallback prop. When you load a new component inside a switch that is not loaded yet, anything inside the Suspense component will be replaced, even the Switch component. Therefore you have to catch the thrown promise inside the Switch component, meaning you should do something like the following:

<Switch>
  <Route exact path="/">
    <Home />
  </Route>
  <Route path="/about">
    <Suspense fallback={<p>Loading ...</p>}>
      <About />
    </Suspense>
  </Route>
</Switch>

EDIT: I looked at your code sandbox example and I think I get your point now. I am not sure if we have to change anything in the implementation but your expected behaviour seem reasonable to me.

[overlookmotel]

OK, I see your point. However, can you offer any pointers as to why it doesn't work like that with a Switch implementation based on a useContext hook rather than Context.Consumer?

https://codesandbox.io/s/react-router-yp42p

By the way, I'm not trying to pick a fight. The exact semantics of Suspense I don't think are well explained in React's docs, so I'm interested in other peoples' interpretations.

I think it's an open question as to whether the Suspense content is "in" the <Suspense> or not when it's suspended. In React's internals it kind of is - the fiber representing the contents of the Suspense boundary remains attached to the Suspense element's fiber as a child, even when the Suspense is suspended. But is that how it's also meant to be conceptualized externally?

[overlookmotel]

Oh, I hadn't seen your edit when I wrote last comment.

So do you think this is a bug in React then that useContext and Context.Consumer work differently?

[MeiKatz]

@overlookmotel No, I don't think that it is a bug of React, but a not implemented feature of RR.

[timdorr]

I'm not seeing this behavior:

Home loads just fine. Are you using a different browser or something? I'm on Chrome.

[overlookmotel]

Hi @timdorr.

The example you're looking at is the one where I've re-implemented react-router's <Switch> to use useContext rather than Context.Consumer.

Please see the original test case for what happens with unaltered React Router:

https://codesandbox.io/s/react-router-t0ig4

[timdorr]

Ah, that's why. Duh.

Might be related to facebook/react#17356

[ljosberinn]

You do not have to wrap every component with Suspense. The code OP provided under the Expected Behaviour section is perfectly valid and would work as expected unless he hadn't reimplemented Switch himself.

https://codesandbox.io/s/trusting-turing-widun (PasswordReset is lazy)

[overlookmotel]

@ljosberinn Sorry, I don't quite understand what you mean by "would work as expected unless he hadn't reimplemented Switch" - the double-negative has confused me!

Are you saying it should have worked without reimplementing Switch. Or something else?

[ljosberinn]

@overlookmotel yes, precisely. Although @MeiKatz is correct in regard that a single Suspense above all routes replaces its children whilst suspended, so with many routes, it matters performance wise.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
You can add the fresh label to prevent me from taking any action.

[gaearon]

If you think there is a React bug please file (in React repo) a new issue with a reduced repro.

[whitelizard]

I'm curious about the status of this, and what the recommendations are if one wants to combine Switch with Suspense. I ran into this problem in a current project at work, and I agree that it doesn't act as expected.

[gaearon]

Does this still reproduce with React 17 RC?

[ljosberinn]

I think so: https://codesandbox.io/s/recursing-ritchie-vvozx?file=/src/App.js navigating to /about and back to / whilst the suspense fallback is shown still waits until loading is finished

@whitelizard wrap your routes with Suspense through a HOC? worked fine for us

[overlookmotel]

Sorry I've not produced a reproduction case without React Router. I tried but it got pretty confusing, and... time! I'll give it another go as soon as I get a chance. I had hoped facebook/react#19216 had solved it, but sounds like maybe not.

Give me a few days and I'll try to put something together.

[overlookmotel]

Just tried the original test case with React + ReactDOM 17.0.0-rc.0, and I'm afraid I can confirm the issue remains.

I've made a repro case without React Router facebook/react#19701.

While this issue remains outstanding in React, might it be worthwhile implementing a workaround in React Router?

The problem is the use of <Context.Consumer> in Switch - using a useContext() hook solves the problem.

This workaround could be implemented so it only kicks in with versions of React which support hooks. Actual implementation would be more complicated, but something along the lines of:

const {useContext} = React;
if (useContext) useContext( RouterContext );

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
You can add the fresh label to prevent me from taking any action.

[overlookmotel]

This issue remains current as far as I'm aware. No progress has been made with React fixing the bug on their side as yet. facebook/react#19701

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
You can add the fresh label to prevent me from taking any action.

[overlookmotel]

Same deal as on Oct 26th. This bug still remains.

[ccnklc]

I know that in React docs it is suggested the other way but what is the downside of putting suspense inside the switch?

function Content() {
  return (
    <Switch>
      <Suspense fallback={<div>Loading...</div>}>
        <Route exact path="/" component={Home} />
        <Route path="/about" component={About} />
      </Suspense>
    </Switch>
  )
}

[overlookmotel]

@ccnklc If I remember right, <Switch> requires its children to each have a path prop, so nesting <Suspense> inside <Switch> won't work - the <Route>s need to be directly inside <Switch>.

Also, the location of <Suspense> in the tree alters behavior. If you have multiple nested <Switch>es, you might want the Suspense boundary to enclose them all together.

[gaearon]

I think we fixed this in facebook/react#23095. Try testing it with the @next npm tag of react/react-dom tomorrow.

[gaearon]

Yea this is fixed. https://codesandbox.io/s/react-router-forked-vj12o?file=/index.js

[timdorr]

Thanks, inventor of React!

[gaearon]

right back at ya, creator of react router!

[MeiKatz]

Isn't the inventor of React Jordan Walke?

[timdorr]

Yes, but the running joke is that people mistake Dan for the creator of React when he actually created Redux (along with Andrew Clark).

[gaearon]

and tim didn’t create react router either!

[MeiKatz]

@gaearon I know :'D I never heard about that running joke. Next time I will do better in REACTing to this joke ;)