From 518e917af27e1cdc8f59ee2b7b1bc927c35c3b10 Mon Sep 17 00:00:00 2001 From: Christian Ruigrok Date: Sat, 5 Dec 2020 13:37:55 +0100 Subject: [PATCH 1/5] Fix ESLint crash on empty react effect hook --- .../ESLintRuleExhaustiveDeps-test.js | 29 +++++++++++++++++++ .../src/ExhaustiveDeps.js | 13 +++++++++ 2 files changed, 42 insertions(+) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index 0c782dc25ae0..d0063d9f9905 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -1692,6 +1692,35 @@ const tests = { }, ], }, + { + code: normalizeIndent` + function MyComponent() { + useEffect() + useCallback() + useMemo() + } + `, + errors: [ + { + message: + 'React Hook useEffect will crash when called with no arguments. ' + + 'Did you forget to pass a function and an array of dependencies?', + suggestions: undefined, + }, + { + message: + 'React Hook useCallback will crash when called with no arguments. ' + + 'Did you forget to pass a function and an array of dependencies?', + suggestions: undefined, + }, + { + message: + 'React Hook useMemo will crash when called with no arguments. ' + + 'Did you forget to pass a function and 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 7c970f7c8f6a..1f6e75736eb4 100644 --- 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 to useEffect. 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} will crash when called with no arguments. ` + + `Did you forget to pass a function and an array of dependencies?`, + }); + 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. From 3fab8c1f98f619eb5e5b5f46e2519e59ae654dc5 Mon Sep 17 00:00:00 2001 From: Christian Ruigrok Date: Sat, 5 Dec 2020 13:49:35 +0100 Subject: [PATCH 2/5] Add layout effect to test --- .../__tests__/ESLintRuleExhaustiveDeps-test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index d0063d9f9905..9a562603a4ed 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -1696,6 +1696,7 @@ const tests = { code: normalizeIndent` function MyComponent() { useEffect() + useLayoutEffect() useCallback() useMemo() } @@ -1707,6 +1708,12 @@ const tests = { 'Did you forget to pass a function and an array of dependencies?', suggestions: undefined, }, + { + message: + 'React Hook useLayoutEffect will crash when called with no arguments. ' + + 'Did you forget to pass a function and an array of dependencies?', + suggestions: undefined, + }, { message: 'React Hook useCallback will crash when called with no arguments. ' + From 204bbd433b822e106c85c707fcf23258166f7011 Mon Sep 17 00:00:00 2001 From: Christian Ruigrok Date: Sat, 5 Dec 2020 13:53:13 +0100 Subject: [PATCH 3/5] Improve wording in comment --- packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js index 1f6e75736eb4..b58f0a0df8fc 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -1119,8 +1119,8 @@ export default { const declaredDependenciesNode = node.arguments[callbackIndex + 1]; const isEffect = /Effect($|[^a-z])/g.test(reactiveHookName); - // Check whether a callback is supplied to useEffect. If there is no - // callback supplied then the hook will not work and React will throw a TypeError. + // 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({ From bbb11f310fd2bb82440fccf6a1a785860aff5827 Mon Sep 17 00:00:00 2001 From: Christian Ruigrok Date: Sat, 5 Dec 2020 13:57:53 +0100 Subject: [PATCH 4/5] Improve lint warning wording --- .../__tests__/ESLintRuleExhaustiveDeps-test.js | 8 ++++---- packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index 9a562603a4ed..dcb224ce6199 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -1705,25 +1705,25 @@ const tests = { { message: 'React Hook useEffect will crash when called with no arguments. ' + - 'Did you forget to pass a function and an array of dependencies?', + 'Did you forget to pass a callback to the hook?', suggestions: undefined, }, { message: 'React Hook useLayoutEffect will crash when called with no arguments. ' + - 'Did you forget to pass a function and an array of dependencies?', + 'Did you forget to pass a callback to the hook?', suggestions: undefined, }, { message: 'React Hook useCallback will crash when called with no arguments. ' + - 'Did you forget to pass a function and an array of dependencies?', + 'Did you forget to pass a callback to the hook?', suggestions: undefined, }, { message: 'React Hook useMemo will crash when called with no arguments. ' + - 'Did you forget to pass a function and an array of dependencies?', + 'Did you forget to pass a callback to the hook?', suggestions: undefined, }, ], diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js index b58f0a0df8fc..e339a4f7ca37 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -1127,7 +1127,7 @@ export default { node: reactiveHook, message: `React Hook ${reactiveHookName} will crash when called with no arguments. ` + - `Did you forget to pass a function and an array of dependencies?`, + `Did you forget to pass a callback to the hook?`, }); return; } From d5767cd6c1bc2a779ed1541d83f34ae11f02c5a3 Mon Sep 17 00:00:00 2001 From: Christian Ruigrok Date: Mon, 11 Jan 2021 19:40:47 +0100 Subject: [PATCH 5/5] Reword missing effect callback message --- .../__tests__/ESLintRuleExhaustiveDeps-test.js | 8 ++++---- packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index dcb224ce6199..2a2786dca885 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -1704,25 +1704,25 @@ const tests = { errors: [ { message: - 'React Hook useEffect will crash when called with no arguments. ' + + 'React Hook useEffect requires an effect callback. ' + 'Did you forget to pass a callback to the hook?', suggestions: undefined, }, { message: - 'React Hook useLayoutEffect will crash when called with no arguments. ' + + 'React Hook useLayoutEffect requires an effect callback. ' + 'Did you forget to pass a callback to the hook?', suggestions: undefined, }, { message: - 'React Hook useCallback will crash when called with no arguments. ' + + 'React Hook useCallback requires an effect callback. ' + 'Did you forget to pass a callback to the hook?', suggestions: undefined, }, { message: - 'React Hook useMemo will crash when called with no arguments. ' + + 'React Hook useMemo requires an effect callback. ' + 'Did you forget to pass a callback to the hook?', suggestions: undefined, }, diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js index e339a4f7ca37..ee259b2d37cd 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -1126,7 +1126,7 @@ export default { reportProblem({ node: reactiveHook, message: - `React Hook ${reactiveHookName} will crash when called with no arguments. ` + + `React Hook ${reactiveHookName} requires an effect callback. ` + `Did you forget to pass a callback to the hook?`, }); return;