Skip to content

[compiler] Use diagnostic for "found suppression" error #33981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 24, 2025
Merged

[compiler] Use diagnostic for "found suppression" error #33981

merged 1 commit into from
Jul 24, 2025

Conversation

josephsavona
Copy link
Member

@josephsavona josephsavona commented Jul 24, 2025 •
edited
Loading

No description provided.

This was referenced Jul 24, 2025
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Jul 24, 2025
mofeiZ
mofeiZ reviewed Jul 24, 2025
View reviewed changes
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts Outdated
error.pushDiagnostic(
CompilerDiagnostic.create({
category: reason,
description: `React Compiler only works when your components follow all the rules of React, disabling them may result in unexpected or incorrect behavior. Found suppresion \`${suppressionRange.disableComment.value.trim()}\``,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typo Found suppresion -> Found suppression

mofeiZ
mofeiZ reviewed Jul 24, 2025
View reviewed changes
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts
Comment on lines +200 to +202
}).withDetail({
kind: 'error',
loc: suppressionRange.disableComment.loc ?? null,
message: 'Found React rule suppression',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! this looks much better

@josephsavona josephsavona force-pushed the pr33981 branch 3 times, most recently from b150a8c to 372c33f Compare July 24, 2025 22:31
josephsavona added a commit that referenced this pull request Jul 24, 2025
@josephsavona
[compiler] Validate against setState in all effect types (#33753)
6f4294a 
---
[//]: # (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
josephsavona added a commit that referenced this pull request Jul 24, 2025
@josephsavona
[compiler] Enable additional lints by default (#33752)
0d39496 
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
josephsavona added a commit that referenced this pull request Jul 24, 2025
@josephsavona
[compiler][wip] Improve diagnostic infra (#33751)
707e321 
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
josephsavona added a commit that referenced this pull request Jul 24, 2025
@josephsavona
[compiler] Improve more error messages (#33758)
7284802 
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
josephsavona added a commit that referenced this pull request Jul 24, 2025
@josephsavona
[compiler] Update diagnostics for ValidatePreservedManualMemoization (#…
48bc166 
…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
josephsavona added a commit that referenced this pull request Jul 24, 2025
@josephsavona
[compiler] Use new diagnostics for core inference errors (#33760)
a39da6c 
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.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33760).
* #33981
* #33777
* #33767
* #33765
* __->__ #33760
josephsavona added a commit that referenced this pull request Jul 24, 2025
@josephsavona
[compiler] Cleanup diagnostic messages (#33765)
7f51055 
Minor sytlistic cleanup

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33765).
* #33981
* #33777
* #33767
* __->__ #33765
josephsavona added a commit that referenced this pull request Jul 24, 2025
@josephsavona
[compiler] Use new diagnostic printing in playground (#33767)
2ae8b3d 
Per title

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33767).
* #33981
* #33777
* __->__ #33767
josephsavona added a commit that referenced this pull request Jul 24, 2025
@josephsavona
[compiler][rfc] Enable more validations in playground. (#33777)
bcea869 
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
@josephsavona josephsavona merged commit 129aa85 into main Jul 24, 2025
11 of 13 checks passed
github-actions bot pushed a commit that referenced this pull request Jul 24, 2025
@josephsavona
[compiler] Enable additional lints by default (#33752)
e671c74 
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

DiffTrain build for [0d39496](0d39496)
github-actions bot pushed a commit that referenced this pull request Jul 24, 2025
@josephsavona
[compiler] Enable additional lints by default (#33752)
3b71e2f 
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

DiffTrain build for [0d39496](0d39496)
@nextjs-bot nextjs-bot mentioned this pull request Jul 25, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mofeiZ mofeiZ mofeiZ approved these changes

Assignees
No one assigned
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@josephsavona @mofeiZ @facebook-github-bot