[ESLint] Fix a bug causing a too coarse dependency suggestion (#19313)

* Add regression test for ESLint rule * Fix the issue
facebook · Jul 10, 2020 · 47915fd · 47915fd
1 parent d5d6590
commit 47915fd
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 1 deletion.
diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
@@ -1343,6 +1343,39 @@ const tests = {
         }
       `,
     },
+    // Regression test.
+    {
+      code: normalizeIndent`
+        function Example(props) {
+          useEffect(() => {
+            let topHeight = 0;
+            topHeight = props.upperViewHeight;
+          }, [props.upperViewHeight]);
+        }
+      `,
+    },
+    // Regression test.
+    {
+      code: normalizeIndent`
+        function Example(props) {
+          useEffect(() => {
+            let topHeight = 0;
+            topHeight = props?.upperViewHeight;
+          }, [props?.upperViewHeight]);
+        }
+      `,
+    },
+    // Regression test.
+    {
+      code: normalizeIndent`
+        function Example(props) {
+          useEffect(() => {
+            let topHeight = 0;
+            topHeight = props?.upperViewHeight;
+          }, [props]);
+        }
+      `,
+    },
   ],
   invalid: [
     {
@@ -7120,6 +7153,70 @@ const testsTypescript = {
         },
       ],
     },
+    // Regression test.
+    {
+      code: normalizeIndent`
+        function Example(props) {
+          useEffect(() => {
+            let topHeight = 0;
+            topHeight = props.upperViewHeight;
+          }, []);
+        }
+      `,
+      errors: [
+        {
+          message:
+            "React Hook useEffect has a missing dependency: 'props.upperViewHeight'. " +
+            'Either include it or remove the dependency array.',
+          suggestions: [
+            {
+              desc:
+                'Update the dependencies array to be: [props.upperViewHeight]',
+              output: normalizeIndent`
+                function Example(props) {
+                  useEffect(() => {
+                    let topHeight = 0;
+                    topHeight = props.upperViewHeight;
+                  }, [props.upperViewHeight]);
+                }
+              `,
+            },
+          ],
+        },
+      ],
+    },
+    // Regression test.
+    {
+      code: normalizeIndent`
+        function Example(props) {
+          useEffect(() => {
+            let topHeight = 0;
+            topHeight = props?.upperViewHeight;
+          }, []);
+        }
+      `,
+      errors: [
+        {
+          message:
+            "React Hook useEffect has a missing dependency: 'props?.upperViewHeight'. " +
+            'Either include it or remove the dependency array.',
+          suggestions: [
+            {
+              desc:
+                'Update the dependencies array to be: [props?.upperViewHeight]',
+              output: normalizeIndent`
+                function Example(props) {
+                  useEffect(() => {
+                    let topHeight = 0;
+                    topHeight = props?.upperViewHeight;
+                  }, [props?.upperViewHeight]);
+                }
+              `,
+            },
+          ],
+        },
+      ],
+    },
   ],
 };
 

diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
@@ -1478,7 +1478,8 @@ function getDependency(node) {
     // Note: we don't check OptionalMemberExpression because it can't be LHS.
     node.type === 'MemberExpression' &&
     node.parent &&
-    node.parent.type === 'AssignmentExpression'
+    node.parent.type === 'AssignmentExpression' &&
+    node.parent.left === node
   ) {
     return node.object;
   } else {