Skip to content

[compiler] Enable additional lints by default #33752

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 2 commits into from
Jul 24, 2025
Merged

[compiler] Enable additional lints by default #33752

merged 2 commits into from
Jul 24, 2025

Conversation

josephsavona
Copy link
Member

@josephsavona josephsavona commented Jul 10, 2025 •
edited
Loading

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.

Stack created with Sapling. Best reviewed with ReviewStack.

This was referenced Jul 10, 2025
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 10, 2025
josephsavona added a commit that referenced this pull request Jul 10, 2025
@josephsavona
[compiler] Errors for eval(), with statments, class declarations (#33746
4a3ff8e 
)

* Error for `eval()`
* More specific error message for `with (expr) { ... }` syntax
* More specific error message for class declarations

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33746).
* #33752
* #33751
* #33750
* #33748
* #33747
* __->__ #33746
@josephsavona josephsavona mentioned this pull request Jul 10, 2025
Merged
josephsavona added a commit that referenced this pull request Jul 10, 2025
@josephsavona
[compiler] Support inline enums (flow/ts), type declarations (#33747)
81e1ee7 
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
josephsavona added a commit that referenced this pull request Jul 10, 2025
@josephsavona
[compiler] More precise errors for invalid import/export/namespace st…
0bfa404 
…atements (#33748)

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
josephsavona added a commit that referenced this pull request Jul 10, 2025
@josephsavona
[compiler] Add CompilerError.UnsupportedJS variant (#33750)
96c61b7 
We use this variant for syntax we intentionally don't support: with
statements, eval, and inline class declarations.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33750).
* #33753
* #33752
* #33751
* __->__ #33750
* #33748
github-actions bot pushed a commit that referenced this pull request Jul 10, 2025
@josephsavona
[compiler] Errors for eval(), with statments, class declarations (#33746
0116228 
)

* Error for `eval()`
* More specific error message for `with (expr) { ... }` syntax
* More specific error message for class declarations

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33746).
* #33752
* #33751
* #33750
* #33748
* #33747
* __->__ #33746

DiffTrain build for [4a3ff8e](4a3ff8e)
github-actions bot pushed a commit that referenced this pull request Jul 10, 2025
@josephsavona
[compiler] Errors for eval(), with statments, class declarations (#33746
1f5d41f 
)

* Error for `eval()`
* More specific error message for `with (expr) { ... }` syntax
* More specific error message for class declarations

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33746).
* #33752
* #33751
* #33750
* #33748
* #33747
* __->__ #33746

DiffTrain build for [4a3ff8e](4a3ff8e)
github-actions bot pushed a commit that referenced this pull request Jul 10, 2025
@josephsavona
[compiler] Add CompilerError.UnsupportedJS variant (#33750)
4ab994b 
We use this variant for syntax we intentionally don't support: with
statements, eval, and inline class declarations.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33750).
* #33753
* #33752
* #33751
* __->__ #33750
* #33748

DiffTrain build for [96c61b7](96c61b7)
github-actions bot pushed a commit that referenced this pull request Jul 10, 2025
@josephsavona
[compiler] Add CompilerError.UnsupportedJS variant (#33750)
5638224 
We use this variant for syntax we intentionally don't support: with
statements, eval, and inline class declarations.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33750).
* #33753
* #33752
* #33751
* __->__ #33750
* #33748

DiffTrain build for [96c61b7](96c61b7)
@josephsavona josephsavona force-pushed the pr33752 branch 2 times, most recently from 737dd9c to 7f64e97 Compare July 10, 2025 15:21
@react-sizebot
Copy link

react-sizebot commented Jul 10, 2025 •
edited
Loading

Comparing: 5020d48...1517c63

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB +0.05% 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 530.70 kB 530.70 kB = 93.70 kB 93.70 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 655.25 kB 655.25 kB = 115.40 kB 115.40 kB
facebook-www/ReactDOM-prod.classic.js = 675.13 kB 675.13 kB = 118.75 kB 118.75 kB
facebook-www/ReactDOM-prod.modern.js = 665.56 kB 665.56 kB = 117.11 kB 117.12 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 1517c63

@josephsavona josephsavona force-pushed the pr33752 branch 2 times, most recently from ce0e8f7 to 6750584 Compare July 10, 2025 17:18
This was referenced Jul 10, 2025
Merged
Merged
Merged
Merged
@josephsavona
Copy link
Member Author

I ran the latest linter against our internal codebase, Meta folks can check at P1867417094. This really just confirms that we're not seeing unexpected cases beyond what we anticipated in the test coverage.

@josephsavona josephsavona mentioned this pull request Jul 12, 2025
Merged
@josephsavona josephsavona mentioned this pull request Jul 12, 2025
Merged
@josephsavona josephsavona mentioned this pull request Jul 24, 2025
Merged
mofeiZ
mofeiZ reviewed Jul 24, 2025
View reviewed changes
compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts
@@ -107,6 +107,12 @@ const COMPILER_OPTIONS: Partial<PluginOptions> = {
flowSuppressions: false,
environment: validateEnvironmentConfig({
validateRefAccessDuringRender: false,
validateNoSetStateInRender: true,
validateNoSetStateInEffects: true,
validateNoJSXInTryStatements: true,
Copy link
Contributor

@mofeiZ mofeiZ Jul 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Makes sense, even for potentially valid code like the following, we can just direct devs to move the JSX creation out of the try statement

function Component();
  try {
    const value = tryGetData();
    return <Foo value={value} />;
  } catch {
    return <Fallback />;
  }
}

@josephsavona
[compiler] Validate against setState in all effect types
5fbe88a
@josephsavona
[compiler] Enable additional lints by default
1517c63 
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.
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 josephsavona merged commit 0d39496 into main Jul 24, 2025
260 of 285 checks passed
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
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.
4 participants
@josephsavona @react-sizebot @mofeiZ @facebook-github-bot