[compiler] Remove local CompilerError accumulators, emit directly to env.recordError() by everettbu · Pull Request #599 · greptileai/react-mirror

everettbu · 2026-02-23T18:33:03Z

Mirror of facebook/react#35882
Original author: josephsavona

Removes unnecessary indirection in 17 compiler passes that previously
accumulated errors in a local CompilerError instance before flushing
them to env.recordErrors() at the end of each pass. Errors are now
emitted directly via env.recordError() as they're discovered.

For passes with recursive error-detection patterns (ValidateNoRefAccessInRender,
ValidateNoSetStateInRender), the internal accumulator is kept but flushed
via individual recordError() calls. For InferMutationAliasingRanges,
a shouldRecordErrors flag preserves the conditional suppression logic.
For TransformFire, the throw-based error propagation is replaced with
direct recording plus an early-exit check in Pipeline.ts.

Stack created with Sapling. Best reviewed with ReviewStack.

#35888
#35884
#35883
-> #35882
#35881
#35880
#35879
#35878
#35877
#35876
#35875
#35874

greptile-apps · 2026-02-23T18:38:55Z

Greptile Summary

This PR removes unnecessary indirection in 17 compiler passes that previously accumulated errors in local CompilerError instances before flushing them to env.recordErrors() at the end of each pass. Errors are now emitted directly via env.recordError() as they are discovered.

HIRBuilder and BuildHIR: The errors: CompilerError field is removed from HIRBuilder and replaced with a recordError() method that delegates to the environment. ~50 call sites in BuildHIR.ts are updated from builder.errors.push() / builder.errors.pushDiagnostic() to builder.recordError() with explicit CompilerErrorDetail or CompilerDiagnostic construction.
CodegenReactiveFunction: Same pattern — Context.errors field replaced with recordError() method, 5 call sites updated.
InferMutationAliasingRanges: Uses a shouldRecordErrors flag to preserve conditional suppression logic (!isFunctionExpression && fn.env.enableValidations). Internal helpers now accept Environment | null instead of CompilerError.
ValidateNoRefAccessInRender / ValidateNoSetStateInRender: Recursive passes that keep their internal CompilerError accumulator but flush errors individually via recordError() instead of batch recordErrors().
11 validation passes: Straightforward removal of local CompilerError accumulators and batch flush calls, replaced with direct env.recordError() at each error site.
All imports are correctly updated — unused CompilerError imports removed, necessary CompilerErrorDetail/Environment imports added.

Confidence Score: 5/5

This PR is a safe, mechanical refactor that eliminates unnecessary error accumulation indirection with no behavioral changes.
All 16 files follow a consistent, well-understood pattern: replacing local CompilerError accumulators + batch flush with direct env.recordError() calls. The Environment.recordError() method handles all the same logic (invariant errors still throw, other errors accumulate). No conditional logic changes, no new features, no behavioral differences. The three special cases (ValidateNoRefAccessInRender, ValidateNoSetStateInRender with internal accumulators; InferMutationAliasingRanges with conditional suppression) are handled correctly. Imports are clean across all files.
No files require special attention

Important Files Changed

