[compiler][poc] Reuse ValidateExhaustiveDeps for effect dep validation

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

@@ -601,7 +601,8 @@ function printErrorSummary(category: ErrorCategory, message: string): string {
     case ErrorCategory.Syntax:
     case ErrorCategory.UseMemo:
     case ErrorCategory.VoidUseMemo:
-    case ErrorCategory.MemoDependencies: {
+    case ErrorCategory.MemoDependencies:
+    case ErrorCategory.EffectExhaustiveDependencies: {
       heading = 'Error';
       break;
     }
@@ -683,6 +684,10 @@ export enum ErrorCategory {
    * Checks for memoized effect deps
    */
   EffectDependencies = 'EffectDependencies',
+  /**
+   * Checks for exhaustive and extraneous effect dependencies
+   */
+  EffectExhaustiveDependencies = 'EffectExhaustiveDependencies',
   /**
    * Checks for no setState in effect bodies
    */
@@ -838,6 +843,16 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule {
         preset: LintRulePreset.Off,
       };
     }
+    case ErrorCategory.EffectExhaustiveDependencies: {
+      return {
+        category,
+        severity: ErrorSeverity.Error,
+        name: 'exhaustive-effect-dependencies',
+        description:
+          'Validates that effect dependencies are exhaustive and without extraneous values',
+        preset: LintRulePreset.Off,
+      };
+    }
     case ErrorCategory.EffectDerivationsOfState: {
       return {
         category,

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

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

compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts

@@ -223,6 +223,11 @@ export const EnvironmentConfigSchema = z.object({
    */
   validateExhaustiveMemoizationDependencies: z.boolean().default(true),
 
+  /**
+   * Validate that dependencies supplied to effect hooks are exhaustive.
+   */
+  validateExhaustiveEffectDependencies: z.boolean().default(false),
+
   /**
    * When this is true, rather than pruning existing manual memoization but ensuring or validating
    * that the memoized values remain memoized, the compiler will simply not prune existing calls to

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

@@ -18,6 +18,7 @@ import {
   areEqualPaths,
   BlockId,
   DependencyPath,
+  Environment,
   FinishMemoize,
   GeneratedSource,
   HIRFunction,
@@ -87,6 +88,7 @@ const DEBUG = false;
  */
 export function validateExhaustiveDependencies(
   fn: HIRFunction,
+  env: Environment,
 ): Result<void, CompilerError> {
   const reactive = collectReactiveIdentifiersHIR(fn);
 
@@ -129,17 +131,20 @@ export function validateExhaustiveDependencies(
         loc: value.loc,
       },
     );
-    visitCandidateDependency(value.decl, temporaries, dependencies, locals);
-    const inferred: Array<InferredDependency> = Array.from(dependencies);
+    if (env.config.validateExhaustiveMemoizationDependencies) {
+      visitCandidateDependency(value.decl, temporaries, dependencies, locals);
+      const inferred: Array<InferredDependency> = Array.from(dependencies);
 
-    const diagnostic = validateDependencies(
-      inferred,
-      startMemo.deps ?? [],
-      reactive,
-      startMemo.depsLoc,
-    );
-    if (diagnostic != null) {
-      error.pushDiagnostic(diagnostic);
+      const diagnostic = validateDependencies(
+        inferred,
+        startMemo.deps ?? [],
+        reactive,
+        startMemo.depsLoc,
+        ErrorCategory.MemoDependencies,
+      );
+      if (diagnostic != null) {
+        error.pushDiagnostic(diagnostic);
+      }
     }
 
     dependencies.clear();
@@ -154,6 +159,9 @@ export function validateExhaustiveDependencies(
       onStartMemoize,
       onFinishMemoize,
       onEffect: (inferred, manual) => {
+        if (env.config.validateExhaustiveEffectDependencies === false) {
+          return;
+        }
         if (DEBUG) {
           console.log(Array.from(inferred, printInferredDependency));
           console.log(Array.from(manual, printInferredDependency));
@@ -192,6 +200,7 @@ export function validateExhaustiveDependencies(
           manualDeps,
           reactive,
           null,
+          ErrorCategory.EffectExhaustiveDependencies,
         );
         if (diagnostic != null) {
           error.pushDiagnostic(diagnostic);
@@ -208,6 +217,9 @@ function validateDependencies(
   manualDependencies: Array<ManualMemoDependency>,
   reactive: Set<IdentifierId>,
   manualMemoLoc: SourceLocation | null,
+  category:
+    | ErrorCategory.MemoDependencies
+    | ErrorCategory.EffectExhaustiveDependencies,
 ): CompilerDiagnostic | null {
   // Sort dependencies by name and path, with shorter/non-optional paths first
   inferred.sort((a, b) => {
@@ -366,23 +378,7 @@ function validateDependencies(
           .join(', ')}]`,
       };
     }
-    const diagnostic = CompilerDiagnostic.create({
-      category: ErrorCategory.MemoDependencies,
-      reason: 'Found missing/extra memoization dependencies',
-      description: [
-        missing.length !== 0
-          ? 'Missing dependencies can cause a value to update less often than it should, ' +
-            'resulting in stale UI'
-          : null,
-        extra.length !== 0
-          ? 'Extra dependencies can cause a value to update more often than it should, ' +
-            'resulting in performance problems such as excessive renders or effects firing too often'
-          : null,
-      ]
-        .filter(Boolean)
-        .join('. '),
-      suggestions: suggestion != null ? [suggestion] : null,
-    });
+    const diagnostic = createDiagnostic(category, missing, extra, suggestion);
     for (const dep of missing) {
       let reactiveStableValueHint = '';
       if (isStableType(dep.identifier)) {
@@ -994,3 +990,64 @@ function isOptionalDependency(
       isPrimitiveType(inferredDependency.identifier))
   );
 }
+
+function createDiagnostic(
+  category:
+    | ErrorCategory.MemoDependencies
+    | ErrorCategory.EffectExhaustiveDependencies,
+  missing: Array<InferredDependency>,
+  extra: Array<ManualMemoDependency>,
+  suggestion: CompilerSuggestion | null,
+): CompilerDiagnostic {
+  let reason: string;
+  let description: string;
+
+  function joinMissingExtraDetail(
+    missingString: string,
+    extraString: string,
+    joinStr: string,
+  ): string {
+    return [
+      missing.length !== 0 ? missingString : null,
+      extra.length !== 0 ? extraString : null,
+    ]
+      .filter(Boolean)
+      .join(joinStr);
+  }
+
+  switch (category) {
+    case ErrorCategory.MemoDependencies: {
+      reason = `Found ${joinMissingExtraDetail('missing', 'extra', '/')} memoization dependencies`;
+      description = joinMissingExtraDetail(
+        'Missing dependencies can cause a value to update less often than it should, resulting in stale UI',
+        'Extra dependencies can cause a value to update more often than it should, resulting in performance' +
+          ' problems such as excessive renders or effects firing too often',
+        '. ',
+      );
+      break;
+    }
+    case ErrorCategory.EffectExhaustiveDependencies: {
+      reason = `Found ${joinMissingExtraDetail('missing', 'extra', '/')} effect dependencies`;
+      description = joinMissingExtraDetail(
+        'Missing dependencies can cause an effect to fire less often than it should',
+        'Extra dependencies can cause an effect to fire more often than it should, resulting' +
+          ' in performance problems such as excessive renders and side effects',
+        '. ',
+      );
+      break;
+    }
+    default: {
+      CompilerError.simpleInvariant(false, {
+        reason: `Unexpected error category: ${category}`,
+        loc: GeneratedSource,
+      });
+    }
+  }
+
+  return CompilerDiagnostic.create({
+    category,
+    reason,
+    description,
+    suggestions: suggestion != null ? [suggestion] : null,
+  });
+}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-dep-on-ref-current-value.expect.md

@@ -21,7 +21,7 @@ function Component() {
 ```
 Found 1 error:
 
-Error: Found missing/extra memoization dependencies
+Error: Found extra memoization dependencies
 
 Extra dependencies can cause a value to update more often than it should, resulting in performance problems such as excessive renders or effects firing too often.
 

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

@@ -25,7 +25,7 @@ function Component() {
 ```
 Found 1 error:
 
-Error: Found missing/extra memoization dependencies
+Error: Found extra memoization dependencies
 
 Extra dependencies can cause a value to update more often than it should, resulting in performance problems such as excessive renders or effects firing too often.
 

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

@@ -51,7 +51,7 @@ function Component({x, y, z}) {
 ## Error
 
 ```
-Found 5 errors:
+Found 4 errors:
 
 Error: Found missing/extra memoization dependencies
 
@@ -101,49 +101,7 @@ error.invalid-exhaustive-deps.ts:17:6
 
 Inferred dependencies: `[x?.y.z.a?.b]`
 
-Error: Found missing/extra memoization dependencies
-
-Extra dependencies can cause a value to update more often than it should, resulting in performance problems such as excessive renders or effects firing too often.
-
-error.invalid-exhaustive-deps.ts:31:6
-  29 |     return [];
-  30 |     // error: unnecessary
-> 31 |   }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
-     |       ^ Unnecessary dependency `x`
-  32 |   const ref1 = useRef(null);
-  33 |   const ref2 = useRef(null);
-  34 |   const ref = z ? ref1 : ref2;
-
-error.invalid-exhaustive-deps.ts:31:9
-  29 |     return [];
-  30 |     // error: unnecessary
-> 31 |   }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
-     |          ^^^ Unnecessary dependency `y.z`
-  32 |   const ref1 = useRef(null);
-  33 |   const ref2 = useRef(null);
-  34 |   const ref = z ? ref1 : ref2;
-
-error.invalid-exhaustive-deps.ts:31:14
-  29 |     return [];
-  30 |     // error: unnecessary
-> 31 |   }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
-     |               ^^^^^^^ Unnecessary dependency `z?.y?.a`
-  32 |   const ref1 = useRef(null);
-  33 |   const ref2 = useRef(null);
-  34 |   const ref = z ? ref1 : ref2;
-
-error.invalid-exhaustive-deps.ts:31:23
-  29 |     return [];
-  30 |     // error: unnecessary
-> 31 |   }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
-     |                        ^^^^^^^^^^^^^ Unnecessary dependency `UNUSED_GLOBAL`. Values declared outside of a component/hook should not be listed as dependencies as the component will not re-render if they change
-  32 |   const ref1 = useRef(null);
-  33 |   const ref2 = useRef(null);
-  34 |   const ref = z ? ref1 : ref2;
-
-Inferred dependencies: `[]`
-
-Error: Found missing/extra memoization dependencies
+Error: Found extra memoization dependencies
 
 Extra dependencies can cause a value to update more often than it should, resulting in performance problems such as excessive renders or effects firing too often.
 
@@ -185,7 +143,7 @@ error.invalid-exhaustive-deps.ts:31:23
 
 Inferred dependencies: `[]`
 
-Error: Found missing/extra memoization dependencies
+Error: Found missing memoization dependencies
 
 Missing dependencies can cause a value to update less often than it should, resulting in stale UI.