Conversation
Add detailed plan for making the React Compiler fault-tolerant by accumulating errors across all passes instead of stopping at the first error. This enables reporting multiple compilation errors at once.
Add error accumulation methods to the Environment class: - #errors field to accumulate CompilerErrors across passes - recordError() to record a single diagnostic (throws if Invariant) - recordErrors() to record all diagnostics from a CompilerError - hasErrors() to check if any errors have been recorded - aggregateErrors() to retrieve the accumulated CompilerError - tryRecord() to wrap callbacks and catch CompilerErrors
…erance - Change runWithEnvironment/run/compileFn to return Result<CodegenFunction, CompilerError> - Wrap all pipeline passes in env.tryRecord() to catch and record CompilerErrors - Record inference pass errors via env.recordErrors() instead of throwing - Handle codegen Result explicitly, returning Err on failure - Add final error check: return Err(env.aggregateErrors()) if any errors accumulated - Update tryCompileFunction and retryCompileFunction in Program.ts to handle Result - Keep lint-only passes using env.logErrors() (non-blocking) - Update 52 test fixture expectations that now report additional errors This is the core integration that enables fault tolerance: errors are caught, recorded, and the pipeline continues to discover more errors.
…rs on env Update 9 validation passes to record errors directly on fn.env instead of returning Result<void, CompilerError>: - validateHooksUsage - validateNoCapitalizedCalls (also changed throwInvalidReact to recordError) - validateUseMemo - dropManualMemoization - validateNoRefAccessInRender - validateNoSetStateInRender - validateNoImpureFunctionsInRender - validateNoFreezingKnownMutableFunctions - validateExhaustiveDependencies Each pass now calls fn.env.recordErrors() instead of returning errors.asResult(). Pipeline.ts call sites updated to remove tryRecord() wrappers and .unwrap().
… tolerance Update remaining validation passes to record errors on env: - validateMemoizedEffectDependencies - validatePreservedManualMemoization - validateSourceLocations (added env parameter) - validateContextVariableLValues (changed throwTodo to recordError) - validateLocalsNotReassignedAfterRender (changed throw to recordError) - validateNoDerivedComputationsInEffects (changed throw to recordError) Update inference passes: - inferMutationAliasingEffects: return void, errors on env - inferMutationAliasingRanges: return Array<AliasingEffect> directly, errors on env Update codegen: - codegenFunction: return CodegenFunction directly, errors on env - codegenReactiveFunction: same pattern Update Pipeline.ts to call all passes directly without tryRecord/unwrap. Also update AnalyseFunctions.ts which called inferMutationAliasingRanges.
Add test fixture demonstrating fault tolerance: the compiler now reports both a mutation error and a ref access error in the same function, where previously only one would be reported before bailing out. Update plan doc to mark all phases as complete.
Greptile SummaryThis PR adds fault tolerance to the React Compiler, enabling it to continue through compilation passes and report all errors rather than stopping at the first one. The key architectural change is that the
Confidence Score: 4/5
Important Files Changed
Last reviewed commit: f773cbc |
| if ( | ||
| (detail instanceof CompilerDiagnostic | ||
| ? detail.category | ||
| : detail.category) === ErrorCategory.Invariant | ||
| ) { |
There was a problem hiding this comment.
Redundant ternary expression
The ternary (detail instanceof CompilerDiagnostic ? detail.category : detail.category) evaluates to detail.category in both branches. Both CompilerDiagnostic and CompilerErrorDetail expose category as a direct property, so the instanceof check has no effect.
| if ( | |
| (detail instanceof CompilerDiagnostic | |
| ? detail.category | |
| : detail.category) === ErrorCategory.Invariant | |
| ) { | |
| if (detail.category === ErrorCategory.Invariant) { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
Line: 220-224
Comment:
**Redundant ternary expression**
The ternary `(detail instanceof CompilerDiagnostic ? detail.category : detail.category)` evaluates to `detail.category` in both branches. Both `CompilerDiagnostic` and `CompilerErrorDetail` expose `category` as a direct property, so the `instanceof` check has no effect.
```suggestion
if (detail.category === ErrorCategory.Invariant) {
```
How can I resolve this? If you propose a fix, please make it concise.|
Upstream PR was closed or merged. Code is synced via branch mirror. |
Mirror of facebook/react#35835
Original author: josephsavona
Stack created with Sapling. Best reviewed with ReviewStack.