From f5eef1b45547f6158c348c8aba45384e0c494703 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Mon, 24 Nov 2025 12:15:53 -0800 Subject: [PATCH] [compiler] Allow manual dependencies to have different optionality than inferred deps Since adding this validation we've already changed our inference to use knowledge from manual memoization to inform when values are frozen and which values are non-nullable. To align with that, if the user chooses to use different optionality btw the deps and the memo block/callback, that's fine. The key is that eg `x?.y` will invalidate whenever `x.y` would, so from a memoization correctness perspective its fine. It's not our job to be a type checker: if a value is potentially nullable, it should likely use a nullable property access in both places but TypeScript/Flow can check that. --- .../ValidateExhaustiveDependencies.ts | 6 +- .../error.invalid-exhaustive-deps.expect.md | 70 +++++++++---------- .../compiler/error.invalid-exhaustive-deps.js | 7 ++ 3 files changed, 44 insertions(+), 39 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts index df85ea8648c..01c82a375c9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -24,6 +24,7 @@ import { InstructionKind, isStableType, isSubPath, + isSubPathIgnoringOptionals, isUseRefType, LoadGlobal, ManualMemoDependency, @@ -240,7 +241,10 @@ export function validateExhaustiveDependencies( manualDependency.root.value.identifier.id === inferredDependency.identifier.id && (areEqualPaths(manualDependency.path, inferredDependency.path) || - isSubPath(manualDependency.path, inferredDependency.path)) + isSubPathIgnoringOptionals( + manualDependency.path, + inferredDependency.path, + )) ) { hasMatchingManualDependency = true; matched.add(manualDependency); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md index 5152f4121a2..bca95a84e1e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md @@ -9,23 +9,29 @@ import {Stringify} from 'shared-runtime'; function Component({x, y, z}) { const a = useMemo(() => { return x?.y.z?.a; + // error: too precise }, [x?.y.z?.a.b]); const b = useMemo(() => { return x.y.z?.a; + // ok, not our job to type check nullability }, [x.y.z.a]); const c = useMemo(() => { return x?.y.z.a?.b; + // error: too precise }, [x?.y.z.a?.b.z]); const d = useMemo(() => { return x?.y?.[(console.log(y), z?.b)]; + // ok }, [x?.y, y, z?.b]); const e = useMemo(() => { const e = []; e.push(x); return e; + // ok }, [x]); const f = useMemo(() => { return []; + // error: unnecessary }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]); const ref1 = useRef(null); const ref2 = useRef(null); @@ -34,6 +40,7 @@ function Component({x, y, z}) { return () => { return ref.current; }; + // error: ref is a stable type but reactive }, []); return ; } @@ -44,7 +51,7 @@ function Component({x, y, z}) { ## Error ``` -Found 5 errors: +Found 4 errors: Error: Found non-exhaustive dependencies @@ -55,61 +62,48 @@ error.invalid-exhaustive-deps.ts:7:11 6 | const a = useMemo(() => { > 7 | return x?.y.z?.a; | ^^^^^^^^^ Missing dependency `x?.y.z?.a` - 8 | }, [x?.y.z?.a.b]); - 9 | const b = useMemo(() => { - 10 | return x.y.z?.a; + 8 | // error: too precise + 9 | }, [x?.y.z?.a.b]); + 10 | const b = useMemo(() => { Error: Found non-exhaustive dependencies Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. -error.invalid-exhaustive-deps.ts:10:11 - 8 | }, [x?.y.z?.a.b]); - 9 | const b = useMemo(() => { -> 10 | return x.y.z?.a; - | ^^^^^^^^ Missing dependency `x.y.z?.a` - 11 | }, [x.y.z.a]); - 12 | const c = useMemo(() => { - 13 | return x?.y.z.a?.b; - -Error: Found non-exhaustive dependencies - -Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. - -error.invalid-exhaustive-deps.ts:13:11 - 11 | }, [x.y.z.a]); - 12 | const c = useMemo(() => { -> 13 | return x?.y.z.a?.b; +error.invalid-exhaustive-deps.ts:15:11 + 13 | }, [x.y.z.a]); + 14 | const c = useMemo(() => { +> 15 | return x?.y.z.a?.b; | ^^^^^^^^^^^ Missing dependency `x?.y.z.a?.b` - 14 | }, [x?.y.z.a?.b.z]); - 15 | const d = useMemo(() => { - 16 | return x?.y?.[(console.log(y), z?.b)]; + 16 | // error: too precise + 17 | }, [x?.y.z.a?.b.z]); + 18 | const d = useMemo(() => { Error: Found unnecessary memoization dependencies Unnecessary dependencies can cause a value to update more often than necessary, which can cause effects to run more than expected. -error.invalid-exhaustive-deps.ts:25:5 - 23 | const f = useMemo(() => { - 24 | return []; -> 25 | }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]); +error.invalid-exhaustive-deps.ts:31:5 + 29 | return []; + 30 | // error: unnecessary +> 31 | }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary dependencies `x`, `y.z`, `z?.y?.a`, `UNUSED_GLOBAL` - 26 | const ref1 = useRef(null); - 27 | const ref2 = useRef(null); - 28 | const ref = z ? ref1 : ref2; + 32 | const ref1 = useRef(null); + 33 | const ref2 = useRef(null); + 34 | const ref = z ? ref1 : ref2; Error: Found non-exhaustive dependencies Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. -error.invalid-exhaustive-deps.ts:31:13 - 29 | const cb = useMemo(() => { - 30 | return () => { -> 31 | return ref.current; +error.invalid-exhaustive-deps.ts:37:13 + 35 | const cb = useMemo(() => { + 36 | return () => { +> 37 | return ref.current; | ^^^ 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 - 32 | }; - 33 | }, []); - 34 | return ; + 38 | }; + 39 | // error: ref is a stable type but reactive + 40 | }, []); ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js index 53c189cc428..c0f8d28837a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js @@ -5,23 +5,29 @@ import {Stringify} from 'shared-runtime'; function Component({x, y, z}) { const a = useMemo(() => { return x?.y.z?.a; + // error: too precise }, [x?.y.z?.a.b]); const b = useMemo(() => { return x.y.z?.a; + // ok, not our job to type check nullability }, [x.y.z.a]); const c = useMemo(() => { return x?.y.z.a?.b; + // error: too precise }, [x?.y.z.a?.b.z]); const d = useMemo(() => { return x?.y?.[(console.log(y), z?.b)]; + // ok }, [x?.y, y, z?.b]); const e = useMemo(() => { const e = []; e.push(x); return e; + // ok }, [x]); const f = useMemo(() => { return []; + // error: unnecessary }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]); const ref1 = useRef(null); const ref2 = useRef(null); @@ -30,6 +36,7 @@ function Component({x, y, z}) { return () => { return ref.current; }; + // error: ref is a stable type but reactive }, []); return ; }