Skip to content

[compiler] Deduplicate errors between ValidateExhaustiveDependencies and ValidatePreservedManualMemoization#626

Closed
everettbu wants to merge 1 commit into
mainfrom
pr35917
Closed

[compiler] Deduplicate errors between ValidateExhaustiveDependencies and ValidatePreservedManualMemoization#626
everettbu wants to merge 1 commit into
mainfrom
pr35917

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#35917
Original author: josephsavona


With the recent changes to make the compiler fault tolerant and always continue through all passes, we can now sometimes report duplicative errors. Specifically, when ValidateExhaustiveDependencies finds incorrect deps for a useMemo/useCallback call, ValidatePreservedManualMemoization will generally also error for the same block, producing duplicate errors. The exhaustive deps error is strictly more informative, so if we've already reported the earlier error we don't need the later one.

This adds a hasInvalidDeps flag to StartMemoize that is set when ValidateExhaustiveDependencies produces a diagnostic. ValidatePreservedManualMemoization then skips validation for memo blocks with this flag set.

…and ValidatePreservedManualMemoization

When ValidateExhaustiveDependencies finds incorrect deps for a useMemo/useCallback call, ValidatePreservedManualMemoization later also fires for the same memo block, producing duplicate errors. The ExhaustiveDeps error is strictly more informative (names missing/extra deps, suggests a fix).

This adds a `hasInvalidDeps` flag to StartMemoize that is set when ValidateExhaustiveDependencies produces a diagnostic. ValidatePreservedManualMemoization then skips all three of its error sites for memo blocks where this flag is set.
@everettbu everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 26, 2026
@greptile-apps

greptile-apps Bot commented Feb 26, 2026

Copy link
Copy Markdown

Greptile Summary

Deduplicates error reporting between ValidateExhaustiveDependencies and ValidatePreservedManualMemoization by adding a hasInvalidDeps flag to StartMemoize. When the exhaustive deps pass detects invalid dependencies, it sets this flag, and the preserved manual memoization pass skips validation for that memo block — eliminating redundant errors that provided less informative diagnostics.

  • Adds hasInvalidDeps?: true optional field to the StartMemoize HIR type
  • Sets the flag in ValidateExhaustiveDependencies when a diagnostic is recorded
  • ValidatePreservedManualMemoization early-returns from both StartMemoize and FinishMemoize handlers when the flag is set
  • Updates 6 test fixture expectations to reflect reduced error counts

Confidence Score: 5/5

  • This PR is safe to merge — it's a clean, well-scoped improvement to error deduplication with no behavioral changes beyond reducing redundant diagnostics.
  • The change is minimal and well-contained: a single boolean flag propagated between two validation passes. The pipeline ordering (exhaustive deps at line 275, preserved memo at line 502) guarantees the flag is set before it's read. All intermediate state accesses in the visitor are properly null-guarded, so skipped memo blocks don't cause invariant violations. Test expectations are updated consistently, and the removed errors are strictly duplicates of more informative ones retained.
  • No files require special attention.

Important Files Changed

Filename Overview
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts Adds optional hasInvalidDeps?: true flag to StartMemoize type for cross-pass communication between validation phases.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts Sets hasInvalidDeps = true on StartMemoize when a dependency validation diagnostic is recorded, signaling downstream passes.
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts Skips validation for memo blocks with hasInvalidDeps, early-returning from both StartMemoize and FinishMemoize handlers to avoid duplicate errors. All intermediate state accesses are properly guarded.

Last reviewed commit: 596e9e8

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

9 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@everettbu

Copy link
Copy Markdown
Author

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

@everettbu everettbu closed this Feb 26, 2026
@everettbu
everettbu deleted the pr35917 branch February 26, 2026 21:24
@everettbu
everettbu restored the pr35917 branch February 26, 2026 22:15
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