Commit Graph

21032 Commits

Author SHA1 Message Date
Joseph Savona d99df87541 Merge 1b4eab90c2 into sapling-pr-archive-josephsavona 2025-07-24 15:41:54 -07:00
Joe Savona 1b4eab90c2 [compiler] Use diagnostic for "found suppression" error 2025-07-24 15:41:43 -07:00
Joe Savona d5e2c0ee1f [compiler][rfc] Enable more validations in playground.
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?
2025-07-24 15:41:42 -07:00
Joe Savona 8c29a45fb6 [compiler] Use new diagnostic printing in playground
Per title
2025-07-24 15:41:42 -07:00
Joe Savona fd2e18f40b [compiler] Cleanup diagnostic messages
Minor sytlistic cleanup
2025-07-24 15:41:42 -07:00
Joe Savona 94fe97db2f [compiler] Use new diagnostics for core inference errors
Uses the new diagnostic type for errors created during mutation/aliasing inference, such as errors for mutating immutable values like props or state, reassigning globals, etc.
2025-07-24 15:41:42 -07:00
Joseph Savona 48bc166428 [compiler] Update diagnostics for ValidatePreservedManualMemoization (#33759)
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
2025-07-24 15:39:53 -07:00
Joseph Savona 72848027a5 [compiler] Improve more error messages (#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
2025-07-24 15:39:42 -07:00
Joseph Savona 6f491e948d Merge c0d9e3b189 into sapling-pr-archive-josephsavona 2025-07-24 15:38:35 -07:00
Joe Savona c0d9e3b189 [compiler] Use diagnostic for "found suppression" error 2025-07-24 15:38:22 -07:00
Joe Savona a3e4b67ce3 [compiler][rfc] Enable more validations in playground.
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?
2025-07-24 15:38:22 -07:00
Joe Savona 504d4e7498 [compiler] Use new diagnostic printing in playground
Per title
2025-07-24 15:38:22 -07:00
Joe Savona c403a64787 [compiler] Cleanup diagnostic messages
Minor sytlistic cleanup
2025-07-24 15:38:22 -07:00
Joe Savona 7a2deaa3bd [compiler] Use new diagnostics for core inference errors
Uses the new diagnostic type for errors created during mutation/aliasing inference, such as errors for mutating immutable values like props or state, reassigning globals, etc.
2025-07-24 15:38:22 -07:00
Joe Savona 43ae8710ca [compiler] Update diagnostics for ValidatePreservedManualMemoization
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.
2025-07-24 15:38:22 -07:00
Joe Savona 2283eb9183 [compiler] Improve more error messages
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.
2025-07-24 15:38:22 -07:00
Joseph Savona 707e321f8f [compiler][wip] Improve diagnostic infra (#33751)
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
2025-07-24 15:37:06 -07:00
Joseph Savona 0d39496eab [compiler] Enable additional lints by default (#33752)
Enable more validations to help catch bad patterns, but only in the
linter. These rules are already enabled by default in the compiler _if_
violations could produce unsafe output.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33752).
* #33981
* #33777
* #33767
* #33765
* #33760
* #33759
* #33758
* #33751
* __->__ #33752
* #33753
2025-07-24 15:36:54 -07:00
Joseph Savona 6f4294af9b [compiler] Validate against setState in all effect types (#33753)
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33753).
* #33981
* #33777
* #33767
* #33765
* #33760
* #33759
* #33758
* #33751
* #33752
* __->__ #33753
2025-07-24 15:36:40 -07:00
Joseph Savona 28acd25497 Merge 372c33f003 into sapling-pr-archive-josephsavona 2025-07-24 15:31:18 -07:00
Joe Savona 372c33f003 [compiler] Use diagnostic for "found suppression" error 2025-07-24 15:31:12 -07:00
Joe Savona c69fc56b4e merge commit for archive created by Sapling 2025-07-24 15:20:24 -07:00
Joe Savona b150a8ca4b [compiler] Use diagnostic for "found suppression" error 2025-07-24 15:20:12 -07:00
Joseph Savona 448f781a52 [compiler] Fix for false positive mutation of destructured spread object (#33786)
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.
2025-07-24 15:16:28 -07:00
Joseph Savona b6764be5f5 Merge 4e1ad72433 into sapling-pr-archive-josephsavona 2025-07-24 14:50:02 -07:00
Joe Savona 4e1ad72433 [compiler] Use diagnostic for "found suppression" error 2025-07-24 14:48:52 -07:00
Joe Savona dc78fb6edd [compiler][rfc] Enable more validations in playground.
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?
2025-07-24 14:48:50 -07:00
Joe Savona 43477c775a [compiler] Use new diagnostic printing in playground
Per title
2025-07-24 14:48:48 -07:00
Joe Savona 5a51b54541 [compiler] Cleanup diagnostic messages
Minor sytlistic cleanup
2025-07-24 14:48:47 -07:00
Joe Savona e857af4b79 [compiler] Use new diagnostics for core inference errors
Uses the new diagnostic type for errors created during mutation/aliasing inference, such as errors for mutating immutable values like props or state, reassigning globals, etc.
2025-07-24 14:48:42 -07:00
Joe Savona 1f4facce19 [compiler] Update diagnostics for ValidatePreservedManualMemoization
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.
2025-07-24 14:48:40 -07:00
Joe Savona 6a2a9c2278 [compiler] Improve more error messages
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.
2025-07-24 14:48:38 -07:00
Joe Savona 2649d102b8 [compiler][wip] Improve diagnostic infra
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.
2025-07-24 14:48:36 -07:00
Joe Savona 1517c63a78 [compiler] Enable additional lints by default
Enable more validations to help catch bad patterns, but only in the linter. These rules are already enabled by default in the compiler _if_ violations could produce unsafe output.
2025-07-24 14:43:03 -07:00
Joe Savona 5fbe88ab40 [compiler] Validate against setState in all effect types 2025-07-24 14:43:03 -07:00
Joseph Savona 0e2f328ad1 Merge 00f0db31fe into sapling-pr-archive-josephsavona 2025-07-24 14:42:18 -07:00
Sebastian Markbåge 5020d48d28 [DevTools] Feature detect createSidebarPane (#33988)
Same as #33987 but for the sidebar pane creation.
2025-07-24 17:42:06 -04:00
Joe Savona 00f0db31fe [compiler] Fix for false positive mutation of destructured spread object
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.
2025-07-24 14:38:42 -07:00
Joseph Savona 5903bb9e48 Merge c6a236aaee into sapling-pr-archive-josephsavona 2025-07-24 14:38:14 -07:00
Sebastian Markbåge 3082604bdc [DevTools] Feature detect sources panel (#33987)
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.
2025-07-24 17:38:12 -04:00
Joe Savona c6a236aaee [compiler] Fix for edge cases of mutation of potentially frozen values
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.
2025-07-24 14:38:05 -07:00
Joe Savona 6c1072a869 merge commit for archive created by Sapling 2025-07-24 14:23:13 -07:00
Joe Savona fa353c90ba [compiler] Fix for edge cases of mutation of potentially frozen values
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.
2025-07-24 14:22:56 -07:00
Joseph Savona 22b6775478 Merge a581c38306 into sapling-pr-archive-josephsavona 2025-07-24 11:27:12 -07:00
Joe Savona a581c38306 [compiler] Use diagnostic for "found suppression" error 2025-07-24 11:26:58 -07:00
Sebastian Markbåge 4f34cc4a2e [Fiber] Don't throw away the Error object retaining the owner stack (#33976)
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.
2025-07-24 13:33:03 -04:00
Sebastian Markbåge 3d14fcf03f [Flight] Use about: protocol instead of rsc: protocol for fake evals (#33977)
Chrome DevTools Extensions has a silly problem where they block access
to load Resources from all protocols except [an allow
list](https://github.com/ChromeDevTools/devtools-frontend/blob/eb970fbc6482f281b95bbec1c33c1c539f6d50f0/front_end/models/extensions/ExtensionServer.ts#L60).

https://issues.chromium.org/issues/416196401

Even though these are `eval()` and not actually loaded from the network
they're blocked. They can really be any string. We just have to pick one
of:

```js
'http:', 'https:', 'file:', 'data:', 'chrome-extension:', 'about:'
```

That way React DevTools extensions can load this content to source map
them.

Webpack has the same issue with its `webpack://` and
`webpack-internal://` urls.
2025-07-24 11:07:11 -04:00
Sebastian Markbåge edac0dded9 [DevTools] Add a Code Editor Sidebar Pane in the Chrome Sources Tab (#33968)
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"
/>
2025-07-23 10:28:11 -04:00
Sebastian Markbåge 3586a7f9e8 [DevTools] Allow file:/// urls to be opened in editor (#33965)
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.
2025-07-23 10:21:50 -04:00
Sebastian "Sebbie" Silbermann f6fb1a07a5 [Flight] Remove superfluous whitespace when console method is called with non-strings (#33953) 2025-07-23 10:07:37 +02:00