diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index 57ce80b99f..1b6e84d889 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -357,6 +357,14 @@ const tests = { } `, }, + { + // Valid because has no deps. + code: normalizeIndent` + function MyComponent({myEffect}) { + useEffect(myEffect); + } + `, + }, { code: normalizeIndent` function MyComponent(props) { @@ -1273,6 +1281,29 @@ const tests = { }, ], }, + { + // Invalid because they don't have a meaning without deps. + code: normalizeIndent` + function MyComponent({ fn1, fn2 }) { + const value = useMemo(fn1); + const fn = useCallback(fn2); + } + `, + errors: [ + { + message: + 'React Hook useMemo does nothing when called with only one argument. ' + + 'Did you forget to pass an array of dependencies?', + suggestions: undefined, + }, + { + message: + 'React Hook useCallback does nothing when called with only one argument. ' + + 'Did you forget to pass an array of dependencies?', + 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 index 9e439da199..2355016172 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -93,6 +93,28 @@ export default { const reactiveHook = node.callee; const reactiveHookName = getNodeWithoutReactNamespace(reactiveHook).name; const declaredDependenciesNode = node.arguments[callbackIndex + 1]; + const isEffect = /Effect($|[^a-z])/g.test(reactiveHookName); + + // 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. + if (!declaredDependenciesNode && !isEffect) { + // These are only used for optimization. + if ( + reactiveHookName === 'useMemo' || + reactiveHookName === 'useCallback' + ) { + // TODO: Can this have a suggestion? + reportProblem({ + node: reactiveHook, + message: + `React Hook ${reactiveHookName} does nothing when called with ` + + `only one argument. Did you forget to pass an array of ` + + `dependencies?`, + }); + } + return; + } switch (callback.type) { case 'FunctionExpression': @@ -101,13 +123,18 @@ export default { callback, declaredDependenciesNode, reactiveHook, + reactiveHookName, + isEffect, ); return; // Handled case 'Identifier': + if (!declaredDependenciesNode) { + // No deps, no problems. + return; // Handled + } // The function passed as a callback is not written inline. // But perhaps it's in the dependencies array? if ( - declaredDependenciesNode && declaredDependenciesNode.elements && declaredDependenciesNode.elements.some( el => el.type === 'Identifier' && el.name === callback.name, @@ -141,6 +168,8 @@ export default { def.node, declaredDependenciesNode, reactiveHook, + reactiveHookName, + isEffect, ); return; // Handled case 'VariableDeclarator': @@ -158,6 +187,8 @@ export default { init, declaredDependenciesNode, reactiveHook, + reactiveHookName, + isEffect, ); return; // Handled } @@ -202,31 +233,9 @@ export default { node, declaredDependenciesNode, reactiveHook, + reactiveHookName, + isEffect, ) { - const reactiveHookName = getNodeWithoutReactNamespace(reactiveHook).name; - const isEffect = /Effect($|[^a-z])/g.test(reactiveHookName); - - // 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. - if (!declaredDependenciesNode && !isEffect) { - // These are only used for optimization. - if ( - reactiveHookName === 'useMemo' || - reactiveHookName === 'useCallback' - ) { - // TODO: Can this have a suggestion? - reportProblem({ - node: reactiveHook, - message: - `React Hook ${reactiveHookName} does nothing when called with ` + - `only one argument. Did you forget to pass an array of ` + - `dependencies?`, - }); - } - return; - } - if (isEffect && node.async) { reportProblem({ node: node,