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
facebook · Jan 25, 2019 · e19c9e1 · e19c9e1
1 parent f11a9c1
commit e19c9e1
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 1 deletion.
diff --git 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
@@ -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;
         }