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

Don't prerender siblings of suspended component #26380

Merged
merged 2 commits into from
Mar 21, 2023

Commits on Mar 21, 2023

  1. Fork completeUnitOfWork into unwind and complete

    A mini-refactor to split completeUnitOfWork into two functions:
    completeUnitOfWork and unwindUnitOfWork. The existing function is
    already almost complete forked. I think splitting them up makes sense
    because it makes it easier to specialize the behavior.
    
    My practical motivation is that I'm going to change the "unwind" phase
    to synchronously unwind to the nearest Suspense/error boundary. This
    means we'll no longer prerender the siblings of a suspended tree. I'll
    address this in a subsequent step.
    acdlite committed Mar 21, 2023
    Configuration menu
    Copy the full SHA
    c9853e7 View commit details
    Browse the repository at this point in the history
  2. Don't prerender siblings of suspended component

    Today if something suspends, React will continue rendering the siblings
    of that component.
    
    Our original rationale for prerendering the siblings of a suspended
    component was to initiate any lazy fetches that they might contain. This
    was when we were more bullish about lazy fetching being a good idea some
    of the time (when combined with prefetching), as opposed to our latest
    thinking, which is that it's almost always a bad idea.
    
    Another rationale for the original behavior was that the render was I/O
    bound, anyway, so we might as do some extra work in the meantime. But
    this was before we had the concept of instant loading states: when
    navigating to a new screen, it's better to show a loading state as soon
    as you can (often a skeleton UI), rather than delay the transition.
    (There are still cases where we block the render, when a suitable
    loading state is not available; it's just not _all_ cases where
    something suspends.) So the biggest issue with our existing
    implementation is that the prerendering of the siblings happens within
    the same render pass as the one that suspended — _before_ the loading
    state appears.
    
    What we should do instead is immediately unwind the stack as soon as
    something suspends, to unblock the loading state.
    
    If we want to preserve the ability to prerender the siblings, what we
    could do is schedule special render pass immediately after the fallback
    is displayed. This is likely what we'll do in the future. However, in
    the new implementation of `use`, there's another reason we don't
    prerender siblings: so we can preserve the state of the stack when
    something suspends, and resume where we left of when the promise
    resolves without replaying the parents. The only way to do this
    currently is to suspend the entire work loop. Fiber does not currently
    support rendering multiple siblings in "parallel". Once you move onto
    the next sibling, the stack of the previous sibling is discarded and
    cannot be restored. We do plan to implement this feature, but it will
    require a not-insignificant refactor.
    
    Given that lazy data fetching is already bad for performance, the best
    trade off for now seems to be to disable prerendering of siblings. This
    gives us the best performance characteristics when you're following best
    practices (i.e. hoist data fetches to Server Components or route
    loaders), at the expense of an already bad pattern a bit worse.
    
    Later, when we implement resumable context stacks, we can reenable
    sibling prerendering. Though even then the use case will mostly be to
    prerender the CPU-bound work, not lazy fetches.
    acdlite committed Mar 21, 2023
    Configuration menu
    Copy the full SHA
    11133b8 View commit details
    Browse the repository at this point in the history