fix[eslint-plugin-react-hooks]: Fix error when callback argument is a…

…n identifier with an `as` expression (#31119)
facebook · Nov 19, 2024 · eaf2d5c · eaf2d5c
1 parent c866d75
commit eaf2d5c
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 25 deletions.
diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
@@ -7088,18 +7088,7 @@ const tests = {
       errors: [
         {
           message:
-            "React Hook useEffect has a missing dependency: 'myEffect'. " +
-            'Either include it or remove the dependency array.',
-          suggestions: [
-            {
-              desc: 'Update the dependencies array to be: [myEffect]',
-              output: normalizeIndent`
-                function MyComponent({myEffect}) {
-                  useEffect(myEffect, [myEffect]);
-                }
-              `,
-            },
-          ],
+            'React Hook useEffect received a function whose dependencies are unknown. Pass an inline function instead.',
         },
       ],
     },
@@ -7670,6 +7659,19 @@ const tests = {
         },
       ],
     },
+    {
+      code: normalizeIndent`
+        function useCustomCallback(callback, deps) {
+          return useCallback(callback, deps)
+        }
+      `,
+      errors: [
+        {
+          message:
+            'React Hook useCallback received a function whose dependencies are unknown. Pass an inline function instead.',
+        },
+      ],
+    },
   ],
 };
 
@@ -8193,6 +8195,19 @@ const testsTypescript = {
         },
       ],
     },
+    {
+      code: normalizeIndent`
+        function useCustomCallback(callback, deps) {
+          return useCallback(callback as any, deps)
+        }
+      `,
+      errors: [
+        {
+          message:
+            'React Hook useCallback received a function whose dependencies are unknown. Pass an inline function instead.',
+        },
+      ],
+    },
   ],
 };
 
@@ -8271,6 +8286,10 @@ if (!process.env.CI) {
   testsFlow.invalid = testsFlow.invalid.filter(predicate);
   testsTypescript.valid = testsTypescript.valid.filter(predicate);
   testsTypescript.invalid = testsTypescript.invalid.filter(predicate);
+  testsTypescriptEslintParserV4.valid =
+    testsTypescriptEslintParserV4.valid.filter(predicate);
+  testsTypescriptEslintParserV4.invalid =
+    testsTypescriptEslintParserV4.invalid.filter(predicate);
 }
 
 describe('rules-of-hooks/exhaustive-deps', () => {

diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
@@ -652,6 +652,7 @@ export default {
       const isTSAsArrayExpression =
         declaredDependenciesNode.type === 'TSAsExpression' &&
         declaredDependenciesNode.expression.type === 'ArrayExpression';
+
       if (!isArrayExpression && !isTSAsArrayExpression) {
         // If the declared dependencies are not an array expression then we
         // can't verify that the user provided the correct dependencies. Tell
@@ -1196,7 +1197,7 @@ export default {
         // Not a React Hook call that needs deps.
         return;
       }
-      const callback = node.arguments[callbackIndex];
+      let callback = node.arguments[callbackIndex];
       const reactiveHook = node.callee;
       const reactiveHookName = getNodeWithoutReactNamespace(reactiveHook).name;
       const maybeNode = node.arguments[callbackIndex + 1];
@@ -1241,6 +1242,13 @@ export default {
         return;
       }
 
+      while (
+        callback.type === 'TSAsExpression' ||
+        callback.type === 'AsExpression'
+      ) {
+        callback = callback.expression;
+      }
+
       switch (callback.type) {
         case 'FunctionExpression':
         case 'ArrowFunctionExpression':
@@ -1252,15 +1260,6 @@ export default {
             isEffect,
           );
           return; // Handled
-        case 'TSAsExpression':
-          visitFunctionWithDependencies(
-            callback.expression,
-            declaredDependenciesNode,
-            reactiveHook,
-            reactiveHookName,
-            isEffect,
-          );
-          return; // Handled
         case 'Identifier':
           if (!declaredDependenciesNode) {
             // No deps, no problems.
@@ -1291,6 +1290,13 @@ export default {
           if (!def || !def.node) {
             break; // Unhandled
           }
+          if (def.type === 'Parameter') {
+            reportProblem({
+              node: reactiveHook,
+              message: getUnknownDependenciesMessage(reactiveHookName),
+            });
+            return;
+          }
           if (def.type !== 'Variable' && def.type !== 'FunctionName') {
             // Parameter or an unusual pattern. Bail out.
             break; // Unhandled
@@ -1333,9 +1339,7 @@ export default {
           // useEffect(generateEffectBody(), []);
           reportProblem({
             node: reactiveHook,
-            message:
-              `React Hook ${reactiveHookName} received a function whose dependencies ` +
-              `are unknown. Pass an inline function instead.`,
+            message: getUnknownDependenciesMessage(reactiveHookName),
           });
           return; // Handled
       }
@@ -1912,3 +1916,10 @@ function isUseEffectEventIdentifier(node) {
   }
   return false;
 }
+
+function getUnknownDependenciesMessage(reactiveHookName) {
+  return (
+    `React Hook ${reactiveHookName} received a function whose dependencies ` +
+    `are unknown. Pass an inline function instead.`
+  );
+}