[compiler] Aggregate error reporting, separate eslint rules #34175
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Aug 11, 2025
github-actions
bot
added
the
React Core Team
|
Comparing: ac7820a...54c9aac Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
All error messages that are not invariants, todos, or config validation errors are now moved to a registry. This helps us check that error messages and linked documentation is uniform and up to date. This also lets us link error codes to linter reporting categories and break the single `react-compiler` rule up into many smaller rules. Changes to error reporting in React Compiler: - instead of writing raw error message, add a new ErrorCode to `CompilerErrorCodes` - create errors by constructing a `CompilerErrorDetailOptions` with the correct error code, or calling `CompilerErrorDetail.fromCode()` / `CompilerDiagnostic.fromCode()`. Changes to linter: - `react-compiler` lint rule is now broken up into many smaller lint rules, each with their own test cases. - linting no longer fails early when a non-fatal error is find. Instead, we now log and move on to lint for more errors! Todos: - Codebases previously using the eslint plugin may already have `react-compiler` eslint suppressions. Since this PR removes this rule, these codebases may get new eslint warnings on already-suppressed ranges. Some options I can think of: - Force everyone to add the new suppressions - Release a script to rewrite previous suppressions to the new suppressions. Codebases that had incomplete suppressions / waited to update may benefit here, as this still surfaces errors on previously-unseen suppressions - Hard code logic in our eslint plugin to avoid reporting if we see a `eslint-disable-next-line react-compiler`. This doesn't feel ideal - Align on eslint plugin rule names and error code mappings - I aggregated a few error codes here. For example, reassigning and modifying local variables after render now have the same error code + message. See changes fixtures for more examples - More test cases for linter rules. Will add these once lint rules are more finalized
josephsavona
changed the title
Leotaby
reviewed
Aug 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
All error messages that are not invariants, todos, or config validation errors are now moved to a registry. This helps us check that error messages and linked documentation is uniform and up to date. This also lets us link error codes to linter reporting categories and break the single
react-compilerrule up into many smaller rules.Changes to error reporting in React Compiler:
CompilerErrorCodesCompilerErrorDetailOptionswith the correct error code, or callingCompilerErrorDetail.fromCode()/CompilerDiagnostic.fromCode().Changes to linter:
react-compilerlint rule is now broken up into many smaller lint rules, each with their own test cases.Todos:
react-compilereslint suppressions. Since this PR removes this rule, these codebases may get new eslint warnings on already-suppressed ranges. Some options I can think of:eslint-disable-next-line react-compiler. This doesn't feel idealStack created with Sapling. Best reviewed with ReviewStack.