Currently, `waitForThrow` tries to diff the expected value against the
thrown value if it doesn't match. However if the expectation is a
string, we are not diffing against the thrown message. This commit makes
it so if we are matching against message we also diff against message.
This updates the Suspense fuzz tester to use `act` to recursively flush
timers instead of doing it manually.
This still isn't great because ideally the fuzz tester wouldn't fake
timers at all. It should resolve promises using a custom queue instead
of Jest's fake timer queue, like we've started doing in our other
Suspense tests (i.e. the `resolveText` pattern). That's because our
internal `act` API (not the public one, the one we use in our tests)
uses Jest's fake timer queue as a way to force Suspense fallbacks to
appear.
However I'm not interested in upgrading this test suite to a better
strategy right now because if I were writing a Suspense fuzzer today I
would probably use an entirely different approach. So this is just an
incremental improvement to make it slightly less decoupled to React
implementation details.
I rewrote some of our tests that deal with microtasks with the aim of
making them less coupled to implementation details. This is related to
an upcoming change to move update processing into a microtask.
Continuing my journey to migrate all the Scheduler flush* methods to
async versions of the same helpers.
`unstable_flushExpired` is a rarely used helper that is only meant to be
used to test a very particular implementation detail (update starvation
prevention, or what we sometimes refer to as "expiration").
I've prefixed the new helper with `unstable_`, too, to indicate that our
tests should almost always prefer one of the other patterns instead.
Added an explicit type to all $FlowFixMe suppressions to reduce
over-suppressions of new errors that might be caused on the same lines.
Also removes suppressions that aren't used (e.g. in a `@noflow` file as
they're purely misleading)
Test Plan:
yarn flow-ci
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.
I updated some of the Float tests that intentionally trigger an error to
assert on the specific error message, rather than swallow any errors
that may or may not happen.
Node's MessageChannel implementation will leak if you don't explicitly
close the port. This updates the enqueueTask function we use in our
internal testing helpers to close the port once it's no longer needed.
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.
This adds an async gap to our internal implementation of `act` (the one
used by our repo, not the public API). Rather than call the provided
scope function synchronously when `act` is called, we call it in a
separate async task. This is an extra precaution to ensure that our
tests do not accidentally rely on work being queued synchronously,
because that is an implementation detail that we should be allowed to
change. We don't do this in the public version of `act`, though we maybe
should in the future, for the same rationale. That might be tricky,
though, because it could break existing tests.
This also fixes the issue where our internal `act` requires an async
function. You can pass it a regular function, too.
To wait for the microtask queue to empty, our internal test helpers
schedule an arbitrary task using `setImmediate`. It doesn't matter what
kind of task it is, only that it's a separate task from the current one,
because by the time it fires, the microtasks for the current event will
have already been processed.
The issue with `setImmediate` is that Jest mocks it. Which can lead to
weird behavior.
I've changed it to instead use a message event, via the MessageChannel
implementation exposed by the `node:worker_threads` module.
We should consider doing this in the public implementation of `act`,
too.
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.
(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.
I'm in the process of codemodding our test suite to the waitFor pattern.
See #26285 for full context.
This module required a lot of manual changes so I'm doing it as its own
PR. The reason is that most of the tests involved simulating an async
import by wrapping them in `Promise.resolve()`, which means they would
immediately resolve the next time the microtask queue was flushed. I
rewrote the tests to resolve the simulated import explicitly.
While converting these tests, I also realized that the `waitFor` helpers
weren't properly waiting for the entire microtask queue to recursively
finish — if a microtask schedules another microtask, the subsequent one
wouldn't fire until after `waitFor` had resolved. To fix this, I used
the same strategy as `act` — wait for a real task to finish before
proceeding, such as a message event.
Over the years, we've gradually aligned on a set of best practices for
for testing concurrent React features in this repo. The default in most
cases is to use `act`, the same as you would do when testing a real
React app. However, because we're testing React itself, as opposed to an
app that uses React, our internal tests sometimes need to make
assertions on intermediate states that `act` intentionally disallows.
For those cases, we built a custom set of Jest assertion matchers that
provide greater control over the concurrent work queue. It works by
mocking the Scheduler package. (When we eventually migrate to using
native postTask, it would probably work by stubbing that instead.)
A problem with these helpers that we recently discovered is, because
they are synchronous function calls, they aren't sufficient if the work
you need to flush is scheduled in a microtask — we don't control the
microtask queue, and can't mock it.
`act` addresses this problem by encouraging you to await the result of
the `act` call. (It's not currently required to await, but in future
versions of React it likely will be.) It will then continue flushing
work until both the microtask queue and the Scheduler queue is
exhausted.
We can follow a similar strategy for our custom test helpers, by
replacing the current set of synchronous helpers with a corresponding
set of async ones:
- `expect(Scheduler).toFlushAndYield(log)` -> `await waitForAll(log)`
- `expect(Scheduler).toFlushAndYieldThrough(log)` -> `await
waitFor(log)`
- `expect(Scheduler).toFlushUntilNextPaint(log)` -> `await
waitForPaint(log)`
These APIs are inspired by the existing best practice for writing e2e
React tests. Rather than mock all task queues, in an e2e test you set up
a timer loop and wait for the UI to match an expecte condition. Although
we are mocking _some_ of the task queues in our tests, the general
principle still holds: it makes it less likely that our tests will
diverge from real world behavior in an actual browser.
In this commit, I've implemented the new testing helpers and converted
one of the Suspense tests to use them. In subsequent steps, I'll codemod
the rest of our test suite.
Josh StoryandGitHub
Andrew ClarkandGitHub
Jan KassensandGitHub