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

compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts

@@ -1067,15 +1067,7 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule {
         name: 'memo-dependencies',
         description:
           'Validates that useMemo() and useCallback() specify comprehensive dependencies without extraneous values. See [`useMemo()` docs](https://react.dev/reference/react/useMemo) for more information.',
-        /**
-         * TODO: the "MemoDependencies" rule largely reimplements the "exhaustive-deps" non-compiler rule,
-         * allowing the compiler to ensure it does not regress change behavior due to different dependencies.
-         * We previously relied on the source having ESLint suppressions for any exhaustive-deps violations,
-         * but it's more reliable to verify it within the compiler.
-         *
-         * Long-term we should de-duplicate these implementations.
-         */
-        preset: LintRulePreset.Off,
+        preset: LintRulePreset.RecommendedLatest,
       };
     }
     case ErrorCategory.IncompatibleLibrary: {

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

@@ -303,11 +303,9 @@ function runWithEnvironment(
   inferReactivePlaces(hir);
   log({kind: 'hir', name: 'InferReactivePlaces', value: hir});
 
-  if (env.enableValidations) {
-    if (env.config.validateExhaustiveMemoizationDependencies) {
-      // NOTE: this relies on reactivity inference running first
-      validateExhaustiveDependencies(hir).unwrap();
-    }
+  if (env.config.validateExhaustiveMemoizationDependencies) {
+    // NOTE: this relies on reactivity inference running first
+    validateExhaustiveDependencies(hir).unwrap();
   }
 
   rewriteInstructionKindsBasedOnReassignment(hir);

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts

@@ -43,37 +43,20 @@ import {retainWhere} from '../Utils/utils';
 const DEBUG = false;
 
 /**
- * Validates that existing manual memoization is exhaustive and does not
- * have extraneous dependencies. The goal of the validation is to ensure
- * that auto-memoization will not substantially change the behavior of
- * the program:
- * - If the manual dependencies were non-exhaustive (missing important deps)
- *   then auto-memoization will include those dependencies, and cause the
- *   value to update *more* frequently.
- * - If the manual dependencies had extraneous deps, then auto memoization
- *   will remove them and cause the value to update *less* frequently.
+ * Validates that existing manual memoization had exhaustive dependencies.
+ * Memoization with missing or extra reactive dependencies is invalid React
+ * and compilation can change behavior, causing a value to be computed more
+ * or less times.
  *
- * We consider a value V as missing if ALL of the following conditions are met:
- * - V is reactive
- * - There is no manual dependency path P such that whenever V would change,
- *   P would also change. If V is `x.y.z`, this means there must be some
- *   path P that is either `x.y.z`, `x.y`, or `x`. Note that we assume no
- *   interior mutability, such that a shorter path "covers" changes to longer
- *   more precise paths.
- *
- * We consider a value V extraneous if either of the folowing are true:
- * - V is a reactive local that is unreferenced
- * - V is a global that is unreferenced
- *
- * In other words, we allow extraneous non-reactive values since we know they cannot
- * impact how often the memoization would run.
- *
- * ## TODO: Invalid, Complex Deps
- *
- * Handle cases where the user deps were not simple identifiers + property chains.
- * We try to detect this in ValidateUseMemo but we miss some cases. The problem
- * is that invalid forms can be value blocks or function calls that don't get
- * removed by DCE, leaving a structure like:
+ * TODOs:
+ * - Handle cases of mixed optional and non-optional versions of the same path,
+ *   eg referecing both x.y.z and x.y?.z in the same memo block. we should collapse
+ *   this into a single canonical dep that we look for in the manual deps. see the
+ *   existing exhaustive deps rule for implementation.
+ * - Handle cases where the user deps were not simple identifiers + property chains.
+ *   We try to detect this in ValidateUseMemo but we miss some cases. The problem
+ *   is that invalid forms can be value blocks or function calls that don't get
+ *   removed by DCE, leaving a structure like:
  *
  * StartMemoize
  * t0 = <value to memoize>
@@ -226,9 +209,31 @@ export function validateExhaustiveDependencies(
         reason: 'Unexpected function dependency',
         loc: value.loc,
       });
-      const isRequiredDependency = reactive.has(
-        inferredDependency.identifier.id,
-      );
+      /**
+       * Dependencies technically only need to include reactive values. However,
+       * reactivity inference for general values is subtle since it involves all
+       * of our complex control and data flow analysis. To keep results more
+       * stable and predictable to developers, we intentionally stay closer to
+       * the rules of the classic exhaustive-deps rule. Values should be included
+       * as dependencies if either of the following is true:
+       * - They're reactive
+       * - They're non-reactive and not a known-stable value type.
+       *
+       * Thus `const ref: Ref = cond ? ref1 : ref2` has to be a dependency
+       * (assuming `cond` is reactive) since it's reactive despite being a ref.
+       *
+       * Similarly, `const x = [1,2,3]` has to be a dependency since even
+       * though it's non reactive, it's not a known stable type.
+       *
+       * TODO: consider reimplementing a simpler form of reactivity inference.
+       * Ideally we'd consider `const ref: Ref = cond ? ref1 : ref2` as a required
+       * dependency even if our data/control flow tells us that `cond` is non-reactive.
+       * It's simpler for developers to reason about based on a more structural/AST
+       * driven approach.
+       */
+      const isRequiredDependency =
+        reactive.has(inferredDependency.identifier.id) ||
+        !isStableType(inferredDependency.identifier);
       let hasMatchingManualDependency = false;
       for (const manualDependency of manualDependencies) {
         if (
@@ -260,13 +265,18 @@ export function validateExhaustiveDependencies(
       extra.push(dep);
     }
 
-    /**
-     * Per docblock, we only consider dependencies as extraneous if
-     * they are unused globals or reactive locals. Notably, this allows
-     * non-reactive locals.
+    /*
+     * For compatiblity with the existing exhaustive-deps rule, we allow
+     * known-stable values as dependencies even if the value is not reactive.
+     * This allows code that takes a dep on a non-reactive setState function
+     * to pass, for example.
      */
     retainWhere(extra, dep => {
-      return dep.root.kind === 'Global' || dep.root.value.reactive;
+      const isNonReactiveStableValue =
+        dep.root.kind === 'NamedLocal' &&
+        !dep.root.value.reactive &&
+        isStableType(dep.root.value.identifier);
+      return !isNonReactiveStableValue;
     });
 
     if (missing.length !== 0 || extra.length !== 0) {
@@ -284,7 +294,7 @@ export function validateExhaustiveDependencies(
       if (missing.length !== 0) {
         const diagnostic = CompilerDiagnostic.create({
           category: ErrorCategory.MemoDependencies,
-          reason: 'Found missing memoization dependencies',
+          reason: 'Found non-exhaustive dependencies',
           description:
             'Missing dependencies can cause a value not to update when those inputs change, ' +
             'resulting in stale UI',
@@ -309,7 +319,7 @@ export function validateExhaustiveDependencies(
           reason: 'Found unnecessary memoization dependencies',
           description:
             'Unnecessary dependencies can cause a value to update more often than necessary, ' +
-            'causing performance regressions and effects to fire more often than expected',
+            'which can cause effects to run more than expected',
         });
         diagnostic.withDetails({
           kind: 'error',

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md

@@ -53,7 +53,7 @@ function Component({x, y, z}) {
 ```
 Found 4 errors:
 
-Error: Found missing memoization dependencies
+Error: Found non-exhaustive dependencies
 
 Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
 
@@ -66,7 +66,7 @@ error.invalid-exhaustive-deps.ts:7:11
    9 |   }, [x?.y.z?.a.b]);
   10 |   const b = useMemo(() => {
 
-Error: Found missing memoization dependencies
+Error: Found non-exhaustive dependencies
 
 Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
 
@@ -81,7 +81,7 @@ error.invalid-exhaustive-deps.ts:15:11
 
 Error: Found unnecessary memoization dependencies
 
-Unnecessary dependencies can cause a value to update more often than necessary, causing performance regressions and effects to fire more often than expected.
+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:31:5
   29 |     return [];
@@ -92,7 +92,7 @@ error.invalid-exhaustive-deps.ts:31:5
   33 |   const ref2 = useRef(null);
   34 |   const ref = z ? ref1 : ref2;
 
-Error: Found missing memoization dependencies
+Error: Found non-exhaustive dependencies
 
 Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
 

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.expect.md

@@ -3,7 +3,7 @@
 
 ```javascript
 // @validateExhaustiveMemoizationDependencies
-import {useCallback, useMemo} from 'react';
+import {useMemo} from 'react';
 import {makeObject_Primitives, Stringify} from 'shared-runtime';
 
 function useHook1(x) {
@@ -47,16 +47,6 @@ function useHook6(x) {
   }, [x]);
 }
 
-function useHook7(x) {
-  const [state, setState] = useState(true);
-  const f = () => {
-    setState(x => !x);
-  };
-  return useCallback(() => {
-    f();
-  }, [f]);
-}
-
 function Component({x, y, z}) {
   const a = useHook1(x);
   const b = useHook2(x);
@@ -64,8 +54,7 @@ function Component({x, y, z}) {
   const d = useHook4(x, y, z);
   const e = useHook5(x);
   const f = useHook6(x);
-  const g = useHook7(x);
-  return <Stringify results={[a, b, c, d, e, f, g]} />;
+  return <Stringify results={[a, b, c, d, e, f]} />;
 }
 
 ```
@@ -74,7 +63,7 @@ function Component({x, y, z}) {
 
 ```javascript
 import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies
-import { useCallback, useMemo } from "react";
+import { useMemo } from "react";
 import { makeObject_Primitives, Stringify } from "shared-runtime";
 
 function useHook1(x) {
@@ -132,36 +121,8 @@ function useHook6(x) {
   return f;
 }
 
-function useHook7(x) {
-  const $ = _c(2);
-  const [, setState] = useState(true);
-  let t0;
-  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    t0 = () => {
-      setState(_temp);
-    };
-    $[0] = t0;
-  } else {
-    t0 = $[0];
-  }
-  const f = t0;
-  let t1;
-  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
-    t1 = () => {
-      f();
-    };
-    $[1] = t1;
-  } else {
-    t1 = $[1];
-  }
-  return t1;
-}
-function _temp(x_0) {
-  return !x_0;
-}
-
 function Component(t0) {
-  const $ = _c(8);
+  const $ = _c(7);
   const { x, y, z } = t0;
   const a = useHook1(x);
   const b = useHook2(x);
@@ -169,7 +130,6 @@ function Component(t0) {
   const d = useHook4(x, y, z);
   const e = useHook5(x);
   const f = useHook6(x);
-  const g = useHook7(x);
   let t1;
   if (
     $[0] !== a ||
@@ -177,20 +137,18 @@ function Component(t0) {
     $[2] !== c ||
     $[3] !== d ||
     $[4] !== e ||
-    $[5] !== f ||
-    $[6] !== g
+    $[5] !== f
   ) {
-    t1 = <Stringify results={[a, b, c, d, e, f, g]} />;
+    t1 = <Stringify results={[a, b, c, d, e, f]} />;
     $[0] = a;
     $[1] = b;
     $[2] = c;
     $[3] = d;
     $[4] = e;
     $[5] = f;
-    $[6] = g;
-    $[7] = t1;
+    $[6] = t1;
   } else {
-    t1 = $[7];
+    t1 = $[6];
   }
   return t1;
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.js

@@ -1,5 +1,5 @@
 // @validateExhaustiveMemoizationDependencies
-import {useCallback, useMemo} from 'react';
+import {useMemo} from 'react';
 import {makeObject_Primitives, Stringify} from 'shared-runtime';
 
 function useHook1(x) {
@@ -43,16 +43,6 @@ function useHook6(x) {
   }, [x]);
 }
 
-function useHook7(x) {
-  const [state, setState] = useState(true);
-  const f = () => {
-    setState(x => !x);
-  };
-  return useCallback(() => {
-    f();
-  }, [f]);
-}
-
 function Component({x, y, z}) {
   const a = useHook1(x);
   const b = useHook2(x);
@@ -60,6 +50,5 @@ function Component({x, y, z}) {
   const d = useHook4(x, y, z);
   const e = useHook5(x);
   const f = useHook6(x);
-  const g = useHook7(x);
-  return <Stringify results={[a, b, c, d, e, f, g]} />;
+  return <Stringify results={[a, b, c, d, e, f]} />;
 }