Skip to content

[compiler] Improve hook validation diagnostics#130

Open
everettbu wants to merge 2 commits into
mainfrom
pr34246
Open

[compiler] Improve hook validation diagnostics#130
everettbu wants to merge 2 commits into
mainfrom
pr34246

Conversation

@everettbu

@everettbu everettbu commented Dec 12, 2025

Copy link
Copy Markdown

Mirror of facebook/react#34246
Original author: josephsavona



Stack created with Sapling. Best reviewed with ReviewStack.

  • -> #34246
  • #34176

mofeiZ and others added 2 commits August 20, 2025 17:37
NOTE: this is a merged version of @mofeiZ's original PR along with my edits per offline discussion. The description is updated to reflect the latest approach.

The key problem we're trying to solve with this PR is to allow developers more control over the compiler's various validations. The idea is to have a number of rules targeting a specific category of issues, such as enforcing immutability of props/state/etc or disallowing access to refs during render. We don't want to have to run the compiler again for every single rule, though, so @mofeiZ added an LRU cache that caches the full compilation output of N most recent files. The first rule to run on a given file will cause it to get cached, and then subsequent rules can pull from the cache, with each rule filtering down to its specific category of errors.

For the categories, I went through and assigned a category roughly 1:1 to existing validations, and then used my judgement on some places that felt distinct enough to warrant a separate error. Every error in the compiler now has to supply both a severity (for legacy reasons) and a category (for ESLint). Each category corresponds 1:1 to a ESLint rule definition, so that the set of rules is automatically populated based on the defined categories.

Categories include a flag for whether they should be in the recommended set or not.

Note that as with the original version of this PR, only eslint-plugin-react-compiler is changed. We still have to update the main lint rule.
@everettbu everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Dec 12, 2025
@greptile-apps

greptile-apps Bot commented Dec 12, 2025

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

Refactored compiler diagnostic system to improve hook validation error messages and consistency across the codebase.

Key changes:

  • Introduced structured reason and description fields to CompilerDiagnostic, separating concise error summaries from detailed explanations
  • Migrated category field from free-form strings to typed ErrorCategory enum for consistency
  • Updated ValidateHooksUsage.ts to use new diagnostic structure with clearer, more actionable error messages
  • Added getRuleForCategory() mapping function to associate error categories with ESLint rule metadata
  • Updated 100+ test fixtures to reflect improved error message formatting
  • Refactored ESLint plugin to extract shared compiler execution logic into RunReactCompiler module
  • Removed duplicate React Compiler rule from eslint-plugin-react-hooks package

Confidence Score: 5/5

  • This PR is safe to merge - it's a well-structured refactoring that improves diagnostic quality without changing validation logic.
  • This is a comprehensive but clean refactoring focused on improving the diagnostic/error reporting infrastructure. The changes are systematic (migrating from CompilerErrorDetail to CompilerDiagnostic, adding reason field, enum-ifying category), extensively tested (141 test fixture updates), and don't alter the underlying validation logic. The separation of concerns between reason and description improves error message clarity.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateHooksUsage.ts 5/5 Refactored error reporting from CompilerErrorDetail to CompilerDiagnostic with structured reason/description separation. Improved error messages with clearer summaries.
compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts 5/5 Major refactoring: added reason field to diagnostics, changed category from string to ErrorCategory enum, and added getRuleForCategory() mapping function.
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts 5/5 Updated all error creation sites to add category field and reason field, migrating from strings to ErrorCategory enum values.
compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts 5/5 Refactored to extract React compiler execution logic into shared RunReactCompiler module, reducing code duplication.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

139 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants