Suspense fallback remounts when each child resolves

[adwilk]

Do you want to request a feature or report a bug?
Not sure if its a bug or a feature request.

What is the current behavior?
The component given as a fallback to Suspense is remounted each time a lazy child resolves.

https://codesandbox.io/s/z6v6x3n1np
This example shows how a fallback component, which counts up, is mounted more than once.

What is the expected behavior?
The fallback is mounted once and is only unmounted when all lazy children are resolved.
In the example the counter would run up to 9.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
react@16.6.0

[arianon]

Looks like expected behavior to me, when LazyText1 resolves, the Suspense component tries to render its children once again, but then LazyText2 throws another promise, so it renders the fallback again, and so on. Nope, all three promises are thrown in parallel, but every time a promise resolves, Suspense attempts to re-render the children, but seeing that some children are unresolved, it renders the fallback, until of course, all the promises are resolved.

However, in Concurrent Mode the observed behaviour aligns with your expected behaviour, with one apparent bug: the children don't render when LazyText3's delay is over 8000 milliseconds, on my machine at least. I'm not sure what's happening, but I'll try to look into this further.

[adwilk]

Here is a slightly less contrived example than what I shared above
https://codesandbox.io/s/2v8l3vn67p

Here there is a CSS loading spinner, due to the remounting of the fallback component the animation of the spinner is not smooth.

[gaearon]

Seems like a bug.

[gaearon]

I added a failing test in #14078. Not sure if this is a bug or unavoidable quirk in sync mode.

[adwilk]

Thanks 👍

[OndeVai]

Has this been fixed? I'm seeing the same issue now with React 17.0.1

[gaearon]

That one was fixed, so you're seeing something different. File a bug with a minimal repro case.

[OndeVai]

Actually, in my case, I have a Suspense (call it SuspenseB) nested several levels below another Suspense (call it SuspenseA). For SuspenseA still renders its fallback UI when an async component is loaded within SuspenseB, but only if I do not provide a fallback for SuspenseB. In my case I did not want any fallback UI for B at all. So, I ended up providing a {null} fallback and everything worked as desired.

Is what I just described by design? If not, I can go ahead and write up another bug...

[gaearon]

Hmm. I'm not sure I understand what the expected and the actual behavior was.

[OndeVai]

@gaearon - The expected behavior (for me, at least) was to not show the fallback from SuspenseA, since the direct child components of SuspenseA were already loaded.

So (psuedoish React code):

<Suspense fallback={<BigPageLoader/>}> //-->SuspenseA
   <AysncComponentA>
       <Suspense> //-->SuspenseB
          <AysncComponentB>..</AysncComponentB> //-->shows the fallback of SuspenseA, unless fallback={null or other} provided in SuspenseB
       </Suspense>
   </AysncComponentA>
</Suspense>

In my case, showing SuspenseA's fallback when AysncComponentB was loaded was unexpected. I assumed that since AysncComponentA already loaded there would be no more fallback showing at that level.

(I can create a CodeSandbox if you'd like, just pressed for time today...)

[gaearon]

I think the issue here is that Suspense without a fallback or with fallback={undefined} currently gets ignored completely. And you seem to have expected that it acts as a null fallback.

We'll need to revisit the behavior (maybe it should always warn and tell you to specify it). But the idea definitely isn't that fallback={null} is a good default. It's not because it's "unintentional" — you'll just see your page jumping when the content loads. Ideally you'd provide an intentionally designed loading state.

[OndeVai]

@gaearon - Thanks for the quick reply.

I would expect SuspenseB to behave like SuspenseA for its child components. So, if I don't provide a fallback, don't show one. IMO, React can't really make too many assumptions on what the best practices for loading states are for individual projects ;)

[gaearon]

If that's the behavior you want, you can always pass fallback={null}. I am simply saying that there is no "default" fallback, you should always specify one. The fact that not passing it accidentally works seems like an unintentional omission on our side.

[OndeVai]

It might be nice to just call out the behavior in the React official docs.

[gaearon]

Yeah. The undefined fallback thing seems like it's accidental so we'll revisit that.