(Almost) all pragmas are now one of the following:
- `@...TestOnly`: custom pragma for test fixtures
- `@<configName>` | `@<configName>:true`: enables with either true or a default enabled value
- `@<configName>:<json value>`
`fragmentInstance.dispatchEvent(evt)` calls `element.dispatchEvent(evt)`
on the fragment's host parent. This mimics bubbling if the
`fragmentInstance` could receive an event itself.
If the parent is disconnected, there is a dev warning and no event is
dispatched.
## Summary
`-constant` is represented as a `UnaryExpression` node that is currently
not part of constant folding. If the operand is a constant number, the
node is folded to `constant * -1`. This also coerces `-0` to `0`,
resulting in `0 === -0` being folded to `true`.
## How did you test this change?
See attached tests
Follow up to #33136.
This clarifies in the types where the conversion happens from a CallSite
which we use to simulate getting the enclosing line/col to a
FunctionLocation which doesn't represent a CallSite but actually just
the function which only has an enclosing line/col.
This enables `focus` and `focusLast` methods on FragmentInstances to
search nested host components, depth first. Attempts focus on each child
and bails if one is successful. Previously, only the first level of host
children would attempt focus.
Now if we have an example like
```
component MenuItem() {
return (<div><a>{...}</a></div>)
}
component Menu() {
return <Fragment>{items.map(i => <MenuItem i={i} />)}</Fragment>
}
```
We can target focus on the first or last a tag, rather than checking
each wrapping div and then noop.
Stacked on #33135.
This encodes the line/column of the enclosing function as part of the
stack traces. When that information is available.
I adjusted the fake function code generation so that the beginning of
the arrow function aligns with these as much as possible.
This ensures that when the browser tries to look up the line/column of
the enclosing function, such as for getting the function name, it gets
the right one. If we can't get the enclosing line/column, then we encode
it at the beginning of the file. This is likely to get a miss in the
source map identifiers, which means that the function name gets
extracted from the runtime name instead which is better.
Another thing where this is used is the in the Performance Track.
Ideally that would be fixed by
https://issues.chromium.org/u/1/issues/415968771 but the enclosing
information is useful for other things like the function name resolution
anyway.
We can also use this for the "View source for this element" in React
DevTools.
This is first step to include more enclosing line/column in the parsed
data.
We install our own `prepareStackTrace` to collect structured callsite
data and only fall back to parsing the string if it was already
evaluated or if `prepareStackTrace` doesn't work in this environment.
We still mirror the default V8 format for encoding the function name
part. A lot of this is covered by tests already.
## Summary
When using React DevTools to highlight component updates, the highlights
would sometimes appear behind elements that use the browser's
[top-layer](https://developer.mozilla.org/en-US/docs/Glossary/Top_layer)
(such as `<dialog>` elements or components using the Popover API). This
made it difficult to see which components were updating when they were
inside or behind top-layer elements.
This PR fixes the issue by using the Popover API to ensure that
highlighting appears on top of all content, including elements in the
top-layer. The implementation maintains backward compatibility with
browsers that don't support the Popover API.
## How did you test this change?
I tested this change in the following ways:
1. Manually tested in Chrome (which supports the Popover API) with:
- Created a test application with React components inside `<dialog>`
elements and custom elements using the Popover API
- Verified that component highlighting appears above these elements when
they update
- Confirmed that highlighting displays correctly for nested components
within top-layer elements
2. Verified backward compatibility:
- Tested in browsers without Popover API support to ensure fallback
behavior works correctly
- Confirmed that no errors occur and highlighting still functions as
before
3. Ran the React DevTools test suite:
- All tests pass successfully
- No regressions were introduced
[demo-page](https://devtools-toplayer-demo.vercel.app/)
[demo-repo](https://github.com/yongsk0066/devtools-toplayer-demo)
### AS-IS
https://github.com/user-attachments/assets/dc2e1281-969f-4f61-82c3-480153916969
### TO-BE
https://github.com/user-attachments/assets/dd52ce35-816c-42f0-819b-0d5d0a8a21e5
This adds `compareDocumentPosition(otherNode)` to fragment instances.
The semantics implemented are meant to match typical element
positioning, with some fragment specifics. See the unit tests for all
expectations.
- An element preceding a fragment is `Node.DOCUMENT_POSITION_PRECEDING`
- An element after a fragment is `Node.DOCUMENT_POSITION_FOLLOWING`
- An element containing the fragment is
`Node.DOCUMENT_POSITION_PRECEDING` and
`Node.DOCUMENT_POSITION_CONTAINING`
- An element within the fragment is
`Node.DOCUMENT_POSITION_CONTAINED_BY`
- An element compared against an empty fragment will result in
`Node.DOCUMENT_POSITION_DISCONNECTED` and
`Node.DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC`
Since we assume a fragment instances target children are DOM siblings
and we want to compare the full fragment as a pseudo container, we can
compare against the first target child outside of handling the special
cases (empty fragments and contained elements).
Multiple things here:
- Improve the mean calculation for metrics so we don't report 0 when
web-vitals fail to be retrieved
- improve ui chaos monkey to use puppeteer APIs since only those trigger
INP/CLS metrics since we need emulated mouse clicks
- Add logic to navigate to a temp page after render since some
web-vitals metrics are only calculated when the page is backgrounded
- Some readability improvements
Originally I thought it was important that SSR used the same View
Transition name as the client so that the Fizz runtime could emit those
names and then the client could pick up and take over. However, I no
longer believe that approach is feasible. Instead, the names can be
generated only during that particular animation.
Therefore we can simplify the auto name assignment to not have to
consider the hydration.
Stacked on #33129. Flagged behind `enableHydrationChangeEvent`.
If you type into a controlled input before hydration and something else
rerenders like a setState in an effect, then the controlled input will
reset to whatever React thought it was. Even with event replaying that
this is stacked on, if the second render happens before event replaying
has fired in a separate task.
We don't want to flush inside the commit phase because then things like
flushSync in these events wouldn't work since they're inside the commit
stack.
This flushes all event replaying between renders by flushing it at the
end of `flushSpawned` work. We've already committed at that point and is
about to either do subsequent renders or yield to event loop for passive
effects which could have these events fired anyway. This just ensures
that they've already happened by the time subsequent renders fire. This
means that there's now a type of event that fire between sync render
passes.
This fixes a long standing issue that controlled inputs gets out of sync
with the browser state if it's changed before we hydrate.
This resolves the issue by replaying the change events (click, input and
change) if the value has changed by the time we commit the hydration.
That way you can reflect the new value in state to bring it in sync. It
does this whether controlled or uncontrolled.
The idea is that this should be ok to replay because it's similar to the
continuous events in that it doesn't replay a sequence but only reflects
the current state of the tree.
Since this is a breaking change I added it behind
`enableHydrationChangeEvent` flag.
There is still an additional issue remaining that I intend to address in
a follow up. If a `useLayoutEffect` triggers an sync rerender on
hydration (always a bad idea) then that can rerender before we have had
a chance to replay the change events. If that renders through a input
then that input will always override the browser value with the
controlled value. Which will reset it before we've had a change to
update to the new value.
Note that bailing out adds false positives for hoisted functions whose
only references are within other functions. For example, this rewrite
would be safe.
```js
// source program
function foo() {
return bar();
}
function bar() {
return 42;
}
// compiler output
let bar;
if (/* deps changed */) {
function foo() {
return bar();
}
bar = function bar() {
return 42;
}
}
```
These false positives are difficult to detect because any maybe-call of
foo before the definition of bar would be invalid.
Instead of bailing out, we should rewrite hoisted function declarations
to the following form.
```js
let bar$0;
if (/* deps changed */) {
// All references within the declaring memo block
// or before the function declaration should use
// the original identifier `bar`
function foo() {
return bar();
}
function bar() {
return 42;
}
bar$0 = bar;
}
// All references after the declaring memo block
// or after the function declaration should use
// the rewritten declaration `bar$0`
```
I noticed that we increase this in the recursive part of the algorithm.
This would mean that we'd count a key more than once if it has Server
Components inside it recursively resolving. This moves it out to where
we enter from toJSON. Which is called once per JSON entry (and therefore
once per key).
This revisits a validation I built a while ago, trying to make it more strict this time to ensure that it's high-signal.
We detect function expressions which are *known* mutable — they definitely can modify a variable defined outside of the function expression itself (modulo control flow). This uses types to look for known Store and Mutate effects only, and disregards mutations of effects. Any such function passed to a location with a Freeze effect is reported as a validation error.
This is behind a flag and disabled by default. If folks agree this makes sense to revisit, i'll test out internally and we can consider enabling by default.
ghstack-source-id: 075a731444
Pull Request resolved: https://github.com/facebook/react/pull/33079
If a function captures a mutable value but never gets called, we don't infer a mutable range for that function. This means that we also don't alias the function with its mutable captures.
This case is tricky, because we don't generally know for sure what is a mutation and what may just be a normal function call. For example:
```js
hook useFoo() {
const x = makeObject();
return () => {
return readObject(x); // could be a mutation!
}
}
```
If we pessimistically assume that all such cases are mutations, we'd have to group lots of memo scopes together unnecessarily. However, if there is definitely a mutation:
```js
hook useFoo(createEntryForKey) {
const cache = new WeakMap();
return (key) => {
let entry = cache.get(key);
if (entry == null) {
entry = createEntryForKey(key);
cache.set(key, entry); // known mutation!
}
return entry;
}
}
```
Then we have to ensure that the function and its mutable captures alias together and end up in the same scope. However, aliasing together isn't enough if the function and operands all have empty mutable ranges (end = start + 1).
This pass finds function expressions and object methods that have an empty mutable range and known-mutable operands which also don't have a mutable range, and ensures that the function and those operands are aliased together *and* that their ranges are updated to end after the function expression. This is sufficient to ensure that a reactive scope is created for the alias set.
NOTE: The alternative is to reject these cases. If we do that we'd also want to similarly disallow cases like passing a mutable function to a hook.
ghstack-source-id: 5d8158246a
Pull Request resolved: https://github.com/facebook/react/pull/33078
Building on mofeiz's recent work to type constructors. Also, types for reanimated values which are useful in the next PR.
ghstack-source-id: 1c81e213a1
Pull Request resolved: https://github.com/facebook/react/pull/33077
This pass didn't previously report the precise difference btw inferred/manual dependencies unless a debug flag was set. But the error message is really good (nice job mofeiz): the only catch is that in theory the inferred dep could be a temporary that can't trivially be reported to the user.
But the messages are really useful for quickly verifying why the compiler couldn't preserve memoization. So here we switch to outputting a detailed message about the discrepancy btw inferred/manual deps so long as the inferred dep root is a named variable. I also slightly adjusted the message to handle the case where there is no diagnostic, which can occur if there were no manual deps but the compiler inferred a dependency.
ghstack-source-id: 534f6f1fec
Pull Request resolved: https://github.com/facebook/react/pull/33095
If a JSX attribute value is a string that contains unicode or other characters that need special escaping, we wrap the attribute value in an expression container. However, our unicode to detect this only handled the basic unicode character plane, not the "astral" plane which includes emojis.
This PR updates the regex to detect such extended characters and also use an expression container.
ghstack-source-id: 6d9c8e4dd2
Pull Request resolved: https://github.com/facebook/react/pull/33096
Our runtime results are still bogus, but so far this tweak let's the llm
use the tools a little more intelligently.
example:
---
```js
import React from 'react';
function App() {
const [count, setCount] = React.useState(0);
const [data, setData] = React.useState([]);
React.useEffect(() => {
const processData = () => {
console.log("Processing data...");
const newData = [];
for (let i = 0; i < 1000000000; i++) {
newData.push({ id: i, value: Math.random() * count });
}
return newData;
};
const processedData = processData();
setData(processedData);
}, [count]);
return (
<div>
<p>Count: {count}</p>
<p>Data Length: {data.length}</p>
<ul>
{data.slice(0, 5).map((item) => (
<li key={item.id}>Item {item.id}: {item.value.toFixed(2)}</li>
))}
</ul>
</div>
);
};
```
why is this component slow? use the react tools to figure this out.
> I'll analyze why this component is slow using the React tools. Let's
first evaluate the performance of the original code.
>
[review-react-runtime ...]
>
> The performance measurement tool timed out, which is a strong
indication of just how severe the performance issue is in this
component. Let's now try to compile the code to see if React Compiler
can identify any issues:
>
[compile...]
>
> Now I understand the issue. Let's analyze why this component > is
extremely slow:
> Main Performance Issues:
>
> Massive Data Processing: The component is trying to create an array
with a billion (1,000,000,000) objects in the processData function. This
is an enormous amount of data that will:
>
> Consume massive amounts of memory
> Take an extremely long time to process
> Likely crash the browser
>
>
> Unnecessary Recalculation: The effect depends on [count], meaning
every time the count changes, it will reprocess this enormous dataset.
> Inefficient Data Handling: Even though only 5 items are displayed, the
component processes a billion items.
>
> Let's fix these issues:
>
[compile...]
>
> Now let's check if our modified code performs better:
>
[review-react-runtime...]
>
> Excellent! The performance has dramatically improved. Now let's check
the React documentation to make sure we're following the best practices:
>
[query-react-dev-docs...]
(hit the maximum length conversation limit because i'm on a free plan)
Since we use esbuild we need to correctly move dependencies that are
required at runtime into `dependencies` and other packages that are only
used in development in to `devDependencies`. This ensures the correct
packages are included in the build.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33101).
* #33085
* #33084
* #33083
* #33082
* __->__ #33101
Because we now decided whether to outline in the flushing phase, when
we're writing the preamble we don't yet know if we will make that
decision so we don't know if it's safe to omit the external runtime.
However, if you are providing an external runtime it's probably a pretty
safe bet you're streaming something dynamically that's likely to need it
so we can always include it.
The main thing is that this makes it hard to test it because it affects
our tests in ways it wouldn't otherwise so we have to add a bunch of
conditions.
Stacked on #33076.
This fixes a bug where we used the "complete" status but the
DOMContentLoaded event. This checks for not "loading" instead.
We also add a new status where the boundary has been marked as complete
by the server but has not yet flushed either due to being throttled,
suspended on CSS or animating.
Stacked on #33073.
React semantics is that Suspense boundaries reveal with a throttle
(300ms). That helps avoid flashing reveals when a stream reveals many
individual steps back to back. It can also improve overall performance
by batching the layout and paint work that has to happen at each step.
Unfortunately we never implemented this for SSR streaming - only for
client navigations. This is highly noticeable on very dynamic sites with
lots of Suspense boundaries. It can look good with a client nav but feel
glitchy when you reload the page or initial load.
This fixes the Fizz runtime to be throttled and reveals batched into a
single paint at a time. We do this by first tracking the last paint
after the complete (this will be the first paint if `rel="expect"` is
respected). Then in the `completeBoundary` operation we queue the
operation and then flush it all into a throttled batch.
Another motivation is that View Transitions need to operate as a batch
and individual steps get queued in a sequence so it's extra important to
include as much content as possible in each animated step. This will be
done in a follow up for SSR View Transitions.
Stacked on #33066 and #33068.
Currently we're passing `errorDigest` to `completeBoundary` if there is
a client side error (only CSS loading atm). This only exists because of
`completeBoundaryWithStyles`. Normally if there's a server-side error
we'd emit the `clientRenderBoundary` instruction instead. This adds
unnecessary code to the common case where all styles are in the head.
This is about to get worse with batching because client render shouldn't
be throttled but complete should be.
The first commit moves the client render logic inline into
`completeBoundaryWithStyles` so we only pay for it when styles are used.
However, the approach I went with in the second commit is to reuse the
`$RX` instruction instead (`clientRenderBoundary`). That way if you have
both it ends up being amortized. However, it does mean we have to emit
the `$RX` (along with the `$RC` helper if any
`completeBoundaryWithStyles` instruction is needed.
Stacked on #33065.
The runtime is about to be a lot more complicated so we need to start
sharing some more code.
The problem with sharing code is that we want the inline runtime to as
much as possible be isolated in its scope using only a few global
variables to refer across runtimes.
A problem with Closure Compiler is that it refuses to inline functions
if they have closures inside of them. Which makes sense because of how
VMs work it can cause memory leaks. However, in our cases this doesn't
matter and code size matters more. So we can't use many clever tricks.
So this just favors writing the source in the inline form. Then we add
an extra compiler pass to turn those global variables into local
variables in the external runtime.
We weren't treating terminal operands as eligible for memoization in PruneNonEscapingScopes, which meant that they could end up un-memoized. Terminal operands can also be compound ReactiveValues like SequenceExpressions, so part of the fix is to make sure we don't just recurse into compound values but record the full aliasing information we would for top-level instructions.
Still WIP, this needs to handle terminals other than for..of.
ghstack-source-id: 09a2923051
Pull Request resolved: https://github.com/facebook/react/pull/33062
## Summary
Fix babel presets, and add a bit more context to the tool so that it is
more reliable
## How did you test this change?
Manually tested the mcp integrated with claude desktop
We normally expect the segment to exist whatever the client does while
streaming. However, when hydration errors at the root of the shell for a
whole document render, then we clear nodes from body which can include
our segments. We don't need them anymore because we switched to client
rendering.
It triggers an error accessing parent node which can safely be ignored.
This just helps avoid confusion in this scenario.
This also covers up the error in #33067. Which doesn't actually cause
any visible problems other than error logging. However, ideally we
wouldn't emit completeBoundary instructions if the boundary is inside a
cancelled fallback.
Mofei Zhang
mofeiZ
Dawid Małecki
Ruslan Lesiutin
Jack Pope
Niklas Mollenhauer
Sebastian Markbåge
YongSeok Jang (장용석)
Jorge Cabiedes
Matt Carroll
Sebastian "Sebbie" Silbermann
Stephen Zhou
Joe Savona
Jan Kassens
lauren