Filename	Overview
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts	Largest change in the PR. Replaces ~50 `builder.errors.push()` / `builder.errors.pushDiagnostic()` calls with `builder.recordError()` using `CompilerErrorDetail` or `CompilerDiagnostic`. Removes the batch `env.recordErrors(builder.errors)` at end of `lower()`. Also replaces `builder.errors.hasAnyErrors()` with `builder.environment.hasErrors()` for the defensive invariant check.
compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts	Removes `errors: CompilerError` field from HIRBuilder. Adds `recordError()` method that delegates to `this.#env.recordError()`. Converts 3 inline error pushes to use `new CompilerErrorDetail()` via the new method.
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts	Replaces local `errors` accumulator with `shouldRecordErrors` flag and direct `fn.env.recordError()` calls. Passes `Environment
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts	Removes `errors: CompilerError` field from Context class. Adds `recordError()` method delegating to `this.env.recordError()`. Converts 5 `cx.errors.push()` calls to use `new CompilerErrorDetail()` via the new method. Removes batch flush at end of `codegenReactiveFunction`.
compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts	Replaces local `errors` with direct `func.env.recordError()` calls. Changes `extractManualMemoizationArgs` to accept `env: Environment` instead of `errors: CompilerError`. Removes batch flush at end.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts	Replaces local `error` accumulator with direct `fn.env.recordError()` calls. Removes batch flush at end.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateHooksUsage.ts	Replaces local `errors` with direct `fn.env.recordError()` calls. Renames internal `recordError` to `trackError` to avoid name collision. Changes `visitFunctionExpression` to accept `env: Environment` parameter instead of `errors: CompilerError`. Removes batch flush.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoCapitalizedCalls.ts	Replaces local `errors` with direct `fn.env.recordError()` using `new CompilerErrorDetail()`. Removes batch flush at end.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts	Replaces local `errors` with direct `env.recordError()` using `new CompilerErrorDetail()`. Changes `validateEffect` to accept `env: Environment` instead of `errors: CompilerError`. Removes unconditional batch flush.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoFreezingKnownMutableFunctions.ts	Replaces local `errors` with direct `fn.env.recordError()` calls. Removes batch flush at end.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureFunctionsInRender.ts	Replaces local `errors` with direct `fn.env.recordError()` calls. Removes batch flush at end.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts	Keeps internal `CompilerError` accumulator for recursive error collection, but flushes errors individually via `fn.env.recordError()` instead of `fn.env.recordErrors()`.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInRender.ts	Same pattern as ValidateNoRefAccessInRender — keeps internal accumulator but flushes errors individually via `fn.env.recordError()`.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts	Replaces `errors: CompilerError` in `VisitorState` with `env: Environment`. Changes `validateInferredDep` to accept `Environment` instead of `CompilerError`. Converts `errors.pushDiagnostic()` calls to `env.recordError()`. Removes batch flush.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateSourceLocations.ts	Replaces local `errors` with direct `env.recordError()` calls. Removes batch flush at end.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts	Replaces local `errors` with direct `fn.env.recordError()` calls. Changes `validateNoContextVariableAssignment` to accept `Environment`. Keeps `voidMemoErrors` accumulator since it uses `logErrors()` (a different path). Removes batch flush.

_{Last reviewed commit: 6f53c39}

greptile-apps

_{79 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-23T18:38:59Z

+    if (this.logger != null) {
+      this.logger.logEvent(this.filename, {
+        kind: 'CompileError',
+        detail: error,
+        fnLoc: null,
+      });
+    }


Duplicate error logging with Program.ts
recordError() logs a CompileError event here with fnLoc: null. Later, when Program.ts receives the aggregated error via Err(env.aggregateErrors()), it calls logError() which logs the same error details again — this time with the actual fnLoc. This is visible in the dynamic-gating-bailout-nopanic test fixture, which now shows two log entries for the same error (one with fnLoc: null, one with the real location).

This may be intentional for early visibility during compilation, but it doubles the log output for every recorded error. Consider either: (1) removing the logging here and letting Program.ts handle all logging, or (2) removing the logging in Program.ts's logError for errors that come back via the Result path (as opposed to caught exceptions).

Prompt To Fix With AI

This is a comment left during a code review. Path: compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts Line: 737-743 Comment: **Duplicate error logging with `Program.ts`** `recordError()` logs a `CompileError` event here with `fnLoc: null`. Later, when `Program.ts` receives the aggregated error via `Err(env.aggregateErrors())`, it calls `logError()` which logs the same error details again — this time with the actual `fnLoc`. This is visible in the `dynamic-gating-bailout-nopanic` test fixture, which now shows two log entries for the same error (one with `fnLoc: null`, one with the real location). This may be intentional for early visibility during compilation, but it doubles the log output for every recorded error. Consider either: (1) removing the logging here and letting `Program.ts` handle all logging, or (2) removing the logging in `Program.ts`'s `logError` for errors that come back via the `Result` path (as opposed to caught exceptions). How can I resolve this? If you propose a fix, please make it concise.

greptile-apps

_{79 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps

_{78 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps

_{78 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps

_{78 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-23T23:29:25Z

-              description: `Handle '${value.kind} lvalues`,
-              suggestions: null,
-            });
+            fn.env.recordError(


Inconsistent env reference
This line uses fn.env.recordError() while the rest of the function uses the env parameter (e.g., line 110). Both refer to the same Environment instance since the compiler shares a single env across nested functions, so this isn't a bug, but it's inconsistent with the pattern in the rest of this function.

Suggested change

fn.env.recordError(

env.recordError(

Prompt To Fix With AI

This is a comment left during a code review. Path: compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateContextVariableLValues.ts Line: 74 Comment: **Inconsistent env reference** This line uses `fn.env.recordError()` while the rest of the function uses the `env` parameter (e.g., line 110). Both refer to the same `Environment` instance since the compiler shares a single env across nested functions, so this isn't a bug, but it's inconsistent with the pattern in the rest of this function. ```suggestion env.recordError( ``` How can I resolve this? If you propose a fix, please make it concise.

…env.recordError() Removes unnecessary indirection in 17 compiler passes that previously accumulated errors in a local `CompilerError` instance before flushing them to `env.recordErrors()` at the end of each pass. Errors are now emitted directly via `env.recordError()` as they're discovered. For passes with recursive error-detection patterns (ValidateNoRefAccessInRender, ValidateNoSetStateInRender), the internal accumulator is kept but flushed via individual `recordError()` calls. For InferMutationAliasingRanges, a `shouldRecordErrors` flag preserves the conditional suppression logic. For TransformFire, the throw-based error propagation is replaced with direct recording plus an early-exit check in Pipeline.ts.

everettbu · 2026-02-24T00:36:27Z

Upstream PR was closed or merged. Code is synced via branch mirror.

greptile-apps

_{16 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 23, 2026