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

gaearon · web-flow · commit 47915fd6e185 · 2020-07-10T19:02:08.000+01:00
* Add regression test for ESLint rule

* Fix the issue
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 {
