Commit Graph
4 Commits
Author SHA1 Message Date
Andrew Clark 12a1d140e3 Don't prerender siblings of suspended component (#26380)
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 making 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.
2023-03-21 10:24:56 -04:00
Andrew Clark 62cd5af08e Codemod redundant async act scopes (#26350)
Prior to #26347, our internal `act` API (not the public API) behaved
differently depending on whether the scope function returned a promise
(i.e. was an async function), for historical reasons that no longer
apply. Now that this is fixed, I've codemodded all async act scopes that
don't contain an await to be sync.

No pressing motivation other than it looks nicer and the codemod was
easy. Might help avoid confusion for new contributors who see async act
scopes with nothing async inside and infer it must be like that for a
reason.
2023-03-08 16:40:23 -05:00
Andrew Clark 44d3807945 Move internalAct to internal-test-utils package (#26344)
This is not a public API. We only use it for our internal tests, the
ones in this repo. Let's move it to this private package. Practically
speaking this will also let us use async/await in the implementation.
2023-03-08 12:58:31 -05:00
Andrew Clark 1528c5ccdf SchedulerMock.unstable_yieldValue -> SchedulerMock.log (#26312)
(This only affects our own internal repo; it's not a public API.)

I think most of us agree this is a less confusing name. It's possible
someone will confuse it with `console.log`. If that becomes a problem we
can warn in dev or something.
2023-03-06 11:09:07 -05:00