@samitouri / QOS-React-1 / commits / 67c1487ffd

[compiler] Allow extraneous non-reactive locals (#35190)

The existing exhaustive-deps rule allows omitting non-reactive dependencies, even if they're not memoized. Conceptually, if a value is non-reactive then it cannot semantically change. Even if the value is a new object, that object represents the exact same value and doesn't necessitate redoing downstream computation. Thus its fine to exclude nonreactive dependencies, whether they're a stable type or not. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35190). * #35201 * #35202 * #35192 * __->__ #35190

Joseph Savona committed Nov 24, 2025 at 12:18 UTC 67c1487ffd872c95a3bb7d8104eac6eca79fe8cb
3 files changed +101 -58
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts
+38 -48
@@ -43,20 +43,37 @@ import {retainWhere} from '../Utils/utils';
43 const DEBUG = false;
44
45 /**
46 - * Validates that existing manual memoization had exhaustive dependencies.
47 - * Memoization with missing or extra reactive dependencies is invalid React
48 - * and compilation can change behavior, causing a value to be computed more
49 - * or less times.
46 + * Validates that existing manual memoization is exhaustive and does not
47 + * have extraneous dependencies. The goal of the validation is to ensure
48 + * that auto-memoization will not substantially change the behavior of
49 + * the program:
50 + * - If the manual dependencies were non-exhaustive (missing important deps)
51 + * then auto-memoization will include those dependencies, and cause the
52 + * value to update *more* frequently.
53 + * - If the manual dependencies had extraneous deps, then auto memoization
54 + * will remove them and cause the value to update *less* frequently.
55 *
51 - * TODOs:
52 - * - Handle cases of mixed optional and non-optional versions of the same path,
53 - * eg referecing both x.y.z and x.y?.z in the same memo block. we should collapse
54 - * this into a single canonical dep that we look for in the manual deps. see the
55 - * existing exhaustive deps rule for implementation.
56 - * - Handle cases where the user deps were not simple identifiers + property chains.
57 - * We try to detect this in ValidateUseMemo but we miss some cases. The problem
58 - * is that invalid forms can be value blocks or function calls that don't get
59 - * removed by DCE, leaving a structure like:
56 + * We consider a value V as missing if ALL of the following conditions are met:
57 + * - V is reactive
58 + * - There is no manual dependency path P such that whenever V would change,
59 + * P would also change. If V is `x.y.z`, this means there must be some
60 + * path P that is either `x.y.z`, `x.y`, or `x`. Note that we assume no
61 + * interior mutability, such that a shorter path "covers" changes to longer
62 + * more precise paths.
63 + *
64 + * We consider a value V extraneous if either of the folowing are true:
65 + * - V is a reactive local that is unreferenced
66 + * - V is a global that is unreferenced
67 + *
68 + * In other words, we allow extraneous non-reactive values since we know they cannot
69 + * impact how often the memoization would run.
70 + *
71 + * ## TODO: Invalid, Complex Deps
72 + *
73 + * Handle cases where the user deps were not simple identifiers + property chains.
74 + * We try to detect this in ValidateUseMemo but we miss some cases. The problem
75 + * is that invalid forms can be value blocks or function calls that don't get
76 + * removed by DCE, leaving a structure like:
77 *
78 * StartMemoize
79 * t0 = <value to memoize>
@@ -209,31 +226,9 @@ export function validateExhaustiveDependencies(
226 reason: 'Unexpected function dependency',
227 loc: value.loc,
228 });
212 - /**
213 - * Dependencies technically only need to include reactive values. However,
214 - * reactivity inference for general values is subtle since it involves all
215 - * of our complex control and data flow analysis. To keep results more
216 - * stable and predictable to developers, we intentionally stay closer to
217 - * the rules of the classic exhaustive-deps rule. Values should be included
218 - * as dependencies if either of the following is true:
219 - * - They're reactive
220 - * - They're non-reactive and not a known-stable value type.
221 - *
222 - * Thus `const ref: Ref = cond ? ref1 : ref2` has to be a dependency
223 - * (assuming `cond` is reactive) since it's reactive despite being a ref.
224 - *
225 - * Similarly, `const x = [1,2,3]` has to be a dependency since even
226 - * though it's non reactive, it's not a known stable type.
227 - *
228 - * TODO: consider reimplementing a simpler form of reactivity inference.
229 - * Ideally we'd consider `const ref: Ref = cond ? ref1 : ref2` as a required
230 - * dependency even if our data/control flow tells us that `cond` is non-reactive.
231 - * It's simpler for developers to reason about based on a more structural/AST
232 - * driven approach.
233 - */
234 - const isRequiredDependency =
235 - reactive.has(inferredDependency.identifier.id) ||
236 - !isStableType(inferredDependency.identifier);
229 + const isRequiredDependency = reactive.has(
230 + inferredDependency.identifier.id,
231 + );
232 let hasMatchingManualDependency = false;
233 for (const manualDependency of manualDependencies) {
234 if (
@@ -265,18 +260,13 @@ export function validateExhaustiveDependencies(
260 extra.push(dep);
261 }
262
268 - /*
269 - * For compatiblity with the existing exhaustive-deps rule, we allow
270 - * known-stable values as dependencies even if the value is not reactive.
271 - * This allows code that takes a dep on a non-reactive setState function
272 - * to pass, for example.
263 + /**
264 + * Per docblock, we only consider dependencies as extraneous if
265 + * they are unused globals or reactive locals. Notably, this allows
266 + * non-reactive locals.
267 */
268 retainWhere(extra, dep => {
275 - const isNonReactiveStableValue =
276 - dep.root.kind === 'NamedLocal' &&
277 - !dep.root.value.reactive &&
278 - isStableType(dep.root.value.identifier);
279 - return !isNonReactiveStableValue;
269 + return dep.root.kind === 'Global' || dep.root.value.reactive;
270 });
271
272 if (missing.length !== 0 || extra.length !== 0) {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.expect.md
+50 -8
@@ -3,7 +3,7 @@
3
4 ```javascript
5 // @validateExhaustiveMemoizationDependencies
6 -import {useMemo} from 'react';
6 +import {useCallback, useMemo} from 'react';
7 import {makeObject_Primitives, Stringify} from 'shared-runtime';
8
9 function useHook1(x) {
@@ -47,6 +47,16 @@ function useHook6(x) {
47 }, [x]);
48 }
49
50 +function useHook7(x) {
51 + const [state, setState] = useState(true);
52 + const f = () => {
53 + setState(x => !x);
54 + };
55 + return useCallback(() => {
56 + f();
57 + }, [f]);
58 +}
59 +
60 function Component({x, y, z}) {
61 const a = useHook1(x);
62 const b = useHook2(x);
@@ -54,7 +64,8 @@ function Component({x, y, z}) {
64 const d = useHook4(x, y, z);
65 const e = useHook5(x);
66 const f = useHook6(x);
57 - return <Stringify results={[a, b, c, d, e, f]} />;
67 + const g = useHook7(x);
68 + return <Stringify results={[a, b, c, d, e, f, g]} />;
69 }
70
71 ```
@@ -63,7 +74,7 @@ function Component({x, y, z}) {
74
75 ```javascript
76 import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies
66 -import { useMemo } from "react";
77 +import { useCallback, useMemo } from "react";
78 import { makeObject_Primitives, Stringify } from "shared-runtime";
79
80 function useHook1(x) {
@@ -121,8 +132,36 @@ function useHook6(x) {
132 return f;
133 }
134
135 +function useHook7(x) {
136 + const $ = _c(2);
137 + const [, setState] = useState(true);
138 + let t0;
139 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
140 + t0 = () => {
141 + setState(_temp);
142 + };
143 + $[0] = t0;
144 + } else {
145 + t0 = $[0];
146 + }
147 + const f = t0;
148 + let t1;
149 + if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
150 + t1 = () => {
151 + f();
152 + };
153 + $[1] = t1;
154 + } else {
155 + t1 = $[1];
156 + }
157 + return t1;
158 +}
159 +function _temp(x_0) {
160 + return !x_0;
161 +}
162 +
163 function Component(t0) {
125 - const $ = _c(7);
164 + const $ = _c(8);
165 const { x, y, z } = t0;
166 const a = useHook1(x);
167 const b = useHook2(x);
@@ -130,6 +169,7 @@ function Component(t0) {
169 const d = useHook4(x, y, z);
170 const e = useHook5(x);
171 const f = useHook6(x);
172 + const g = useHook7(x);
173 let t1;
174 if (
175 $[0] !== a ||
@@ -137,18 +177,20 @@ function Component(t0) {
177 $[2] !== c ||
178 $[3] !== d ||
179 $[4] !== e ||
140 - $[5] !== f
180 + $[5] !== f ||
181 + $[6] !== g
182 ) {
142 - t1 = <Stringify results={[a, b, c, d, e, f]} />;
183 + t1 = <Stringify results={[a, b, c, d, e, f, g]} />;
184 $[0] = a;
185 $[1] = b;
186 $[2] = c;
187 $[3] = d;
188 $[4] = e;
189 $[5] = f;
149 - $[6] = t1;
190 + $[6] = g;
191 + $[7] = t1;
192 } else {
151 - t1 = $[6];
193 + t1 = $[7];
194 }
195 return t1;
196 }
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.js
+13 -2
@@ -1,5 +1,5 @@
1 // @validateExhaustiveMemoizationDependencies
2 -import {useMemo} from 'react';
2 +import {useCallback, useMemo} from 'react';
3 import {makeObject_Primitives, Stringify} from 'shared-runtime';
4
5 function useHook1(x) {
@@ -43,6 +43,16 @@ function useHook6(x) {
43 }, [x]);
44 }
45
46 +function useHook7(x) {
47 + const [state, setState] = useState(true);
48 + const f = () => {
49 + setState(x => !x);
50 + };
51 + return useCallback(() => {
52 + f();
53 + }, [f]);
54 +}
55 +
56 function Component({x, y, z}) {
57 const a = useHook1(x);
58 const b = useHook2(x);
@@ -50,5 +60,6 @@ function Component({x, y, z}) {
60 const d = useHook4(x, y, z);
61 const e = useHook5(x);
62 const f = useHook6(x);
53 - return <Stringify results={[a, b, c, d, e, f]} />;
63 + const g = useHook7(x);
64 + return <Stringify results={[a, b, c, d, e, f, g]} />;
65 }