[compiler] Phase 1: Add error accumulation infrastructure to Environment

[everettbu]

Mirror of facebook/react#35829
Original author: josephsavona

---

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

---

Stack created with Sapling. Best reviewed with ReviewStack.

• #35838
• #35837
• #35836
• #35835
• #35834
• #35833
• #35832
• #35831
• #35830
• -> #35829
• #35828

[greptile-apps]

Greptile Summary

This PR adds Phase 1 of the React Compiler fault tolerance initiative: error accumulation infrastructure on the Environment class. It introduces methods (recordError, recordErrors, hasErrors, aggregateErrors, tryRecord) that allow compiler passes to record errors without halting, while correctly re-throwing invariant errors that represent internal bugs. A comprehensive plan document (fault-tolerance-overview.md) outlines the full 8-phase migration strategy.

• The code changes are well-structured and consistent with existing patterns (e.g., the logging in recordError mirrors the existing logErrors method)
• tryRecord correctly checks for invariant errors upfront before partially recording, preventing inconsistent state
• hasErrors() delegates to hasAnyErrors() which includes all severities (warnings, hints) — this should be verified as intentional before Phase 2 uses it to determine pipeline success/failure

Confidence Score: 4/5

• This PR is safe to merge — it adds new infrastructure methods with no callers yet, so existing behavior is unchanged.
• Phase 1 only adds new methods and a private field to Environment. No existing code paths are modified, and the new methods are not yet called by any passes or the pipeline. The implementation correctly handles invariant errors and is consistent with existing type contracts.
• No files require special attention. The Environment.ts changes are additive and the plan document is informational.

Important Files Changed

Filename	Overview
compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts	Adds error accumulation infrastructure (#errors field, recordError, recordErrors, hasErrors, aggregateErrors, tryRecord) to the Environment class. The implementation is clean and correctly handles invariant errors by re-throwing. Minor naming consideration: hasErrors() delegates to hasAnyErrors() which includes warnings/hints, not just errors.
compiler/fault-tolerance-overview.md	Comprehensive design document for the fault tolerance initiative. Covers 8 phases from environment infrastructure through testing. Phase 1 items are marked as complete, matching the code changes in this PR.

_{Last reviewed commit: af88b0b}

[greptile-apps]

_{2 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

hasErrors() returns true for warnings and hints too

hasErrors() delegates to CompilerError.hasAnyErrors(), which returns true when details.length > 0 — this includes diagnostics of any severity (Error, Warning, Hint). This is different from CompilerError.hasErrors() (no "Any" prefix) which only checks for ErrorSeverity.Error.

In Phase 2 (per the plan doc), the pipeline will use env.hasErrors() to decide between Ok(ast) and Err(aggregatedErrors). If a pass records only warnings/hints, hasErrors() will return true and the pipeline will return Err(...), potentially causing a bailout for non-error diagnostics. If this is intentional (i.e., any recorded diagnostic should prevent emitting compiled output), the naming may be worth a doc comment clarifying the semantics. If not, consider delegating to this.#errors.hasErrors() instead.

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: 1011-1013

Comment:
**`hasErrors()` returns true for warnings and hints too**

`hasErrors()` delegates to `CompilerError.hasAnyErrors()`, which returns `true` when `details.length > 0` — this includes diagnostics of any severity (Error, Warning, Hint). This is different from `CompilerError.hasErrors()` (no "Any" prefix) which only checks for `ErrorSeverity.Error`.

In Phase 2 (per the plan doc), the pipeline will use `env.hasErrors()` to decide between `Ok(ast)` and `Err(aggregatedErrors)`. If a pass records only warnings/hints, `hasErrors()` will return `true` and the pipeline will return `Err(...)`, potentially causing a bailout for non-error diagnostics. If this is intentional (i.e., any recorded diagnostic should prevent emitting compiled output), the naming may be worth a doc comment clarifying the semantics. If not, consider delegating to `this.#errors.hasErrors()` instead.

How can I resolve this? If you propose a fix, please make it concise.

[everettbu]

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