From e19c9e106401fd41c91b67273d4c22289ed0917c Mon Sep 17 00:00:00 2001 From: Yurick Date: Fri, 25 Jan 2019 15:06:33 -0200 Subject: [PATCH] Fix issue with multiple code branches in hooks linter (#14661) * Fix issue with multiple code branches * Add solution by @calebmer * Add performance test * Undo unrelated change --- .../__tests__/ESLintRulesOfHooks-test.js | 69 +++++++++++++++++++ .../src/RulesOfHooks.js | 9 ++- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index 410f5d1646f69..4a5623dcd4d61 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -270,6 +270,75 @@ eslintTester.run('react-hooks', ReactHooksESLintRule, { useState(); } `, + ` + // Valid because the loop doesn't change the order of hooks calls. + function RegressionTest() { + const res = []; + const additionalCond = true; + for (let i = 0; i !== 10 && additionalCond; ++i ) { + res.push(i); + } + React.useLayoutEffect(() => {}); + } + `, + ` + // Is valid but hard to compute by brute-forcing + function MyComponent() { + // 40 conditions + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + if (c) {} else {} + + // 10 hooks + useHook(); + useHook(); + useHook(); + useHook(); + useHook(); + useHook(); + useHook(); + useHook(); + useHook(); + useHook(); + } + `, ], invalid: [ { diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js index 7c1f0bc6f9309..111cb054ad0ab 100644 --- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js @@ -149,7 +149,14 @@ export default { paths += countPathsFromStart(prevSegment); } } - cache.set(segment.id, paths); + + // If our segment is reachable then there should be at least one path + // to it from the start of our code path. + if (segment.reachable && paths === 0) { + cache.delete(segment.id); + } else { + cache.set(segment.id, paths); + } return paths; }