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

Nested <Suspense> boundaries do not work with useTransition #24759

Closed
jezzgoodwin opened this issue Jun 20, 2022 · 7 comments
Closed

Nested <Suspense> boundaries do not work with useTransition #24759

jezzgoodwin opened this issue Jun 20, 2022 · 7 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@jezzgoodwin
Copy link

React version: 18

Steps To Reproduce

  1. Create a component tree which contains nested <Suspense> boundaries
  2. Reveal this component tree using useTransition

Link to code example: https://codesandbox.io/s/inspiring-wind-1bedy7

The current behavior

useTransition waits for the first Suspense boundary to be ready and then reveals showing the nested Suspense boundary still in its fallback state.

The expected behavior

useTransition should wait for the entire tree to be ready before revealing

Thanks React team for the great work by the way!

@jezzgoodwin jezzgoodwin added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jun 20, 2022
@gaearon
Copy link
Collaborator

gaearon commented Jun 20, 2022

This behavior is intentional.

useTransition should wait for the entire tree to be ready before revealing

This is how it worked in very early prototypes but we've changed the behavior. In the stable release, it only "waits" for "existing" boundaries — but it won't wait for the "new" ones. In other words, wrapping a piece of content that delays a transition into Suspense lets you decouple it from the transition, and let it finish later.

@gaearon gaearon closed this as completed Jun 20, 2022
@jezzgoodwin
Copy link
Author

Thanks for the quick reply. Are they maybe any plans to make this behaviour more flexible?

I'm working on a system with dashboard pages. The dashboard page itself uses suspense to load the page state/what widgets are configured etc. And then as a second level there are widgets which each have suspense wrappers.

It would be great if I could use startTransition to switch to a different dashboard with the widgets displaying data, but with the current solution the page switch just takes me to a page with loading spinners on each of the widgets.

@gaearon
Copy link
Collaborator

gaearon commented Jun 20, 2022

What we've seen with the old behavior in practice is that a single mistake (adding a slow data source somewhere on the page) causes regressions to many interactions (all transitions to that page) without a clear way to fix it. Whereas with the current behavior, you have control: if you don't want to wait for something, you wrap it in Suspense. I would suggest to think about two questions:

  1. Why is it a problem that moving to a page shows the spinners? Is it because there are many of them? In that case, move the Suspense boundaries from individual dashboard items to wrap all of them. (In the future, you would also be able to control the order in which they reveal using SuspenseList.)
  2. If it's completely undesirable then you can remove those Suspense boundaries altogether, and put one above the place where you start the transition. Then it will be an "existing" boundary and won't switch to fallback.

If neither of these approaches work well for you, can you share why?

@jezzgoodwin
Copy link
Author

That's an interesting point about performance. I guess at the moment I've been primarily been focusing on the transition effect and assuming that if performance was really bad the transition would timeout. (Luckily at the moment our apis are nice and fast, but as you say it only takes one). Re your questions:

  1. It's not a major problem. Having the page show with the widgets in place is better than a brief single spinner in the middle of the page. But yes lots of widgets means lots of spinners!
  2. If I remove the suspense boundary from the widgets themselves it means when adding a new widget the whole page would resuspend. I could use a solution where boundaries only get placed on new widgets but this doesn't feel very clean.

I guess in an ideal world I'm imagining being able to mark specific suspense boundaries as 'ignore when transitioning' and also, re performance, perhaps customize the transition timeout time if React's default time feels a bit slow. Overall though, transitions are better than what we've had before, so I can't complain.

@gaearon
Copy link
Collaborator

gaearon commented Jun 20, 2022

We've removed the concept of "timeouts" because there was no good way to pick one (three seconds? five seconds? ten seconds?) and the overall UI effect felt very non-determinisic (a sudden jump to a spinner.. but unclear why since the user didn't do anything different). So now it's either "everything reveals asap*" (for new boundaries) or "everything stays pending".

(* — not actually asap because we throttle "revealing the next level of fallbacks" every 500ms to avoid flashes of different levels of spinners)

Having the page show with the widgets in place is better than a brief single spinner in the middle of the page.

You can design an intentional glimmer that approximates all the widgets together but is not actually a bunch of spinners. I think overall that's preferred to spinners anyway, and many UIs are moving to that style these days.

Eg Vercel dashboard:

Screenshot 2022-06-20 at 17 27 40

If I remove the suspense boundary from the widgets themselves it means when adding a new widget the whole page would resuspend.

It wouldn't if you wrap "adding a widget" into a transition.

@jezzgoodwin
Copy link
Author

Ah that is all very interesting. Clearly my understanding of suspense/transitions is outdated - I didn't realise there are no timeouts anymore. That does make a big difference as to what I'm thinking of loading behind transitions. Your explanation about non-determinism does make sense though. I think I need to play around with a simulated slow user experience for a tab switch to see if removing widget boundaries is a good idea.

It wouldn't if you wrap "adding a widget" into a transition.

Cunning!

Yes we should move to glimmers to have the best experience. We actually use delay/fade spinners so it doesn't look quite as busy (assuming performance is good). And your comment earlier about the upcoming SuspenseList should definitely help with the overall transition as widgets are ready.

Thanks for your detailed replies.

@gaearon
Copy link
Collaborator

gaearon commented Jun 20, 2022

No prob! In general, our conclusion was that for navigations you want to get to the "skeleton" of the next page as soon as possible (to communicate the navigation is happening), but for "refreshing" the existing content, you want to use a transition. And then where you place the Suspense boundaries lets you adjust the behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

2 participants