[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.

Author: josephsavona

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);

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 <Stringify results={[a, b, c, d, e, f, cb]} />;
 }
@@ -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 <Stringify results={[a, b, c, d, e, f, cb]} />;
+  38 |     };
+  39 |     // error: ref is a stable type but reactive
+  40 |   }, []);
 ```
           
       

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 <Stringify results={[a, b, c, d, e, f, cb]} />;
 }