React: Suspense: timeout expiration and siblings rendering issues

[alexeyraspopov]

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

Code for reproducing is here. I've also deployed an example to Zeit Now: https://suspense-fpwoufdzfv.now.sh/.

This is a basic example of using React Suspense and Simple Cache Provider. Postponing text rendering and showing loading spinners when it's necessary.

I can see my use of <Timeout /> doesn't really care about ms I'm passing — it always become expired right after the render. You can open a page and the loading bar appears immediately despite 1000ms delay that it has.

There is content that is placed next to an async component (the one that's going to be suspended).

https://github.com/alexeyraspopov/react-suspense-sandbox/blob/cb3b60be69523de8f720a474c965face2b521b9b/src/index.js#L51-L56

Looking at React Suspense tests, it is assumed that sibling elements can be rendered in any way.

What is the expected behavior?

<Timeout /> component only shows placehold when expired, sibling content is shown even if an async component was suspended.

I built sources for react, react-dom, and simple-cache-provider from the current master, updating enableSuspense flag. I also used <unstable_AsyncMode /> but it didn't seem to make any difference.

I overall was really satisfied with this feature. I hope I did the code correctly so it shows the real bug.

[gaearon]

Hmm, I'm not sure it's supposed to work this way.

React won't "break" a tree in several parts because some of them are ready earlier. That would lead to very jumpy behavior if different siblings and children suspend for different periods of time. Preventing them from appearing until everything is ready is arguably the main point of this feature.

You can "break" it explicitly though—that's what <Timeout> is for. You can think of timeout as a "gate". When everything below it is ready, it displays the children. Otherwise it displays the fallback. So even only one of two siblings is ready, timeout will still show the placeholder for the whole thing.

If you want to go more granular you need to use nested timeouts. So that you explicitly say "try waiting for both for N, but if this particular subtree takes too long, don't wait for it and show a nested placeholder instead".

<Timeout>
  <AsyncThingA />
  <Timeout>
    <AsyncThingBThatDoesntPreventAFromShowing />
  </Timeout>
</Timeout>

Does this make sense?

[alexeyraspopov]

I initially assumed it may work based on what I saw in this test

https://github.com/facebook/react/blob/61777a78f6f9d90bea6ecdab4038675010bd70db/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js#L94-L140

But now I think I may have understood it incorrectly.

React won't "break" a tree in several parts because some of them are ready earlier. That would lead to very jumpy behavior if different siblings and children suspend for different periods of time. Preventing them from appearing until everything is ready is arguably the main point of this feature.

It does sound reasonable. Thanks for the tip for using Timeout as a "gate", I'll try to play with it more.

[alexeyraspopov]

@gaearon, I saw you've added question label. There is one more case described in the ticket, related to the fallback content that shows up too early. How can we verify that it is not a bug? From what I understand and what seems to be reasonable to me, fallback content will only show up after some delay if the necessary promise is not resolved yet.

[gaearon]

In that test note the difference between assertions on flush result (which correspond to logs from render methods that we emit with yield) and getChildren (which is what you see on the screen). Even if we call render method on sibling (to do CPU work eagerly) we don’t want to show it yet.

Regarding fallback showing up early, I think maybe it has to do with this being the initial render? I don’t recall exactly but I guess it has a higher priority. Try adding suspense to a state transition instead, and wrap it in unstable_deferredUpdates to make it a low priority.

[aweary]

Regarding fallback showing up early, I think maybe it has to do with this being the initial render? I don’t recall exactly but I guess it has a higher priority. Try adding suspense to a state transition instead, and wrap it in unstable_deferredUpdates to make it a low priority.

You could use ReactDOM.unstable_createRoot to make the tree async on the initial render as well

[alexeyraspopov]

Even if we call render method on sibling (to do CPU work eagerly) we don’t want to show it yet

Thanks for the clarification.

Regarding fallback showing up early, I think maybe it has to do with this being the initial render? I don’t recall exactly but I guess it has a higher priority. Try adding suspense to a state transition instead, and wrap it in unstable_deferredUpdates to make it a low priority.

It did work for me. I've implemented <Fallback /> component in they way that it doesn't render anything but triggers its state change on componentDidMount() where I can use unstable_deferredUpdates():

class Fallback extends React.Component {
  state = { shouldRender: false };

  componentDidMount() {
    ReactDOM.unstable_deferredUpdates(() => {
      this.setState({ shouldRender: true });
    });
  }

  render() {
    let { ms, placeholder, children } = this.props;
    return this.state.shouldRender ? (
      <React.Timeout ms={ms}>
        {(didExpire) => didExpire ? placeholder : children}
      </React.Timeout>
    ) : null;
  }
}

This is at least enough for my sandbox. Works as expected.

You could use ReactDOM.unstable_createRoot to make the tree async on the initial render as well

@aweary, I actually have tried to use it. My page has static content in the root and async content a bit deeper, but for some reasons the whole tree was rendering only after nested timeout's expired.

[alexeyraspopov]

@gaearon, sorry, forgot about the ticket, needed to close it earlier. The suggestions did help and I continue exploring the potential of Suspense. It is amazing so far.

@gaearon, @aweary, thank you for the responses, especially during the weekend. You're, guys, amazing!