The test case here previously reported a "Cannot modify local variables after render completes" error (from ValidateNoFreezingKnownMutableFunctions). This happens because one of the functions passed to a hook clearly mutates a ref — except that we try to ignore mutations of refs! The problem in this case is that the `const ref = ...` was getting converted to a context variable since the ref is accessed in a function before its declaration. We don't infer types for context variables at all, and our ref handling is based on types, so we failed to ignore this ref mutation.
The fix is to recognize that `StoreLocal const ...` is a special case: the variable may be referenced in code before the declaration, but at runtime it's either a TDZ error or the variable will have the type from the declaration. So we can safely infer a type.
Fixes two related cases of mutation of potentially frozen values.
The first is method calls on frozen values. Previously, we modeled unknown function calls as potentially aliasing their receiver+args into the return value. If the receiver or argument were known to be frozen, then we would downgrade the `Alias` effect into an `ImmutableCapture`. However, within a function expression it's possible to call a function using a frozen value as an argument (that gets `Alias`-ed into the return) but where we don't have the context locally to know that the value is frozen.
This results in cases like this:
```js
const frozen = useContext(...);
useEffect(() => {
frozen.method().property = true;
^^^^^^^^^^^^^^^^^^^^^^^^ cannot mutate frozen value
}, [...]);
```
Within the function we would infer:
```
t0 = MethodCall ...
Create t0 = mutable
Alias t0 <- frozen
t1 = PropertyStore ...
Mutate t0
```
And then transitively infer the function expression as having a `Mutate 'frozen'` effect, which when evaluated against the outer context (`frozen` is frozen) is an error.
The fix is to model unknown function calls as _maybe_ aliasing their receiver/args in the return, and then considering mutations of a maybe-aliased value to only be a conditional mutation of the source:
```
t0 = MethodCall ...
Create t0 = mutable
MaybeAlias t0 <- frozen // maybe alias now
t1 = PropertyStore ...
Mutate t0
```
Then, the `Mutate t0` turns into a `MutateConditional 'frozen'`, which just gets ignored when we process the outer context.
The second, related fix is for known mutation of phis that may be a frozen value. The previous inference model correctly recorded these as errors, the new model does not. We now correctly report a validation error for this case in the new model.
This is mostly to kick off conversation, i think we should go with a
modified version of the implemented approach that i'll describe here.
The playground currently serves two roles. The primary one we think
about is for verifying compiler output. We use it for this sometimes,
and developers frequently use it for this, including to send us repros
if they have a potential bug. The second mode is to help developers
learn about React. Part of that includes learning how to use React
correctly — where it's helpful to see feedback about problematic code —
and also to understand what kind of tools we provide compared to other
frameworks, to make an informed choice about what tools they want to
use.
Currently we primarily think about the first role, but I think we should
emphasize the second more. In this PR i'm doing the worst of both:
enabling all the validations used by both the compiler and the linter by
default. This means that code that would actually compile can fail with
validations, which isn't great.
What I think we should actually do is compile twice, one in
"compilation" mode and once in "linter" mode, and combine the results as
follows:
* If "compilation" mode succeeds, show the compiled output _and_ any
linter errors.
* If "compilation" mode fails, show only the compilation mode failures.
We should also distinguish which case it is when we show errors:
"Compilation succeeded", "Compilation succeeded with linter errors",
"Compilation failed".
This lets developers continue to verify compiler output, while also
turning the playground into a much more useful tool for learning React.
Thoughts?
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33777).
* #33981
* __->__ #33777
Uses the new diagnostic infrastructure for this validation, which lets
us provide a more targeted message on the text that we highlight (eg
"This dependency may be mutated later") separately from the overall
error message.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33759).
* #33981
* #33777
* #33767
* #33765
* #33760
* __->__ #33759
* #33758
This PR uses the new diagnostic type for most of the error messages
produced in our explicit validation passes (`Validation/` directory).
One of the validations produced multiple errors as a hack to showing
multiple related locations, which we can now consolidate into a single
diagnostic.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33758).
* #33981
* #33777
* #33767
* #33765
* #33760
* #33759
* __->__ #33758
Work in progress, i'm experimenting with revamping our diagnostic infra.
Starting with a better format for representing errors, with an ability
to point ot multiple locations, along with better printing of errors. Of
course, Babel still controls the printing in the majority case so this
still needs more work.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33751).
* #33981
* #33777
* #33767
* #33765
* #33760
* #33759
* #33758
* __->__ #33751
* #33752
* #33753
When destructuring, spread creates a new mutable object that _captures_
part of the original rvalue. This new value is safe to modify.
When making this change I realized that we weren't inferring array
pattern spread as creating an array (in type inference) so I also added
that here.
I broke Firefox DevTools extension in #33968.
It turns out the Firefox has a placeholder object for the sources panel
which is empty. We need to detect the actual event handler.
We currently throw away the Error once we've used to the owner stack of
a Fiber once. This maybe helps a bit with memory and redoing it but we
really don't expect most Fibers to hit this at all. It's not very hot.
If we throw away the Error, then we can't use native debugger protocols
to inspect the native stack. Instead, we'd have to maintain a url to
resource map indefinitely like what Chrome DevTools does to map a url to
a resource. Technically it's not even technically correct since the file
path might not be reversible and could in theory conflict.
This adds a "Code Editor" pane for the Chrome extension in the bottom
right corner of the "Sources" panel. If you end up getting linked to the
"Sources" panel from stack traces in console, performance tab, stacks in
React Component tab like the one added in #33954 basically everywhere
there's a link to source code. Then going from there to open in a code
editor should be more convenient. This adds a button to open the current
file.
<img width="1387" height="389" alt="Screenshot 2025-07-22 at 10 22
19 PM"
src="https://github.com/user-attachments/assets/fe01f84c-83c2-4639-9b64-4af1a90c3f7d"
/>
This only makes sense in the extensions since in standalone it needs to
always open by default in an editor. Unfortunately Firefox doesn't
support extending the Sources panel.
Chrome is also a bit buggy where it doesn't send a selection update
event when you switch tabs in the Sources panel. Only when the actual
cursor position changes. This means that the link can be lagging behind
sometimes. We also have some general bugs where if React DevTools loses
connection it can break the UI which includes this pane too.
This has a small inline configuration too so that it's discoverable:
<img width="559" height="143" alt="Screenshot 2025-07-22 at 10 22 42 PM"
src="https://github.com/user-attachments/assets/1270bda8-ce10-4f9d-9fcb-080c0198366a"
/>
<img width="527" height="123" alt="Screenshot 2025-07-22 at 10 22 30 PM"
src="https://github.com/user-attachments/assets/45848c95-afd8-495f-a7cf-eb2f46e698f2"
/>
Since we can't add a separate link to open-in-editor or open-in-sources
everywhere I plan on adding an option to open in editor by default in a
follow up. That option needs to be even more discoverable.
I moved the configuration from the Components settings to the General
settings since this is now a much more general features for opening
links to resources in all types of panes.
<img width="673" height="311" alt="Screenshot 2025-07-22 at 10 22 57 PM"
src="https://github.com/user-attachments/assets/ea2c0871-942c-4b55-a362-025835d2c2bd"
/>
If a `file:///` path is specified as the url of a file, like after
source mapping into an ESM file, then we should be able to open it in a
code editor.
In RSC and other stacks now we use a lot of `ReactFunctionLocation` type
to represent the location of a function. I.e. the location of the
beginning of the function (the enclosing line/col) that is represented
by the "Source" of the function. This is also what the parent Component
Stacks represents.
As opposed to `ReactCallSite` which is what normal stack traces and
owner stacks represent. I.e. the line/column number of the callsite into
the next function.
We can start sharing more code by using the `ReactFunctionLocation` type
to represent the component source location and it also helps clarify
which ones are function locations and which ones are callsites as we
start adding more stack traces (e.g. for async debug info and owner
stack traces).
This makes it so you can click the source location itself to view the
source. This is similar styling as the link to jump to function props
like events and actions. We're going to need a lot more linkifying to
jump to various source locations. Also, I always was trying to click
this file anyway.
Hover state:
<img width="485" height="382" alt="Screenshot 2025-07-21 at 4 36 10 PM"
src="https://github.com/user-attachments/assets/1f0f8f8c-6866-4e62-ab84-1fb5ba012986"
/>
We need a "value" to represent the I/O that was loaded. We don't
normally actually use the Promise at the callsite that started the I/O
because that's usually deep inside internals. Instead we override the
value of the I/O entry with the Promise that was first awaited in user
space. This means that you could potentially have different values
depending on if multiple things await the same I/O. We just take one of
them. (Maybe we should actually just write the first user space awaited
Promise as the I/O entry? This might instead have other implications
like less deduping.)
When you pass a Promise forward, we may skip the awaits that happened in
earlier components because they're not part of the currently rendering
component. That's mainly for the stack and time stamps though. The value
is still probably conceptually the best value because it represents the
I/O value as far user space is concerned.
This writes the I/O early with the first await we find in user space
even if we're not going to use that particular await for the stack.
If you pass a promise to a client component to be rendered `<Client
promise={promise} />` then there's an internal await inside Flight.
There might also be user space awaits but those awaits may already have
happened before we render this component. Conceptually they were part of
the parent component and not this component. It's tricky to attribute
which await should be used for the stack in this case.
If we can't find an await we can use the JSX callsite as the stack
frame.
However, we don't want to do this for simple cases like if you return a
non-native Promise from a Server Component. Since that would now use the
stack of the thing that rendered the Server Component which is worse
than the stack of the I/O. To fix this, I update the
`debugOwner`/`debugTask`/`debugStack` when we start rendering inside the
Server Component. Conceptually these represent the "parent" component
and is used for errors referring to the parent like when we serialize
client component props the parent is the JSX of the client component.
However, when we're directly inside the Server Component we don't have a
callsite of the parent really. Conceptually it would be the return call
of the Server Component. This might negatively affect other types of
errors but I think this is ok since this feature mainly exists for the
case when you enter the child JSX.
This resolves an outstanding issue where it was possible for debug info
and console logs to become out of order if they up blocked. E.g. by a
future reference or a client reference that hasn't loaded yet. Such as
if you console.log a client reference followed by one that doesn't. This
encodes the order similar to how the stream chunks work.
This also blocks the main chunk from resolving until the last debug info
has fully loaded, including future references and client references.
This also ensures that we could send some of that data in a different
stream, since then it can come out of order.
We already do this with `"new Promise"` and `"Promise.then"`. There are
also many helpers that both create promises and awaits other promises
inside of it like `Promise.all`.
The way this is filtered is different from just filtering out all
anonymous stacks since they're used to determine where the boundary is
between ignore listed and user space.
Ideally we'd cover more wrappers that are internal to Promise libraries.
This fixes displaying incorrect component render entries on a timeline,
when we are reconnecting passive effects.
### Before
<img width="2318" height="1127" alt="1"
src="https://github.com/user-attachments/assets/9b6b2824-d2de-43a3-8615-2c45d67c3668"
/>
The cloned nodes will persist original `actualStartTime`, when these
were first mounted. When we "replay", the end time will be "now" or
whatever the actual start time of the sibling. Depending on when this is
being recorded, the diff between end and start could be tens of seconds
and doesn't represent what React was doing.
We shouldn't log these entries at all.
### After
We are only logging newly finished renders, but could potentially loose
renders that never commit.
## Summary
The `TSAsExpression` and `TSNonNullExpression` nodes are supported by
`lowerExpression()` but `isReorderableExpression()` does not check if
they can be reordered. This PR updates `isReorderableExpression()` to
handle these two node types by adding cases that fall through to the
existing `TypeCastExpression` case.
We ran `react-compiler-healthcheck` at scale on several of our repos and
found dozens of `` (BuildHIR::node.lowerReorderableExpression)
Expression type `TSAsExpression` cannot be safely reordered`` errors and
a handful for `TSNonNullExpression`.
## How did you test this change?
In this case I added two fixture tests
React Elements reference debug data (their stack and owner) in the debug
channel. If the debug channel isn't wired up this can block the client
from resolving.
We can infer that if there's no debug channel wired up and the reference
wasn't emitted before the element, then it's probably because it's in
the debug channel. So we can skip it.
This should also apply to debug chunks but they're not yet blocking
until #33665 lands.
Summary:
useEffectEvent is meant to be used specifically in combination with
useEffect, and using
the feature in arbitrary closures can lead to surprising reactivity
semantics. In order to
minimize risk in the experimental rollout, we are going to restrict its
usage to being
called directly inside an effect or another useEffectEvent, effectively
enforcing the function
coloring statically. Without an effect system this is the best we can
do.
This lets us pass a writable on the server side and readable on the
client side to send debug info through a separate channel so that it
doesn't interfere with the main payload as much. The main payload refers
to chunks defined in the debug info which means it's still blocked on it
though. This ensures that the debug data has loaded by the time the
value is rendered so that the next step can forward the data.
This will be a bit fragile to race conditions until #33665 lands.
Another follow up needed is the ability to skip the debug channel on the
receiving side. Right now it'll block forever if you don't provide one
since we're blocking on the debug data.
When postponing the root we encode the segment Id into the postponed
state but we should really be reseting it to zero so we can restart the
counter from the beginning when the resume is actually just a re-render.
This also no longer assigns the root segment id based on the postponed
state when resuming the root for the same reason. In the future we may
use the embedded replay segment id if we implement resuming the root
without re-rendering everything but that is not yet implemented or
planned.
import, export, and TS namespace statements can only be used at the
top-level of a module, which is enforced by parsers already. Here we add
a backup validation of that. As of this PR, we now have only major
statement type (class declarations) listed as a todo.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33748).
* #33753
* #33752
* #33751
* #33750
* __->__ #33748
Supports inline enum declarations in both Flow and TS by treating the
node as pass-through (enums can't capture values mutably). Related, this
PR extends the set of type-related declarations that we ignore.
Previously we threw a todo for things like DeclareClass or
DeclareVariable, but these are type related and can simply be dropped
just like we dropped TypeAlias.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33747).
* #33753
* #33752
* #33751
* #33750
* #33748
* __->__ #33747
In playground it's helpful to show all errors, even those that don't
completely abort compilation. For example, to help demonstrate that the
compiler catches things like setState in effects. This detects these
errors and ensures we show them.
This is the same as we do for currently rendering tasks. They get
effectively sync aborted when the listener is invoked.
We potentially miss out on some debug info in that case but that would
only apply to any entries inside the stream which doesn't really have
their own debug info anyway.
Follow up to #33736.
If we need to save on CPU/memory pressure, we can instead just pray and
hope that a Promise doesn't get garbage collected before we need to read
it.
This can cause fragile access to the Promise value in devtools
especially if it's a slow and pressured render.
Basically, you'd have to hope that GC doesn't run after the inner await
finishes its microtask callback and before the resolution of the
component being rendered is invoked.
If we have the ability to lazy load Promise values, i.e. if we have a
debug channel, then we should always use it for Promises that aren't
already resolved and instrumented.
There's little downside to this since they're async anyway.
This also lets us avoid adding `.then()` listeners too early. E.g. if
adding the listener would have side-effect. This avoids covering up
"unhandled rejection" errors. Since if we listen to a promise eagerly,
including reject listeners, we'd have marked that Promise's rejection as
handled where as maybe it wouldn't have been otherwise.
In this mode we can also indefinitely wait for the Promise to resolve
instead of just waiting a microtask for it to resolve.
Joe Savona
Joseph Savona
Sebastian Markbåge
Sebastian "Sebbie" Silbermann
Jordan Brown
Rubén Norte
Ruslan Lesiutin
Jack Pope
Henry Q. Dineen
Josh Story