Add new ErrorCategory.EffectExhaustiveDependencies and update reason/description for effect dep errors

Author: jackpope

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/Validation/ValidateExhaustiveDependencies.ts

@@ -140,6 +140,7 @@ export function validateExhaustiveDependencies(
         startMemo.deps ?? [],
         reactive,
         startMemo.depsLoc,
+        ErrorCategory.MemoDependencies,
       );
       if (diagnostic != null) {
         error.pushDiagnostic(diagnostic);
@@ -199,6 +200,7 @@ export function validateExhaustiveDependencies(
           manualDeps,
           reactive,
           null,
+          ErrorCategory.EffectExhaustiveDependencies,
         );
         if (diagnostic != null) {
           error.pushDiagnostic(diagnostic);
@@ -215,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) => {
@@ -373,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)) {
@@ -1001,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

@@ -101,7 +101,7 @@ error.invalid-exhaustive-deps.ts:17:6
 
 Inferred dependencies: `[x?.y.z.a?.b]`
 
-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.
 
@@ -143,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.
 

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

@@ -8,13 +8,23 @@ import {useEffect} from 'react';
 function Component({x, y, z}) {
   // error: missing dep - x
   useEffect(() => {
-    console.log(x);
+    log(x);
   }, []);
 
   // error: extra dep - y
   useEffect(() => {
-    console.log(x);
+    log(x);
   }, [x, y]);
+
+  // error: missing dep - z; extra dep - y
+  useEffect(() => {
+    log(x, z);
+  }, [x, y]);
+
+  // error: missing dep x
+  useEffect(() => {
+    log(x);
+  }, [x.y]);
 }
 
 ```
@@ -23,32 +33,76 @@ function Component({x, y, z}) {
 ## Error
 
 ```
-Found 2 errors:
+Found 4 errors:
 
-Error: Found missing/extra memoization dependencies
+Error: Found missing effect dependencies
 
-Missing dependencies can cause a value to update less often than it should, resulting in stale UI.
+Missing dependencies can cause an effect to fire less often than it should.
 
-error.invalid-exhaustive-effect-deps.ts:7:16
+error.invalid-exhaustive-effect-deps.ts:7:8
    5 |   // error: missing dep - x
    6 |   useEffect(() => {
->  7 |     console.log(x);
-     |                 ^ Missing dependency `x`
+>  7 |     log(x);
+     |         ^ Missing dependency `x`
    8 |   }, []);
    9 |
   10 |   // error: extra dep - y
 
-Error: Found missing/extra memoization dependencies
+Error: Found extra effect 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.
+Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects.
 
 error.invalid-exhaustive-effect-deps.ts:13:9
   11 |   useEffect(() => {
-  12 |     console.log(x);
+  12 |     log(x);
 > 13 |   }, [x, y]);
      |          ^ Unnecessary dependency `y`
-  14 | }
-  15 |
+  14 |
+  15 |   // error: missing dep - z; extra dep - y
+  16 |   useEffect(() => {
+
+Error: Found missing/extra effect dependencies
+
+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.
+
+error.invalid-exhaustive-effect-deps.ts:17:11
+  15 |   // error: missing dep - z; extra dep - y
+  16 |   useEffect(() => {
+> 17 |     log(x, z);
+     |            ^ Missing dependency `z`
+  18 |   }, [x, y]);
+  19 |
+  20 |   // error: missing dep x
+
+error.invalid-exhaustive-effect-deps.ts:18:9
+  16 |   useEffect(() => {
+  17 |     log(x, z);
+> 18 |   }, [x, y]);
+     |          ^ Unnecessary dependency `y`
+  19 |
+  20 |   // error: missing dep x
+  21 |   useEffect(() => {
+
+Error: Found missing/extra effect dependencies
+
+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.
+
+error.invalid-exhaustive-effect-deps.ts:22:8
+  20 |   // error: missing dep x
+  21 |   useEffect(() => {
+> 22 |     log(x);
+     |         ^ Missing dependency `x`
+  23 |   }, [x.y]);
+  24 | }
+  25 |
+
+error.invalid-exhaustive-effect-deps.ts:23:6
+  21 |   useEffect(() => {
+  22 |     log(x);
+> 23 |   }, [x.y]);
+     |       ^^^ Overly precise dependency `x.y`, use `x` instead
+  24 | }
+  25 |
 ```
           
       

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

@@ -4,11 +4,21 @@ import {useEffect} from 'react';
 function Component({x, y, z}) {
   // error: missing dep - x
   useEffect(() => {
-    console.log(x);
+    log(x);
   }, []);
 
   // error: extra dep - y
   useEffect(() => {
-    console.log(x);
+    log(x);
   }, [x, y]);
+
+  // error: missing dep - z; extra dep - y
+  useEffect(() => {
+    log(x, z);
+  }, [x, y]);
+
+  // error: missing dep x
+  useEffect(() => {
+    log(x);
+  }, [x.y]);
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep-inner-function.expect.md

@@ -26,7 +26,7 @@ function useHook() {
 ```
 Found 1 error:
 
-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.
 

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

@@ -24,7 +24,7 @@ function useHook() {
 ```
 Found 1 error:
 
-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.
 

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

@@ -21,7 +21,7 @@ function useHook() {
 ```
 Found 1 error:
 
-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.