@samitouri / QOS-React-1 / commits / 40b4a5bf71

[compiler] ValidateExhaustiveDeps disallows unnecessary non-reactive deps (#34472)

Just to be consistent, we disallow unnecessary deps even if they're known to be non-reactive.

Joseph Savona committed Nov 20, 2025 at 19:30 UTC 40b4a5bf71ba7864556a5589b270b237f453c032
4 files changed +139 -51
compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts
+16 -1
@@ -600,7 +600,8 @@ function printErrorSummary(category: ErrorCategory, message: string): string {
600 case ErrorCategory.Suppression:
601 case ErrorCategory.Syntax:
602 case ErrorCategory.UseMemo:
603 - case ErrorCategory.VoidUseMemo: {
603 + case ErrorCategory.VoidUseMemo:
604 + case ErrorCategory.MemoDependencies: {
605 heading = 'Error';
606 break;
607 }
@@ -658,6 +659,10 @@ export enum ErrorCategory {
659 * Checks that manual memoization is preserved
660 */
661 PreserveManualMemo = 'PreserveManualMemo',
662 + /**
663 + * Checks for exhaustive useMemo/useCallback dependencies without extraneous values
664 + */
665 + MemoDependencies = 'MemoDependencies',
666 /**
667 * Checks for known incompatible libraries
668 */
@@ -1055,6 +1060,16 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule {
1060 preset: LintRulePreset.RecommendedLatest,
1061 };
1062 }
1063 + case ErrorCategory.MemoDependencies: {
1064 + return {
1065 + category,
1066 + severity: ErrorSeverity.Error,
1067 + name: 'memo-dependencies',
1068 + description:
1069 + 'Validates that useMemo() and useCallback() specify comprehensive dependencies without extraneous values. See [`useMemo()` docs](https://react.dev/reference/react/useMemo) for more information.',
1070 + preset: LintRulePreset.RecommendedLatest,
1071 + };
1072 + }
1073 case ErrorCategory.IncompatibleLibrary: {
1074 return {
1075 category,
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts
+71 -23
@@ -22,7 +22,9 @@ import {
22 Identifier,
23 IdentifierId,
24 InstructionKind,
25 + isStableType,
26 isSubPath,
27 + isUseRefType,
28 LoadGlobal,
29 ManualMemoDependency,
30 Place,
@@ -46,9 +48,10 @@ const DEBUG = false;
48 * or less times.
49 *
50 * TODOs:
49 - * - Better handling of cases where we infer multiple dependencies related to a single
50 - * variable. Eg if the user has dep `x` and we inferred `x.y, x.z`, the user's dep
51 - * is sufficient.
51 + * - Handle cases of mixed optional and non-optional versions of the same path,
52 + * eg referecing both x.y.z and x.y?.z in the same memo block. we should collapse
53 + * this into a single canonical dep that we look for in the manual deps. see the
54 + * existing exhaustive deps rule for implementation.
55 * - Handle cases where the user deps were not simple identifiers + property chains.
56 * We try to detect this in ValidateUseMemo but we miss some cases. The problem
57 * is that invalid forms can be value blocks or function calls that don't get
@@ -108,7 +111,7 @@ export function validateExhaustiveDependencies(
111 );
112 visitCandidateDependency(value.decl, temporaries, dependencies, locals);
113 const inferred: Array<InferredDependency> = Array.from(dependencies);
111 - // Sort dependencies by name, and path, with shorter/non-optional paths first
114 + // Sort dependencies by name and path, with shorter/non-optional paths first
115 inferred.sort((a, b) => {
116 if (a.kind === 'Global' && b.kind == 'Global') {
117 return a.binding.name.localeCompare(b.binding.name);
@@ -205,6 +208,31 @@ export function validateExhaustiveDependencies(
208 reason: 'Unexpected function dependency',
209 loc: value.loc,
210 });
211 + /**
212 + * Dependencies technically only need to include reactive values. However,
213 + * reactivity inference for general values is subtle since it involves all
214 + * of our complex control and data flow analysis. To keep results more
215 + * stable and predictable to developers, we intentionally stay closer to
216 + * the rules of the classic exhaustive-deps rule. Values should be included
217 + * as dependencies if either of the following is true:
218 + * - They're reactive
219 + * - They're non-reactive and not a known-stable value type.
220 + *
221 + * Thus `const ref: Ref = cond ? ref1 : ref2` has to be a dependency
222 + * (assuming `cond` is reactive) since it's reactive despite being a ref.
223 + *
224 + * Similarly, `const x = [1,2,3]` has to be a dependency since even
225 + * though it's non reactive, it's not a known stable type.
226 + *
227 + * TODO: consider reimplementing a simpler form of reactivity inference.
228 + * Ideally we'd consider `const ref: Ref = cond ? ref1 : ref2` as a required
229 + * dependency even if our data/control flow tells us that `cond` is non-reactive.
230 + * It's simpler for developers to reason about based on a more structural/AST
231 + * driven approach.
232 + */
233 + const isRequiredDependency =
234 + reactive.has(inferredDependency.identifier.id) ||
235 + !isStableType(inferredDependency.identifier);
236 let hasMatchingManualDependency = false;
237 for (const manualDependency of manualDependencies) {
238 if (
@@ -216,19 +244,18 @@ export function validateExhaustiveDependencies(
244 ) {
245 hasMatchingManualDependency = true;
246 matched.add(manualDependency);
247 + if (!isRequiredDependency) {
248 + extra.push(manualDependency);
249 + }
250 }
251 }
221 - if (!hasMatchingManualDependency) {
252 + if (isRequiredDependency && !hasMatchingManualDependency) {
253 missing.push(inferredDependency);
254 }
255 }
256
257 for (const dep of startMemo.deps ?? []) {
227 - if (
228 - matched.has(dep) ||
229 - (dep.root.kind === 'NamedLocal' &&
230 - !reactive.has(dep.root.value.identifier.id))
231 - ) {
258 + if (matched.has(dep)) {
259 continue;
260 }
261 extra.push(dep);
@@ -247,36 +274,39 @@ export function validateExhaustiveDependencies(
274 ];
275 }
276 if (missing.length !== 0) {
250 - // Error
277 const diagnostic = CompilerDiagnostic.create({
252 - category: ErrorCategory.PreserveManualMemo,
278 + category: ErrorCategory.MemoDependencies,
279 reason: 'Found non-exhaustive dependencies',
280 description:
281 'Missing dependencies can cause a value not to update when those inputs change, ' +
256 - 'resulting in stale UI. This memoization cannot be safely rewritten by the compiler.',
282 + 'resulting in stale UI',
283 suggestions,
284 });
285 for (const dep of missing) {
286 + let reactiveStableValueHint = '';
287 + if (isStableType(dep.identifier)) {
288 + reactiveStableValueHint =
289 + '. Refs, setState functions, and other "stable" values generally do not need to be added as dependencies, but this variable may change over time to point to different values';
290 + }
291 diagnostic.withDetails({
292 kind: 'error',
262 - message: `Missing dependency \`${printInferredDependency(dep)}\``,
293 + message: `Missing dependency \`${printInferredDependency(dep)}\`${reactiveStableValueHint}`,
294 loc: dep.loc,
295 });
296 }
297 error.pushDiagnostic(diagnostic);
298 } else if (extra.length !== 0) {
299 const diagnostic = CompilerDiagnostic.create({
269 - category: ErrorCategory.PreserveManualMemo,
300 + category: ErrorCategory.MemoDependencies,
301 reason: 'Found unnecessary memoization dependencies',
302 description:
303 'Unnecessary dependencies can cause a value to update more often than necessary, ' +
273 - 'which can cause effects to run more than expected. This memoization cannot be safely ' +
274 - 'rewritten by the compiler',
304 + 'which can cause effects to run more than expected',
305 });
306 diagnostic.withDetails({
307 kind: 'error',
308 message: `Unnecessary dependencies ${extra.map(dep => `\`${printManualMemoDependency(dep)}\``).join(', ')}`,
279 - loc: value.loc,
309 + loc: startMemo.depsLoc ?? value.loc,
310 });
311 error.pushDiagnostic(diagnostic);
312 }
@@ -287,10 +317,15 @@ export function validateExhaustiveDependencies(
317 startMemo = null;
318 }
319
290 - collectDependencies(fn, temporaries, {
291 - onStartMemoize,
292 - onFinishMemoize,
293 - });
320 + collectDependencies(
321 + fn,
322 + temporaries,
323 + {
324 + onStartMemoize,
325 + onFinishMemoize,
326 + },
327 + false, // isFunctionExpression
328 + );
329 return error.asResult();
330 }
331
@@ -383,12 +418,20 @@ function collectDependencies(
418 locals: Set<IdentifierId>,
419 ) => void;
420 } | null,
421 + isFunctionExpression: boolean,
422 ): Extract<Temporary, {kind: 'Function'}> {
423 const optionals = findOptionalPlaces(fn);
424 if (DEBUG) {
425 console.log(prettyFormat(optionals));
426 }
427 const locals: Set<IdentifierId> = new Set();
428 + if (isFunctionExpression) {
429 + for (const param of fn.params) {
430 + const place = param.kind === 'Identifier' ? param : param.place;
431 + locals.add(place.identifier.id);
432 + }
433 + }
434 +
435 const dependencies: Set<InferredDependency> = new Set();
436 function visit(place: Place): void {
437 visitCandidateDependency(place, temporaries, dependencies, locals);
@@ -523,7 +566,11 @@ function collectDependencies(
566 break;
567 }
568 case 'PropertyLoad': {
526 - if (typeof value.property === 'number') {
569 + if (
570 + typeof value.property === 'number' ||
571 + (isUseRefType(value.object.identifier) &&
572 + value.property === 'current')
573 + ) {
574 visit(value.object);
575 break;
576 }
@@ -553,6 +600,7 @@ function collectDependencies(
600 value.loweredFunc.func,
601 temporaries,
602 null,
603 + true, // isFunctionExpression
604 );
605 temporaries.set(lvalue.identifier.id, functionDeps);
606 addDependency(functionDeps, dependencies, locals);
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md
+42 -25
@@ -26,8 +26,16 @@ function Component({x, y, z}) {
26 }, [x]);
27 const f = useMemo(() => {
28 return [];
29 - }, [x, y.z, z?.y?.a]);
30 - return <Stringify results={[a, b, c, d, e, f]} />;
29 + }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
30 + const ref1 = useRef(null);
31 + const ref2 = useRef(null);
32 + const ref = z ? ref1 : ref2;
33 + const cb = useMemo(() => {
34 + return () => {
35 + return ref.current;
36 + };
37 + }, []);
38 + return <Stringify results={[a, b, c, d, e, f, cb]} />;
39 }
40
41 ```
@@ -36,11 +44,11 @@ function Component({x, y, z}) {
44 ## Error
45
46 ```
39 -Found 4 errors:
47 +Found 5 errors:
48
41 -Compilation Skipped: Found non-exhaustive dependencies
49 +Error: Found non-exhaustive dependencies
50
43 -Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. This memoization cannot be safely rewritten by the compiler..
51 +Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
52
53 error.invalid-exhaustive-deps.ts:7:11
54 5 | function Component({x, y, z}) {
@@ -51,9 +59,9 @@ error.invalid-exhaustive-deps.ts:7:11
59 9 | const b = useMemo(() => {
60 10 | return x.y.z?.a;
61
54 -Compilation Skipped: Found non-exhaustive dependencies
62 +Error: Found non-exhaustive dependencies
63
56 -Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. This memoization cannot be safely rewritten by the compiler..
64 +Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
65
66 error.invalid-exhaustive-deps.ts:10:11
67 8 | }, [x?.y.z?.a.b]);
@@ -64,9 +72,9 @@ error.invalid-exhaustive-deps.ts:10:11
72 12 | const c = useMemo(() => {
73 13 | return x?.y.z.a?.b;
74
67 -Compilation Skipped: Found non-exhaustive dependencies
75 +Error: Found non-exhaustive dependencies
76
69 -Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. This memoization cannot be safely rewritten by the compiler..
77 +Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
78
79 error.invalid-exhaustive-deps.ts:13:11
80 11 | }, [x.y.z.a]);
@@ -77,22 +85,31 @@ error.invalid-exhaustive-deps.ts:13:11
85 15 | const d = useMemo(() => {
86 16 | return x?.y?.[(console.log(y), z?.b)];
87
80 -Compilation Skipped: Found unnecessary memoization dependencies
81 -
82 -Unnecessary dependencies can cause a value to update more often than necessary, which can cause effects to run more than expected. This memoization cannot be safely rewritten by the compiler.
83 -
84 -error.invalid-exhaustive-deps.ts:23:20
85 - 21 | return e;
86 - 22 | }, [x]);
87 -> 23 | const f = useMemo(() => {
88 - | ^^^^^^^
89 -> 24 | return [];
90 - | ^^^^^^^^^^^^^^
91 -> 25 | }, [x, y.z, z?.y?.a]);
92 - | ^^^^ Unnecessary dependencies `x`, `y.z`, `z?.y?.a`
93 - 26 | return <Stringify results={[a, b, c, d, e, f]} />;
94 - 27 | }
95 - 28 |
88 +Error: Found unnecessary memoization dependencies
89 +
90 +Unnecessary dependencies can cause a value to update more often than necessary, which can cause effects to run more than expected.
91 +
92 +error.invalid-exhaustive-deps.ts:25:5
93 + 23 | const f = useMemo(() => {
94 + 24 | return [];
95 +> 25 | }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
96 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary dependencies `x`, `y.z`, `z?.y?.a`, `UNUSED_GLOBAL`
97 + 26 | const ref1 = useRef(null);
98 + 27 | const ref2 = useRef(null);
99 + 28 | const ref = z ? ref1 : ref2;
100 +
101 +Error: Found non-exhaustive dependencies
102 +
103 +Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
104 +
105 +error.invalid-exhaustive-deps.ts:31:13
106 + 29 | const cb = useMemo(() => {
107 + 30 | return () => {
108 +> 31 | return ref.current;
109 + | ^^^ Missing dependency `ref`. Refs, setState functions, and other "stable" values generally do not need to be added as dependencies, but this variable may change over time to point to different values
110 + 32 | };
111 + 33 | }, []);
112 + 34 | return <Stringify results={[a, b, c, d, e, f, cb]} />;
113 ```
114
115
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js
+10 -2
@@ -22,6 +22,14 @@ function Component({x, y, z}) {
22 }, [x]);
23 const f = useMemo(() => {
24 return [];
25 - }, [x, y.z, z?.y?.a]);
26 - return <Stringify results={[a, b, c, d, e, f]} />;
25 + }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
26 + const ref1 = useRef(null);
27 + const ref2 = useRef(null);
28 + const ref = z ? ref1 : ref2;
29 + const cb = useMemo(() => {
30 + return () => {
31 + return ref.current;
32 + };
33 + }, []);
34 + return <Stringify results={[a, b, c, d, e, f, cb]} />;
35 }