Bug: Nested SuspenseList may display fallbacks while the component is loaded on re-render

[dubzzz]

React version: 0.0.0-experimental-e5d06e34b

Hoping this can be helpful for you. Here is what looks to be a bug with concurrent mode and nested SuspenseList.

Steps To Reproduce

In concurrent mode only

0. Setup

Let's suppose we have three components <A /> (not lazy loaded), <B /> (not lazy loaded) and <C /> (lazy loaded).

In other words:

import A from './A';
import B from './B';
const C = React.lazy(() => import('./C'));

1. Render

render(
  <SuspenseList key="1" revealOrder="forwards">
    <SuspenseList key="1.1" revealOrder="forwards">
      <Suspense key="1.1.a" fallback={<div>Loading A</div>}>
        <A />
      </Suspense>
    </SuspenseList>
  </SuspenseList>
)

2. Update the component (component now shows A, B, and C)

render(
  <SuspenseList key="1" revealOrder="forwards">
    <SuspenseList key="1.1" revealOrder="forwards">
      <Suspense key="1.1.a" fallback={<div>Loading A</div>}>
        <A />
      </Suspense>
      <Suspense key="1.1.b" fallback={<div>Loading B</div>}>
        <B />
      </Suspense>
      <Suspense key="1.1.c" fallback={<div>Loading C</div>}>
        <C />
      </Suspense>
    </SuspenseList>
  </SuspenseList>
)

3. Output is: A / Loading B / Loading C. While B has already been loaded (not lazy loaded). If I understand well the behaviour of forwards I would have expect to have A / B / Loading C instead.

Please note that the behaviour is not the same if I do not use nested <SuspenseList />.

CodeSandbox: https://codesandbox.io/s/mutable-rain-3ikor
GitHub pages repro: https://dubzzz.github.io/react-suspenselist-bug/build/
GitHub pages source code: https://github.com/dubzzz/react-suspenselist-bug

The current behavior

Output is: A / Loading B / Loading C with nested SuspenseList

The expected behavior

Output is: A / B / Loading C with nested SuspenseList

How did I found this bug?

This potential bug has been discovered while I was trying to run property based tests based on fast-check against React library.

See dubzzz@e2cb477

[gaearon]

Haven’t looked at the issue yet (and might not for a bit). But we need a lot stronger fuzz coverage for Suspense, SuspenseList, and priority scheduling. See the recent closed bugs with “concurrent mode” label for examples of how things go wrong and what catches them. If you’d like to contribute, coming up with a stronger fuzz and submitting failing tests would be much appreciated.

[gaearon]

For SuspenseList specifically I don’t remember what the semantics for updating are. There was something subtle there. So maybe this isn’t a bug per se.

[gaearon]

I agree with you this looks weird though. Can you submit an isolated failing case PR for now that’s manually written?

And then add fuzz separately.

[gaearon]

I also opened #18669 for the broader fuzz question.

[dubzzz]

Can you submit an isolated failing case PR for now that’s manually written?

Yes, I will open a specific PR only containing a failing unit test related to this potential bug.

And then add fuzz separately.

And then open PRs to discuss fuzzing on SuspenseList. And link them to #18669.

[sebmarkbage]

Updates are indeed strange. Nesting is also strange.

What happens here is that we have two rules that can yield counter intuitive results:

• A row is not allowed to show content until the previous row is done.
• A row is not allowed to show partial content until the whole row is done.
• Never hide already shown content until it explicitly resuspends, to avoid content disappearing.

What's happening here is that the inner Suspense list is also a row. We follow the rule: "A row is not allowed to show partial content until the whole row is done."

It gets extra confusing because A is still showing due to the rule "Never hide already shown content until it explicitly resuspends, to avoid content disappearing".

So this behavior is "intentional" according to the rules.

We might have the wrong heuristics but a real world use case would be helpful to inform how they should change.

When you have two Suspense boundaries in a row that aren't wrapped in a SuspenseList there has to be kind of an implicit list there to decide how they should work with regard to each other. You wouldn't want the last one to pop in before the first one. So a good default is "A row is not allowed to show partial content until the whole row is done". It is as if the list was wrapped in SuspenseList revealOrder="together". However, I could imagine a rule like if all boundaries are in a single SuspenseList in a row, then that list decides.