Skip to content

[compiler] Add fault tolerance plan document#549

Closed
everettbu wants to merge 1 commit into
mainfrom
pr35828
Closed

[compiler] Add fault tolerance plan document#549
everettbu wants to merge 1 commit into
mainfrom
pr35828

Conversation

@everettbu

@everettbu everettbu commented Feb 20, 2026

Copy link
Copy Markdown

Mirror of facebook/react#35828
Original author: josephsavona


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.


Stack created with Sapling. Best reviewed with ReviewStack.

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

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.
@everettbu everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 20, 2026
@greptile-apps

greptile-apps Bot commented Feb 20, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a detailed plan document for making the React Compiler fault-tolerant by accumulating errors across all compilation passes instead of halting at the first error. The plan outlines 8 phases covering environment infrastructure, pipeline changes, HIR lowering, validation/inference pass updates, codegen, integration, and testing.

  • The plan is well-structured and technically accurate based on the current codebase (e.g., lower() returning Result<HIRFunction, CompilerError>, validation passes using .unwrap(), runWithEnvironment returning CodegenFunction directly)
  • Proposes an incremental migration path using env.tryRecord() as a transitional safety net, allowing phases to be landed independently
  • Minor typo on line 20 ("construct record" should be "record" or "construct and record")
  • Pattern labeling skips from "Pattern B" to "Pattern D" (no "Pattern C")

Confidence Score: 4/5

  • This PR is safe to merge — it only adds a planning document with no code changes.
  • Score of 4 reflects that this is a documentation-only PR with no runtime impact. The plan document is technically sound and aligns with the actual codebase structure. Minor issues (a typo and a skipped pattern label) are non-blocking.
  • No files require special attention — this is a documentation-only change.

Important Files Changed

Filename Overview
compiler/fault-tolerance-overview.md New plan document detailing an 8-phase approach for making the React Compiler fault-tolerant. Technically sound and well-structured; minor typo on line 20.

Last reviewed commit: d30d189

@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.

1 file reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

## Detailed Design

* The Environment needs a way to record errors as compilation proceeds. This should generally store the error (and log, if a logger is configured), but should immediately throw if the error is an invariant (see above).
* BuildHIR should always produce an HIR without error. For syntax forms that are unsupported (currently throwing a Todo error), we should instead construct record the todo error on the environment, and construct a partial HIR. The exact form of the partial HIR can be situation specific:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Typo: duplicate word
"we should instead construct record the todo error" appears to have an extra word. Should this be "we should instead record the todo error" or "we should instead construct and record the todo error"?

Suggested change
* BuildHIR should always produce an HIR without error. For syntax forms that are unsupported (currently throwing a Todo error), we should instead construct record the todo error on the environment, and construct a partial HIR. The exact form of the partial HIR can be situation specific:
* BuildHIR should always produce an HIR without error. For syntax forms that are unsupported (currently throwing a Todo error), we should instead record the todo error on the environment, and construct a partial HIR. The exact form of the partial HIR can be situation specific:
Prompt To Fix With AI
This is a comment left during a code review.
Path: compiler/fault-tolerance-overview.md
Line: 20

Comment:
**Typo: duplicate word**
"we should instead construct record the todo error" appears to have an extra word. Should this be "we should instead **record** the todo error" or "we should instead **construct and record** the todo error"?

```suggestion
* BuildHIR should always produce an HIR without error. For syntax forms that are unsupported (currently throwing a Todo error), we should instead record the todo error on the environment, and construct a partial HIR. The exact form of the partial HIR can be situation specific:
```

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

- [ ] **4.15 `validateNoJSXInTryStatement`** — change to record on env directly
- [ ] **4.16 `validateStaticComponents`** — change to record on env directly

#### Pattern D passes (currently throw directly, no Result)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing "Pattern C" label
The document labels error handling patterns as "Pattern A", "Pattern B", and "Pattern D", skipping "Pattern C". Was there an intended Pattern C that was removed, or should "Pattern D" be relabeled to "Pattern C" for consistency?

Prompt To Fix With AI
This is a comment left during a code review.
Path: compiler/fault-tolerance-overview.md
Line: 201

Comment:
**Missing "Pattern C" label**
The document labels error handling patterns as "Pattern A", "Pattern B", and "Pattern D", skipping "Pattern C". Was there an intended Pattern C that was removed, or should "Pattern D" be relabeled to "Pattern C" for consistency?

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

@everettbu

Copy link
Copy Markdown
Author

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

@everettbu everettbu closed this Feb 21, 2026
@everettbu
everettbu deleted the pr35828 branch February 21, 2026 04:00
@everettbu
everettbu restored the pr35828 branch February 21, 2026 04:38
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.

2 participants