@samitouri / QOS-React / commits / b946a249b5

[compiler] Improve setState-in-effects rule to account for ref-gated conditionals (#35147)

Conditionally calling setState in an effect is sometimes necessary, but should generally follow the pattern of using a "previous vaue" ref to manually compare and ensure that the setState is idempotent. See fixture for an example. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35147). * #35148 * __->__ #35147

Joseph Savona committed Nov 17, 2025 at 12:07 UTC b946a249b560a2d3afe1a1c8553d5491b1767cb3
4 files changed +245 -6
compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts
+8 -3
@@ -672,9 +672,14 @@ export const EnvironmentConfigSchema = z.object({
672 validateNoDynamicallyCreatedComponentsOrHooks: z.boolean().default(false),
673
674 /**
675 - * When enabled, allows setState calls in effects when the value being set is
676 - * derived from a ref. This is useful for patterns where initial layout measurements
677 - * from refs need to be stored in state during mount.
675 + * When enabled, allows setState calls in effects based on valid patterns involving refs:
676 + * - Allow setState where the value being set is derived from a ref. This is useful where
677 + * state needs to take into account layer information, and a layout effect reads layout
678 + * data from a ref and sets state.
679 + * - Allow conditionally calling setState after manually comparing previous/new values
680 + * for changes via a ref. Relying on effect deps is insufficient for non-primitive values,
681 + * so a ref is generally required to manually track previous values and compare prev/next
682 + * for meaningful changes before setting state.
683 */
684 enableAllowSetStateFromRefsInEffects: z.boolean().default(true),
685
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInEffects.ts
+80 -3
@@ -21,13 +21,17 @@ import {
21 isUseRefType,
22 isRefValueType,
23 Place,
24 + Effect,
25 + BlockId,
26 } from '../HIR';
27 import {
28 eachInstructionLValue,
29 eachInstructionValueOperand,
30 } from '../HIR/visitors';
31 +import {createControlDominators} from '../Inference/ControlDominators';
32 +import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables';
33 import {Result} from '../Utils/Result';
30 -import {Iterable_some} from '../Utils/utils';
34 +import {assertExhaustive, Iterable_some} from '../Utils/utils';
35
36 /**
37 * Validates against calling setState in the body of an effect (useEffect and friends),
@@ -140,6 +144,8 @@ function getSetStateCall(
144 setStateFunctions: Map<IdentifierId, Place>,
145 env: Environment,
146 ): Place | null {
147 + const enableAllowSetStateFromRefsInEffects =
148 + env.config.enableAllowSetStateFromRefsInEffects;
149 const refDerivedValues: Set<IdentifierId> = new Set();
150
151 const isDerivedFromRef = (place: Place): boolean => {
@@ -150,9 +156,38 @@ function getSetStateCall(
156 );
157 };
158
159 + const isRefControlledBlock: (id: BlockId) => boolean =
160 + enableAllowSetStateFromRefsInEffects
161 + ? createControlDominators(fn, place => isDerivedFromRef(place))
162 + : (): boolean => false;
163 +
164 for (const [, block] of fn.body.blocks) {
165 + if (enableAllowSetStateFromRefsInEffects) {
166 + for (const phi of block.phis) {
167 + if (isDerivedFromRef(phi.place)) {
168 + continue;
169 + }
170 + let isPhiDerivedFromRef = false;
171 + for (const [, operand] of phi.operands) {
172 + if (isDerivedFromRef(operand)) {
173 + isPhiDerivedFromRef = true;
174 + break;
175 + }
176 + }
177 + if (isPhiDerivedFromRef) {
178 + refDerivedValues.add(phi.place.identifier.id);
179 + } else {
180 + for (const [pred] of phi.operands) {
181 + if (isRefControlledBlock(pred)) {
182 + refDerivedValues.add(phi.place.identifier.id);
183 + break;
184 + }
185 + }
186 + }
187 + }
188 + }
189 for (const instr of block.instructions) {
155 - if (env.config.enableAllowSetStateFromRefsInEffects) {
190 + if (enableAllowSetStateFromRefsInEffects) {
191 const hasRefOperand = Iterable_some(
192 eachInstructionValueOperand(instr.value),
193 isDerivedFromRef,
@@ -162,6 +197,46 @@ function getSetStateCall(
197 for (const lvalue of eachInstructionLValue(instr)) {
198 refDerivedValues.add(lvalue.identifier.id);
199 }
200 + // Ref-derived values can also propagate through mutation
201 + for (const operand of eachInstructionValueOperand(instr.value)) {
202 + switch (operand.effect) {
203 + case Effect.Capture:
204 + case Effect.Store:
205 + case Effect.ConditionallyMutate:
206 + case Effect.ConditionallyMutateIterator:
207 + case Effect.Mutate: {
208 + if (isMutable(instr, operand)) {
209 + refDerivedValues.add(operand.identifier.id);
210 + }
211 + break;
212 + }
213 + case Effect.Freeze:
214 + case Effect.Read: {
215 + // no-op
216 + break;
217 + }
218 + case Effect.Unknown: {
219 + CompilerError.invariant(false, {
220 + reason: 'Unexpected unknown effect',
221 + description: null,
222 + details: [
223 + {
224 + kind: 'error',
225 + loc: operand.loc,
226 + message: null,
227 + },
228 + ],
229 + suggestions: null,
230 + });
231 + }
232 + default: {
233 + assertExhaustive(
234 + operand.effect,
235 + `Unexpected effect kind \`${operand.effect}\``,
236 + );
237 + }
238 + }
239 + }
240 }
241
242 if (
@@ -203,7 +278,7 @@ function getSetStateCall(
278 isSetStateType(callee.identifier) ||
279 setStateFunctions.has(callee.identifier.id)
280 ) {
206 - if (env.config.enableAllowSetStateFromRefsInEffects) {
281 + if (enableAllowSetStateFromRefsInEffects) {
282 const arg = instr.value.args.at(0);
283 if (
284 arg !== undefined &&
@@ -216,6 +291,8 @@ function getSetStateCall(
291 * be needed when initial layout measurements from refs need to be stored in state.
292 */
293 return null;
294 + } else if (isRefControlledBlock(block.id)) {
295 + continue;
296 }
297 }
298 /*
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-setState-in-useEffect-controlled-by-ref-value.expect.md new
+117
@@ -0,0 +1,117 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @validateNoSetStateInEffects @enableAllowSetStateFromRefsInEffects @loggerTestOnly @compilationMode:"infer"
6 +import {useState, useRef, useEffect} from 'react';
7 +
8 +function Component({x, y}) {
9 + const previousXRef = useRef(null);
10 + const previousYRef = useRef(null);
11 +
12 + const [data, setData] = useState(null);
13 +
14 + useEffect(() => {
15 + const previousX = previousXRef.current;
16 + previousXRef.current = x;
17 + const previousY = previousYRef.current;
18 + previousYRef.current = y;
19 + if (!areEqual(x, previousX) || !areEqual(y, previousY)) {
20 + const data = load({x, y});
21 + setData(data);
22 + }
23 + }, [x, y]);
24 +
25 + return data;
26 +}
27 +
28 +function areEqual(a, b) {
29 + return a === b;
30 +}
31 +
32 +function load({x, y}) {
33 + return x * y;
34 +}
35 +
36 +export const FIXTURE_ENTRYPOINT = {
37 + fn: Component,
38 + params: [{x: 0, y: 0}],
39 + sequentialRenders: [
40 + {x: 0, y: 0},
41 + {x: 1, y: 0},
42 + {x: 1, y: 1},
43 + ],
44 +};
45 +
46 +```
47 +
48 +## Code
49 +
50 +```javascript
51 +import { c as _c } from "react/compiler-runtime"; // @validateNoSetStateInEffects @enableAllowSetStateFromRefsInEffects @loggerTestOnly @compilationMode:"infer"
52 +import { useState, useRef, useEffect } from "react";
53 +
54 +function Component(t0) {
55 + const $ = _c(4);
56 + const { x, y } = t0;
57 + const previousXRef = useRef(null);
58 + const previousYRef = useRef(null);
59 +
60 + const [data, setData] = useState(null);
61 + let t1;
62 + let t2;
63 + if ($[0] !== x || $[1] !== y) {
64 + t1 = () => {
65 + const previousX = previousXRef.current;
66 + previousXRef.current = x;
67 + const previousY = previousYRef.current;
68 + previousYRef.current = y;
69 + if (!areEqual(x, previousX) || !areEqual(y, previousY)) {
70 + const data_0 = load({ x, y });
71 + setData(data_0);
72 + }
73 + };
74 +
75 + t2 = [x, y];
76 + $[0] = x;
77 + $[1] = y;
78 + $[2] = t1;
79 + $[3] = t2;
80 + } else {
81 + t1 = $[2];
82 + t2 = $[3];
83 + }
84 + useEffect(t1, t2);
85 + return data;
86 +}
87 +
88 +function areEqual(a, b) {
89 + return a === b;
90 +}
91 +
92 +function load({ x, y }) {
93 + return x * y;
94 +}
95 +
96 +export const FIXTURE_ENTRYPOINT = {
97 + fn: Component,
98 + params: [{ x: 0, y: 0 }],
99 + sequentialRenders: [
100 + { x: 0, y: 0 },
101 + { x: 1, y: 0 },
102 + { x: 1, y: 1 },
103 + ],
104 +};
105 +
106 +```
107 +
108 +## Logs
109 +
110 +```
111 +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":163},"end":{"line":22,"column":1,"index":631},"filename":"valid-setState-in-useEffect-controlled-by-ref-value.ts"},"fnName":"Component","memoSlots":4,"memoBlocks":1,"memoValues":2,"prunedMemoBlocks":0,"prunedMemoValues":0}
112 +```
113 +
114 +### Eval output
115 +(kind: ok) 0
116 +0
117 +1
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-setState-in-useEffect-controlled-by-ref-value.js new
+40
@@ -0,0 +1,40 @@
1 +// @validateNoSetStateInEffects @enableAllowSetStateFromRefsInEffects @loggerTestOnly @compilationMode:"infer"
2 +import {useState, useRef, useEffect} from 'react';
3 +
4 +function Component({x, y}) {
5 + const previousXRef = useRef(null);
6 + const previousYRef = useRef(null);
7 +
8 + const [data, setData] = useState(null);
9 +
10 + useEffect(() => {
11 + const previousX = previousXRef.current;
12 + previousXRef.current = x;
13 + const previousY = previousYRef.current;
14 + previousYRef.current = y;
15 + if (!areEqual(x, previousX) || !areEqual(y, previousY)) {
16 + const data = load({x, y});
17 + setData(data);
18 + }
19 + }, [x, y]);
20 +
21 + return data;
22 +}
23 +
24 +function areEqual(a, b) {
25 + return a === b;
26 +}
27 +
28 +function load({x, y}) {
29 + return x * y;
30 +}
31 +
32 +export const FIXTURE_ENTRYPOINT = {
33 + fn: Component,
34 + params: [{x: 0, y: 0}],
35 + sequentialRenders: [
36 + {x: 0, y: 0},
37 + {x: 1, y: 0},
38 + {x: 1, y: 1},
39 + ],
40 +};