## Summary
```js
assertConsoleErrorDev([
['Hello', {withoutStack: true}]
])
```
now errors with a helpful diff message if the message mismatched. See
first commit for previous behavior.
## How did you test this change?
- `yarn test --watch
packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js`
Fixes issue where if the first letter of the expected string appeared
anywhere in actual message, the assertion would pass, leading to false
negatives. We should check the entire expected string.
---------
Co-authored-by: Ricky <rickhanlonii@gmail.com>
## Overview
**Internal React repo tests only**
Depends on https://github.com/facebook/react/pull/28710
Adds three new assertions:
- `assertConsoleLogDev`
- `assertConsoleWarnDev`
- `assertConsoleErrorDev`
These will replace this pattern:
```js
await expect(async () => {
await expect(async () => {
await act(() => {
root.render(<Fail />)
});
}).toThrow();
}).toWarnDev('Warning');
```
With this:
```js
await expect(async () => {
await act(() => {
root.render(<Fail />)
});
}).toThrow();
assertConsoleWarnDev('Warning');
```
It works similar to our other `assertLog` matchers which clear the log
and assert on it, failing the tests if the log is not asserted before
the test ends.
## Diffs
There are a few improvements I also added including better log diffs and
more logging.
When there's a failure, the output will look something like:
<img width="655" alt="Screenshot 2024-04-03 at 11 50 08 AM"
src="https://github.com/facebook/react/assets/2440089/0c4bf1b2-5f63-4204-8af3-09e0c2d752ad">
Check out the test suite for snapshots of all the failures we may log.
Moving this to `internal-test-utils` so I can add helpers in the next PR
for:
- assertLogDev
- assertWarnDev
- assertErrorDev
Which will be exported from `internal-test-utils`. This isn't strictly
necessary, but it makes the factoring nicer, so internal-test-until
doesn't need to depend on `scripts/jest`.
## Overview
For events, the browser will yield to microtasks between calling event
handers, allowing time to flush work inbetween. For example, in the
browser, this code will log the flushes between events:
```js
<body onclick="console.log('body'); Promise.resolve().then(() => console.log('flush body'));">
<div onclick="console.log('div'); Promise.resolve().then(() => console.log('flush div'));">
hi
</div>
</body>
// Logs
div
flush div
body
flush body
```
[Sandbox](https://codesandbox.io/s/eloquent-noether-mw2cjg?file=/index.html)
The problem is, `dispatchEvent` (either in the browser, or JSDOM) does
not yield to microtasks. Which means, this code will log the flushes
after the events:
```js
const target = document.getElementsByTagName("div")[0];
const nativeEvent = document.createEvent("Event");
nativeEvent.initEvent("click", true, true);
target.dispatchEvent(nativeEvent);
// Logs
div
body
flush div
flush body
```
## The problem
This mostly isn't a problem because React attaches event handler at the
root, and calls the event handlers on components via the synthetic event
system. We handle flushing between calling event handlers as needed.
However, if you're mixing capture and bubbling events, or using multiple
roots, then the problem of not flushing microtasks between events can
come into play. This was found when converting a test to `createRoot` in
https://github.com/facebook/react/pull/28050#discussion_r1462118422, and
that test is an example of where this is an issue with nested roots.
Here's a sandox for
[discrete](https://codesandbox.io/p/sandbox/red-http-2wg8k5) and
[continuous](https://codesandbox.io/p/sandbox/gracious-voice-6r7tsc?file=%2Fsrc%2Findex.js%3A25%2C28)
events, showing how the test should behave. The existing test, when
switched to `createRoot` matches the browser behavior for continuous
events, but not discrete. Continuous events should be batched, and
discrete should flush individually.
## The fix
This PR implements the fix suggested by @sebmarkbage, to manually
traverse the path up from the element and dispatch events, yielding
between each call.
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.
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 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.
Sebastian SilbermannandGitHub
