@samitouri / QOS-React-1 / commits / de97ef9ad5

[Compiler] Don't count a setState in the dependency array of the effect it is called on as a usage (#35134)

Summary: The validation only allows setState declaration as a usage outside of the effect. Another edge case is that if you add the setState being validated in the dependency array you also make the validation opt out since it counts as a usage outside of the effect. Added a bit of logic to consider the effect's deps when creating the cache for setState usages within the effect Test Plan: Added a fixture --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35134). * #35135 * __->__ #35134

Jorge Cabiedes committed Nov 13, 2025 at 22:52 UTC de97ef9ad510352ba37434085098bf026dd46b9b
3 files changed +108 -7
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts
+35 -7
@@ -22,6 +22,7 @@ import {
22 BasicBlock,
23 isUseRefType,
24 SourceLocation,
25 + ArrayExpression,
26 } from '../HIR';
27 import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors';
28 import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables';
@@ -36,11 +37,17 @@ type DerivationMetadata = {
37 isStateSource: boolean;
38 };
39
40 +type EffectMetadata = {
41 + effect: HIRFunction;
42 + dependencies: ArrayExpression;
43 +};
44 +
45 type ValidationContext = {
46 readonly functions: Map<IdentifierId, FunctionExpression>;
47 + readonly candidateDependencies: Map<IdentifierId, ArrayExpression>;
48 readonly errors: CompilerError;
49 readonly derivationCache: DerivationCache;
43 - readonly effects: Set<HIRFunction>;
50 + readonly effectsCache: Map<IdentifierId, EffectMetadata>;
51 readonly setStateLoads: Map<IdentifierId, IdentifierId | null>;
52 readonly setStateUsages: Map<IdentifierId, Set<SourceLocation>>;
53 };
@@ -175,18 +182,20 @@ export function validateNoDerivedComputationsInEffects_exp(
182 fn: HIRFunction,
183 ): Result<void, CompilerError> {
184 const functions: Map<IdentifierId, FunctionExpression> = new Map();
185 + const candidateDependencies: Map<IdentifierId, ArrayExpression> = new Map();
186 const derivationCache = new DerivationCache();
187 const errors = new CompilerError();
180 - const effects: Set<HIRFunction> = new Set();
188 + const effectsCache: Map<IdentifierId, EffectMetadata> = new Map();
189
190 const setStateLoads: Map<IdentifierId, IdentifierId> = new Map();
191 const setStateUsages: Map<IdentifierId, Set<SourceLocation>> = new Map();
192
193 const context: ValidationContext = {
194 functions,
195 + candidateDependencies,
196 errors,
197 derivationCache,
189 - effects,
198 + effectsCache,
199 setStateLoads,
200 setStateUsages,
201 };
@@ -229,8 +238,8 @@ export function validateNoDerivedComputationsInEffects_exp(
238 isFirstPass = false;
239 } while (context.derivationCache.snapshot());
240
232 - for (const effect of effects) {
233 - validateEffect(effect, context);
241 + for (const [, effect] of effectsCache) {
242 + validateEffect(effect.effect, effect.dependencies, context);
243 }
244
245 return errors.asResult();
@@ -354,8 +363,14 @@ function recordInstructionDerivations(
363 value.args[1].kind === 'Identifier'
364 ) {
365 const effectFunction = context.functions.get(value.args[0].identifier.id);
357 - if (effectFunction != null) {
358 - context.effects.add(effectFunction.loweredFunc.func);
366 + const deps = context.candidateDependencies.get(
367 + value.args[1].identifier.id,
368 + );
369 + if (effectFunction != null && deps != null) {
370 + context.effectsCache.set(value.args[0].identifier.id, {
371 + effect: effectFunction.loweredFunc.func,
372 + dependencies: deps,
373 + });
374 }
375 } else if (isUseStateType(lvalue.identifier) && value.args.length > 0) {
376 typeOfValue = 'fromState';
@@ -367,6 +382,8 @@ function recordInstructionDerivations(
382 );
383 return;
384 }
385 + } else if (value.kind === 'ArrayExpression') {
386 + context.candidateDependencies.set(lvalue.identifier.id, value);
387 }
388
389 for (const operand of eachInstructionOperand(instr)) {
@@ -596,6 +613,7 @@ function getFnLocalDeps(
613
614 function validateEffect(
615 effectFunction: HIRFunction,
616 + dependencies: ArrayExpression,
617 context: ValidationContext,
618 ): void {
619 const seenBlocks: Set<BlockId> = new Set();
@@ -612,6 +630,16 @@ function validateEffect(
630 Set<SourceLocation>
631 > = new Map();
632
633 + // Consider setStates in the effect's dependency array as being part of effectSetStateUsages
634 + for (const dep of dependencies.elements) {
635 + if (dep.kind === 'Identifier') {
636 + const root = getRootSetState(dep.identifier.id, context.setStateLoads);
637 + if (root !== null) {
638 + effectSetStateUsages.set(root, new Set([dep.loc]));
639 + }
640 + }
641 + }
642 +
643 let cleanUpFunctionDeps: Set<IdentifierId> | undefined;
644
645 const globals: Set<IdentifierId> = new Set();
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-used-in-dep-array-still-errors.expect.md new
+63
@@ -0,0 +1,63 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
6 +
7 +function Component({prop}) {
8 + const [s, setS] = useState(0);
9 + useEffect(() => {
10 + setS(prop);
11 + }, [prop, setS]);
12 +
13 + return <div>{prop}</div>;
14 +}
15 +
16 +```
17 +
18 +## Code
19 +
20 +```javascript
21 +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
22 +
23 +function Component(t0) {
24 + const $ = _c(5);
25 + const { prop } = t0;
26 + const [, setS] = useState(0);
27 + let t1;
28 + let t2;
29 + if ($[0] !== prop) {
30 + t1 = () => {
31 + setS(prop);
32 + };
33 + t2 = [prop, setS];
34 + $[0] = prop;
35 + $[1] = t1;
36 + $[2] = t2;
37 + } else {
38 + t1 = $[1];
39 + t2 = $[2];
40 + }
41 + useEffect(t1, t2);
42 + let t3;
43 + if ($[3] !== prop) {
44 + t3 = <div>{prop}</div>;
45 + $[3] = prop;
46 + $[4] = t3;
47 + } else {
48 + t3 = $[4];
49 + }
50 + return t3;
51 +}
52 +
53 +```
54 +
55 +## Logs
56 +
57 +```
58 +{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [prop]\n\nData Flow Tree:\n└── prop (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":6,"column":4,"index":150},"end":{"line":6,"column":8,"index":154},"filename":"effect-used-in-dep-array-still-errors.ts","identifierName":"setS"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
59 +{"kind":"CompileSuccess","fnLoc":{"start":{"line":3,"column":0,"index":64},"end":{"line":10,"column":1,"index":212},"filename":"effect-used-in-dep-array-still-errors.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
60 +```
61 +
62 +### Eval output
63 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/effect-used-in-dep-array-still-errors.js new
+10
@@ -0,0 +1,10 @@
1 +// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
2 +
3 +function Component({prop}) {
4 + const [s, setS] = useState(0);
5 + useEffect(() => {
6 + setS(prop);
7 + }, [prop, setS]);
8 +
9 + return <div>{prop}</div>;
10 +}