@samitouri / QOS-React / commits / d39a1d6b63

[compiler] Distingush optional/extraneous deps (#35204)

In ValidateExhaustiveDependencies, I previously changed to allow extraneous dependencies as long as they were non-reactive. Here we make that more precise, and distinguish between values that are definitely referenced in the memo function but optional as dependencies vs values that are not even referenced in the memo function. The latter now error as extraneous even if they're non-reactive. This also turned up a case where constant-folded primitives could show up as false positives of the latter category, so now we track manual deps which quality for constant folding and don't error on them. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35204). * #35213 * #35201 * __->__ #35204

Joseph Savona committed Nov 25, 2025 at 12:06 UTC d39a1d6b638de6b990ea783544df775b8be59f1a
15 files changed +169 -38
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
+1
@@ -803,6 +803,7 @@ export type ManualMemoDependency = {
803 | {
804 kind: 'NamedLocal';
805 value: Place;
806 + constant: boolean;
807 }
808 | {kind: 'Global'; identifierName: string};
809 path: DependencyPath;
compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts
+1
@@ -92,6 +92,7 @@ export function collectMaybeMemoDependencies(
92 root: {
93 kind: 'NamedLocal',
94 value: {...value.place},
95 + constant: false,
96 },
97 path: [],
98 };
compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts
+13
@@ -609,6 +609,19 @@ function evaluateInstruction(
609 constantPropagationImpl(value.loweredFunc.func, constants);
610 return null;
611 }
612 + case 'StartMemoize': {
613 + if (value.deps != null) {
614 + for (const dep of value.deps) {
615 + if (dep.root.kind === 'NamedLocal') {
616 + const placeValue = read(constants, dep.root.value);
617 + if (placeValue != null && placeValue.kind === 'Primitive') {
618 + dep.root.constant = true;
619 + }
620 + }
621 + }
622 + }
623 + return null;
624 + }
625 default: {
626 // TODO: handle more cases
627 return null;
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts
+35 -31
@@ -22,6 +22,7 @@ import {
22 Identifier,
23 IdentifierId,
24 InstructionKind,
25 + isPrimitiveType,
26 isStableType,
27 isSubPath,
28 isSubPathIgnoringOptionals,
@@ -53,20 +54,18 @@ const DEBUG = false;
54 * - If the manual dependencies had extraneous deps, then auto memoization
55 * will remove them and cause the value to update *less* frequently.
56 *
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.
57 + * The implementation compares the manual dependencies against the values
58 + * actually used within the memoization function
59 + * - For each value V referenced in the memo function, either:
60 + * - If the value is non-reactive *and* a known stable type, then the
61 + * value may optionally be specified as an exact dependency.
62 + * - Otherwise, report an error unless there is a manual dependency that will
63 + * invalidate whenever V invalidates. If `x.y.z` is referenced, there must
64 + * be a manual dependency for `x.y.z`, `x.y`, or `x`. Note that we assume
65 + * no interior mutability, ie we assume that any changes to inner paths must
66 + * always cause the other path to change as well.
67 + * - Any dependencies that do not correspond to a value referenced in the memo
68 + * function are considered extraneous and throw an error
69 *
70 * ## TODO: Invalid, Complex Deps
71 *
@@ -226,9 +225,6 @@ export function validateExhaustiveDependencies(
225 reason: 'Unexpected function dependency',
226 loc: value.loc,
227 });
229 - const isRequiredDependency = reactive.has(
230 - inferredDependency.identifier.id,
231 - );
228 let hasMatchingManualDependency = false;
229 for (const manualDependency of manualDependencies) {
230 if (
@@ -243,32 +239,40 @@ export function validateExhaustiveDependencies(
239 ) {
240 hasMatchingManualDependency = true;
241 matched.add(manualDependency);
246 - if (!isRequiredDependency) {
247 - extra.push(manualDependency);
248 - }
242 }
243 }
251 - if (isRequiredDependency && !hasMatchingManualDependency) {
252 - missing.push(inferredDependency);
244 + const isOptionalDependency =
245 + !reactive.has(inferredDependency.identifier.id) &&
246 + (isStableType(inferredDependency.identifier) ||
247 + isPrimitiveType(inferredDependency.identifier));
248 + if (hasMatchingManualDependency || isOptionalDependency) {
249 + continue;
250 }
251 + missing.push(inferredDependency);
252 }
253
254 for (const dep of startMemo.deps ?? []) {
255 if (matched.has(dep)) {
256 continue;
257 }
258 + if (dep.root.kind === 'NamedLocal' && dep.root.constant) {
259 + CompilerError.simpleInvariant(
260 + !dep.root.value.reactive &&
261 + isPrimitiveType(dep.root.value.identifier),
262 + {
263 + reason: 'Expected constant-folded dependency to be non-reactive',
264 + loc: dep.root.value.loc,
265 + },
266 + );
267 + /*
268 + * Constant primitives can get constant-folded, which means we won't
269 + * see a LoadLocal for the value within the memo function.
270 + */
271 + continue;
272 + }
273 extra.push(dep);
274 }
275
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 => {
269 - return dep.root.kind === 'Global' || dep.root.value.reactive;
270 - });
271 -
276 if (missing.length !== 0 || extra.length !== 0) {
277 let suggestions: Array<CompilerSuggestion> | null = null;
278 if (startMemo.depsLoc != null && typeof startMemo.depsLoc !== 'symbol') {
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts
+3
@@ -267,6 +267,7 @@ function validateInferredDep(
267 effect: Effect.Read,
268 reactive: false,
269 },
270 + constant: false,
271 },
272 path: [...dep.path],
273 };
@@ -379,6 +380,7 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
380 root: {
381 kind: 'NamedLocal',
382 value: storeTarget,
383 + constant: false,
384 },
385 path: [],
386 });
@@ -408,6 +410,7 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
410 root: {
411 kind: 'NamedLocal',
412 value: {...lvalue},
413 + constant: false,
414 },
415 path: [],
416 });
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.expect.md
+2 -1
@@ -11,7 +11,7 @@ function Component(props) {
11
12 Component = useMemo(() => {
13 return Component;
14 - });
14 + }, [Component]);
15
16 return <Component {...props} />;
17 }
@@ -36,6 +36,7 @@ function Component(props) {
36 if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
37 Component = Stringify;
38
39 + Component;
40 Component = Component;
41 $[0] = Component;
42 } else {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.js
+1 -1
@@ -7,7 +7,7 @@ function Component(props) {
7
8 Component = useMemo(() => {
9 return Component;
10 - });
10 + }, [Component]);
11
12 return <Component {...props} />;
13 }
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps-disallow-unused-stable-types.expect.md new
+42
@@ -0,0 +1,42 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @validateExhaustiveMemoizationDependencies
6 +
7 +import {useState} from 'react';
8 +import {Stringify} from 'shared-runtime';
9 +
10 +function Component() {
11 + const [state, setState] = useState(0);
12 + const x = useMemo(() => {
13 + return [state];
14 + // error: `setState` is a stable type, but not actually referenced
15 + }, [state, setState]);
16 +
17 + return 'oops';
18 +}
19 +
20 +```
21 +
22 +
23 +## Error
24 +
25 +```
26 +Found 1 error:
27 +
28 +Error: Found unnecessary memoization dependencies
29 +
30 +Unnecessary dependencies can cause a value to update more often than necessary, causing performance regressions and effects to fire more often than expected.
31 +
32 +error.invalid-exhaustive-deps-disallow-unused-stable-types.ts:11:5
33 + 9 | return [state];
34 + 10 | // error: `setState` is a stable type, but not actually referenced
35 +> 11 | }, [state, setState]);
36 + | ^^^^^^^^^^^^^^^^^ Unnecessary dependencies `setState`
37 + 12 |
38 + 13 | return 'oops';
39 + 14 | }
40 +```
41 +
42 +
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps-disallow-unused-stable-types.js new
+14
@@ -0,0 +1,14 @@
1 +// @validateExhaustiveMemoizationDependencies
2 +
3 +import {useState} from 'react';
4 +import {Stringify} from 'shared-runtime';
5 +
6 +function Component() {
7 + const [state, setState] = useState(0);
8 + const x = useMemo(() => {
9 + return [state];
10 + // error: `setState` is a stable type, but not actually referenced
11 + }, [state, setState]);
12 +
13 + return 'oops';
14 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps-allow-constant-folded-values.expect.md new
+41
@@ -0,0 +1,41 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @validateExhaustiveMemoizationDependencies
6 +
7 +function Component() {
8 + const x = 0;
9 + const y = useMemo(() => {
10 + return [x];
11 + // x gets constant-folded but shouldn't count as extraneous,
12 + // it was referenced in the memo block
13 + }, [x]);
14 + return y;
15 +}
16 +
17 +```
18 +
19 +## Code
20 +
21 +```javascript
22 +import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies
23 +
24 +function Component() {
25 + const $ = _c(1);
26 + const x = 0;
27 + let t0;
28 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
29 + t0 = [0];
30 + $[0] = t0;
31 + } else {
32 + t0 = $[0];
33 + }
34 + const y = t0;
35 + return y;
36 +}
37 +
38 +```
39 +
40 +### Eval output
41 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps-allow-constant-folded-values.js new
+11
@@ -0,0 +1,11 @@
1 +// @validateExhaustiveMemoizationDependencies
2 +
3 +function Component() {
4 + const x = 0;
5 + const y = useMemo(() => {
6 + return [x];
7 + // x gets constant-folded but shouldn't count as extraneous,
8 + // it was referenced in the memo block
9 + }, [x]);
10 + return y;
11 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/todo-ensure-constant-prop-decls-get-removed.expect.md
+2 -2
@@ -2,7 +2,7 @@
2 ## Input
3
4 ```javascript
5 -// @validatePreserveExistingMemoizationGuarantees
5 +// @validatePreserveExistingMemoizationGuarantees @validateExhaustiveMemoizationDependencies:false
6
7 import {useMemo} from 'react';
8
@@ -27,7 +27,7 @@ export const FIXTURE_ENTRYPOINT = {
27 ## Code
28
29 ```javascript
30 -import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees
30 +import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees @validateExhaustiveMemoizationDependencies:false
31
32 import { useMemo } from "react";
33
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/todo-ensure-constant-prop-decls-get-removed.ts
+1 -1
@@ -1,4 +1,4 @@
1 -// @validatePreserveExistingMemoizationGuarantees
1 +// @validatePreserveExistingMemoizationGuarantees @validateExhaustiveMemoizationDependencies:false
2
3 import {useMemo} from 'react';
4
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-maybe-modify-free-variable-preserve-memoization-guarantee.expect.md
+1 -1
@@ -15,7 +15,7 @@ function Component(props) {
15 const x = makeObject_Primitives();
16 x.value = props.value;
17 mutate(x, free, part);
18 - }, [props.value]);
18 + }, [props.value, free, part]);
19 mutate(free, part);
20 return callback;
21 }
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-maybe-modify-free-variable-preserve-memoization-guarantee.js
+1 -1
@@ -11,7 +11,7 @@ function Component(props) {
11 const x = makeObject_Primitives();
12 x.value = props.value;
13 mutate(x, free, part);
14 - }, [props.value]);
14 + }, [props.value, free, part]);
15 mutate(free, part);
16 return callback;
17 }