[pull] main from facebook:main

compiler/.claude/settings.local.json

@@ -0,0 +1,9 @@
+{
+  "permissions": {
+    "allow": [
+      "Bash(node scripts/enable-feature-flag.js:*)"
+    ],
+    "deny": [],
+    "ask": []
+  }
+}

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

@@ -1067,7 +1067,15 @@ 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.',
-        preset: LintRulePreset.RecommendedLatest,
+        /**
+         * 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,
       };
     }
     case ErrorCategory.IncompatibleLibrary: {

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

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

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

@@ -400,7 +400,15 @@ export function compileProgram(
    */
   const suppressions = findProgramSuppressions(
     pass.comments,
-    pass.opts.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS,
+    /*
+     * If the compiler is validating hooks rules and exhaustive memo dependencies, we don't need to check
+     * for React ESLint suppressions
+     */
+    pass.opts.environment.validateExhaustiveMemoizationDependencies &&
+      pass.opts.environment.validateHooksUsage
+      ? null
+      : (pass.opts.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS),
+    // Always bail on Flow suppressions
     pass.opts.flowSuppressions,
   );
 

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

@@ -78,7 +78,7 @@ export function filterSuppressionsThatAffectFunction(
 
 export function findProgramSuppressions(
   programComments: Array<t.Comment>,
-  ruleNames: Array<string>,
+  ruleNames: Array<string> | null,
   flowSuppressions: boolean,
 ): Array<SuppressionRange> {
   const suppressionRanges: Array<SuppressionRange> = [];
@@ -89,7 +89,7 @@ export function findProgramSuppressions(
   let disableNextLinePattern: RegExp | null = null;
   let disablePattern: RegExp | null = null;
   let enablePattern: RegExp | null = null;
-  if (ruleNames.length !== 0) {
+  if (ruleNames != null && ruleNames.length !== 0) {
     const rulePattern = `(${ruleNames.join('|')})`;
     disableNextLinePattern = new RegExp(
       `eslint-disable-next-line ${rulePattern}`,

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

@@ -24,6 +24,7 @@ import {
   InstructionKind,
   isStableType,
   isSubPath,
+  isSubPathIgnoringOptionals,
   isUseRefType,
   LoadGlobal,
   ManualMemoDependency,
@@ -42,20 +43,37 @@ import {retainWhere} from '../Utils/utils';
 const DEBUG = false;
 
 /**
- * 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.
+ * 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.
  *
- * 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:
+ * 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:
  *
  * StartMemoize
  * t0 = <value to memoize>
@@ -208,39 +226,20 @@ export function validateExhaustiveDependencies(
         reason: 'Unexpected function dependency',
         loc: value.loc,
       });
-      /**
-       * 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);
+      const isRequiredDependency = reactive.has(
+        inferredDependency.identifier.id,
+      );
       let hasMatchingManualDependency = false;
       for (const manualDependency of manualDependencies) {
         if (
           manualDependency.root.kind === 'NamedLocal' &&
           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);
@@ -261,6 +260,15 @@ 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.
+     */
+    retainWhere(extra, dep => {
+      return dep.root.kind === 'Global' || dep.root.value.reactive;
+    });
+
     if (missing.length !== 0 || extra.length !== 0) {
       let suggestions: Array<CompilerSuggestion> | null = null;
       if (startMemo.depsLoc != null && typeof startMemo.depsLoc !== 'symbol') {
@@ -276,7 +284,7 @@ export function validateExhaustiveDependencies(
       if (missing.length !== 0) {
         const diagnostic = CompilerDiagnostic.create({
           category: ErrorCategory.MemoDependencies,
-          reason: 'Found non-exhaustive dependencies',
+          reason: 'Found missing memoization dependencies',
           description:
             'Missing dependencies can cause a value not to update when those inputs change, ' +
             'resulting in stale UI',
@@ -301,7 +309,7 @@ export function validateExhaustiveDependencies(
           reason: 'Found unnecessary memoization dependencies',
           description:
             'Unnecessary dependencies can cause a value to update more often than necessary, ' +
-            'which can cause effects to run more than expected',
+            'causing performance regressions and effects to fire more often than expected',
         });
         diagnostic.withDetails({
           kind: 'error',

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/compile-files-with-exhaustive-deps-violation-in-effects.expect.md

@@ -0,0 +1,91 @@
+
+## Input
+
+```javascript
+// @validateExhaustiveMemoizationDependencies
+
+import {useMemo} from 'react';
+import {ValidateMemoization} from 'shared-runtime';
+
+function Component({x}) {
+  useEffect(
+    () => {
+      console.log(x);
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    },
+    [
+      /* intentionally missing deps */
+    ]
+  );
+
+  const memo = useMemo(() => {
+    return [x];
+  }, [x]);
+
+  return <ValidateMemoization inputs={[x]} output={memo} />;
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies
+
+import { useMemo } from "react";
+import { ValidateMemoization } from "shared-runtime";
+
+function Component(t0) {
+  const $ = _c(10);
+  const { x } = t0;
+  let t1;
+  if ($[0] !== x) {
+    t1 = () => {
+      console.log(x);
+    };
+    $[0] = x;
+    $[1] = t1;
+  } else {
+    t1 = $[1];
+  }
+  let t2;
+  if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
+    t2 = [];
+    $[2] = t2;
+  } else {
+    t2 = $[2];
+  }
+  useEffect(t1, t2);
+  let t3;
+  if ($[3] !== x) {
+    t3 = [x];
+    $[3] = x;
+    $[4] = t3;
+  } else {
+    t3 = $[4];
+  }
+  const memo = t3;
+  let t4;
+  if ($[5] !== x) {
+    t4 = [x];
+    $[5] = x;
+    $[6] = t4;
+  } else {
+    t4 = $[6];
+  }
+  let t5;
+  if ($[7] !== memo || $[8] !== t4) {
+    t5 = <ValidateMemoization inputs={t4} output={memo} />;
+    $[7] = memo;
+    $[8] = t4;
+    $[9] = t5;
+  } else {
+    t5 = $[9];
+  }
+  return t5;
+}
+
+```
+
+### Eval output
+(kind: exception) Fixture not implemented

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/compile-files-with-exhaustive-deps-violation-in-effects.js

@@ -0,0 +1,22 @@
+// @validateExhaustiveMemoizationDependencies
+
+import {useMemo} from 'react';
+import {ValidateMemoization} from 'shared-runtime';
+
+function Component({x}) {
+  useEffect(
+    () => {
+      console.log(x);
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    },
+    [
+      /* intentionally missing deps */
+    ]
+  );
+
+  const memo = useMemo(() => {
+    return [x];
+  }, [x]);
+
+  return <ValidateMemoization inputs={[x]} output={memo} />;
+}