Fix ESLint crash on empty react effect hook (facebook#20385)

* Fix ESLint crash on empty react effect hook * Add layout effect to test * Improve wording in comment * Improve lint warning wording * Reword missing effect callback message
koto · Jun 15, 2021 · cc915a2 · cc915a2
1 parent 00df81a
commit cc915a2
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 0 deletions.
diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
@@ -1692,6 +1692,42 @@ const tests = {
         },
       ],
     },
+    {
+      code: normalizeIndent`
+        function MyComponent() {
+          useEffect()
+          useLayoutEffect()
+          useCallback()
+          useMemo()
+        }
+      `,
+      errors: [
+        {
+          message:
+            'React Hook useEffect requires an effect callback. ' +
+            'Did you forget to pass a callback to the hook?',
+          suggestions: undefined,
+        },
+        {
+          message:
+            'React Hook useLayoutEffect requires an effect callback. ' +
+            'Did you forget to pass a callback to the hook?',
+          suggestions: undefined,
+        },
+        {
+          message:
+            'React Hook useCallback requires an effect callback. ' +
+            'Did you forget to pass a callback to the hook?',
+          suggestions: undefined,
+        },
+        {
+          message:
+            'React Hook useMemo requires an effect callback. ' +
+            'Did you forget to pass a callback to the hook?',
+          suggestions: undefined,
+        },
+      ],
+    },
     {
       // Regression test
       code: normalizeIndent`

diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
@@ -1119,6 +1119,19 @@ export default {
       const declaredDependenciesNode = node.arguments[callbackIndex + 1];
       const isEffect = /Effect($|[^a-z])/g.test(reactiveHookName);
 
+      // Check whether a callback is supplied. If there is no callback supplied
+      // then the hook will not work and React will throw a TypeError.
+      // So no need to check for dependency inclusion.
+      if (!callback) {
+        reportProblem({
+          node: reactiveHook,
+          message:
+            `React Hook ${reactiveHookName} requires an effect callback. ` +
+            `Did you forget to pass a callback to the hook?`,
+        });
+        return;
+      }
+
       // Check the declared dependencies for this reactive hook. If there is no
       // second argument then the reactive callback will re-run on every render.
       // So no need to check for dependency inclusion.