[eslint-plugin-exhaustive-deps] Fix exhaustive deps check for unstab…

…le vars (#24343) * Fix exhaustive deps for unstable vars * Fix formatting * Optimise iterations * Fix linting
facebook · Apr 11, 2022 · 069d23b · 069d23b
1 parent 4997515
commit 069d23b
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 3 deletions.
diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
@@ -1095,6 +1095,22 @@ const tests = {
         }
       `,
     },
+    {
+      code: normalizeIndent`
+        function Counter(unstableProp) {
+          let [count, setCount] = useState(0);
+          setCount = unstableProp
+          useEffect(() => {
+            let id = setInterval(() => {
+              setCount(c => c + 1);
+            }, 1000);
+            return () => clearInterval(id);
+          }, [setCount]);
+
+          return <h1>{count}</h1>;
+        }
+      `,
+    },
     {
       code: normalizeIndent`
         function Counter() {
@@ -1581,6 +1597,48 @@ const tests = {
         },
       ],
     },
+    {
+      code: normalizeIndent`
+        function Counter(unstableProp) {
+          let [count, setCount] = useState(0);
+          setCount = unstableProp
+          useEffect(() => {
+            let id = setInterval(() => {
+              setCount(c => c + 1);
+            }, 1000);
+            return () => clearInterval(id);
+          }, []);
+
+          return <h1>{count}</h1>;
+        }
+      `,
+      errors: [
+        {
+          message:
+            "React Hook useEffect has a missing dependency: 'setCount'. " +
+            'Either include it or remove the dependency array.',
+          suggestions: [
+            {
+              desc: 'Update the dependencies array to be: [setCount]',
+              output: normalizeIndent`
+                function Counter(unstableProp) {
+                  let [count, setCount] = useState(0);
+                  setCount = unstableProp
+                  useEffect(() => {
+                    let id = setInterval(() => {
+                      setCount(c => c + 1);
+                    }, 1000);
+                    return () => clearInterval(id);
+                  }, [setCount]);
+        
+                  return <h1>{count}</h1>;
+                }
+              `,
+            },
+          ],
+        },
+      ],
+    },
     {
       // Note: we *could* detect it's a primitive and never assigned
       // even though it's not a constant -- but we currently don't.

diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
@@ -234,7 +234,14 @@ export default {
             if (id.elements[1] === resolved.identifiers[0]) {
               if (name === 'useState') {
                 const references = resolved.references;
+                let writeCount = 0;
                 for (let i = 0; i < references.length; i++) {
+                  if (references[i].isWrite()) {
+                    writeCount++;
+                  }
+                  if (writeCount > 1) {
+                    return false;
+                  }
                   setStateCallSites.set(
                     references[i].identifier,
                     id.elements[0],
@@ -321,7 +328,7 @@ export default {
             pureScopes.has(ref.resolved.scope) &&
             // Stable values are fine though,
             // although we won't check functions deeper.
-            !memoizedIsStablecKnownHookValue(ref.resolved)
+            !memoizedIsStableKnownHookValue(ref.resolved)
           ) {
             return false;
           }
@@ -332,7 +339,7 @@ export default {
       }
 
       // Remember such values. Avoid re-running extra checks on them.
-      const memoizedIsStablecKnownHookValue = memoizeWithWeakMap(
+      const memoizedIsStableKnownHookValue = memoizeWithWeakMap(
         isStableKnownHookValue,
         stableKnownValueCache,
       );
@@ -435,7 +442,7 @@ export default {
           if (!dependencies.has(dependency)) {
             const resolved = reference.resolved;
             const isStable =
-              memoizedIsStablecKnownHookValue(resolved) ||
+              memoizedIsStableKnownHookValue(resolved) ||
               memoizedIsFunctionWithoutCapturedValues(resolved);
             dependencies.set(dependency, {
               isStable,