Skip to content

[Compiler] ValidateNoDerivedComputationsInEffects test cases#143

Closed
everettbu wants to merge 1 commit into
mainfrom
pr34574
Closed

[Compiler] ValidateNoDerivedComputationsInEffects test cases#143
everettbu wants to merge 1 commit into
mainfrom
pr34574

Conversation

@everettbu

@everettbu everettbu commented Dec 12, 2025

Copy link
Copy Markdown

Mirror of facebook/react#34574
Original author: jorge-cab


Summary:
This creates the test cases we expect this first iteration of calculate in render to catch

The goal is to have tests that will be in a good state once we have the first iteration of the calculate in render validation working, which should be pretty limited in what its capturing.

Test Plan:
Test cases


Stack created with Sapling. Best reviewed with ReviewStack.

  • #34580
  • #34579
  • #34578
  • #34577
  • #34575
  • -> #34574

Summary:
This creates the test cases we expect this first iteration of calculate in render to catch

The goal is to have tests that will be in a good state once we have the first iteration of the calculate in render validation working, which should be pretty limited in what its capturing.

Test Plan:
Test cases
@greptile-apps

greptile-apps Bot commented Dec 12, 2025

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

Adds experimental validation pass ValidateNoDerivedComputationsInEffects_exp that detects when useEffect is misused for derived state computations that should be calculated during render. The implementation includes comprehensive test coverage with 13 positive test cases (should trigger validation errors) and 3 negative test cases (should not trigger errors).

Key changes:

  • New validation module with dataflow analysis to track effect dependencies and detect pure derived computations
  • Integration into compiler pipeline via validateNoDerivedComputationsInEffects_exp config flag
  • Test cases covering various patterns: basic derived state, destructured props, computed props, conditional effects, local state derivation, and edge cases with function calls
  • Modified existing test to align with the simpler validation logic (removed capitalize call)

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • This is a well-structured test-only PR that mirrors changes from the upstream React repository. The new validation is experimental and gated behind a config flag, with comprehensive test coverage. The implementation follows established patterns in the codebase and includes proper error handling.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts 5/5 New validation pass that detects derived state computations in effects, with comprehensive dependency tracking and control flow analysis
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts 5/5 Added experimental validation flag integration into the compiler pipeline
compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts 5/5 Added configuration flag validateNoDerivedComputationsInEffects_exp for experimental validation
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/error.invalid-derived-computation-in-effect.js 5/5 Modified existing test to work with the experimental validation (removed function call, added imports)

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

29 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@everettbu everettbu added the Resolution: Stale Automatically closed due to inactivity label Jan 19, 2026
@everettbu

Copy link
Copy Markdown
Author

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

@everettbu everettbu closed this Jan 26, 2026
@everettbu
everettbu deleted the pr34574 branch January 26, 2026 23:16
@everettbu
everettbu restored the pr34574 branch January 27, 2026 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Resolution: Stale Automatically closed due to inactivity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants