Add regression tests for every occurrence of Optional*

packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js

@@ -766,6 +766,24 @@ const tests = {
         }
       `,
     },
+    {
+      // Valid because we assign ref.current
+      // ourselves. Therefore it's likely not
+      // a ref managed by React.
+      code: normalizeIndent`
+        function MyComponent() {
+          const myRef = useRef();
+          useEffect(() => {
+            const handleMove = () => {};
+            myRef.current = {};
+            return () => {
+              console.log(myRef?.current?.toString())
+            };
+          }, []);
+          return <div />;
+        }
+      `,
+    },
     {
       // Valid because we assign ref.current
       // ourselves. Therefore it's likely not
@@ -3639,6 +3657,47 @@ const tests = {
         },
       ],
     },
+    {
+      code: normalizeIndent`
+        function MyComponent(props) {
+          const ref1 = useRef();
+          const ref2 = useRef();
+          useEffect(() => {
+            ref1?.current?.focus();
+            console.log(ref2?.current?.textContent);
+            alert(props.someOtherRefs.current.innerHTML);
+            fetch(props.color);
+          }, [ref1?.current, ref2?.current, props.someOtherRefs, props.color]);
+        }
+      `,
+      errors: [
+        {
+          message:
+            "React Hook useEffect has unnecessary dependencies: 'ref1.current' and 'ref2.current'. " +
+            'Either exclude them or remove the dependency array. ' +
+            "Mutable values like 'ref1.current' aren't valid dependencies " +
+            "because mutating them doesn't re-render the component.",
+          suggestions: [
+            {
+              desc:
+                'Update the dependencies array to be: [props.someOtherRefs, props.color]',
+              output: normalizeIndent`
+                function MyComponent(props) {
+                  const ref1 = useRef();
+                  const ref2 = useRef();
+                  useEffect(() => {
+                    ref1?.current?.focus();
+                    console.log(ref2?.current?.textContent);
+                    alert(props.someOtherRefs.current.innerHTML);
+                    fetch(props.color);
+                  }, [props.someOtherRefs, props.color]);
+                }
+              `,
+            },
+          ],
+        },
+      ],
+    },
     {
       code: normalizeIndent`
         function MyComponent() {
@@ -3843,6 +3902,42 @@ const tests = {
         },
       ],
     },
+    {
+      code: normalizeIndent`
+        function MyComponent(props) {
+          useEffect(() => {
+            if (props?.onChange) {
+              props?.onChange();
+            }
+          }, []);
+        }
+      `,
+      errors: [
+        {
+          message:
+            "React Hook useEffect has a missing dependency: 'props'. " +
+            'Either include it or remove the dependency array. ' +
+            `However, 'props' will change when *any* prop changes, so the ` +
+            `preferred fix is to destructure the 'props' object outside ` +
+            `of the useEffect call and refer to those specific ` +
+            `props inside useEffect.`,
+          suggestions: [
+            {
+              desc: 'Update the dependencies array to be: [props]',
+              output: normalizeIndent`
+                function MyComponent(props) {
+                  useEffect(() => {
+                    if (props?.onChange) {
+                      props?.onChange();
+                    }
+                  }, [props]);
+                }
+              `,
+            },
+          ],
+        },
+      ],
+    },
     {
       code: normalizeIndent`
         function MyComponent(props) {
@@ -4234,6 +4329,29 @@ const tests = {
         },
       ],
     },
+    {
+      code: normalizeIndent`
+        function MyComponent() {
+          const myRef = useRef();
+          useEffect(() => {
+            const handleMove = () => {};
+            myRef?.current?.addEventListener('mousemove', handleMove);
+            return () => myRef?.current?.removeEventListener('mousemove', handleMove);
+          }, []);
+          return <div ref={myRef} />;
+        }
+      `,
+      errors: [
+        {
+          message:
+            `The ref value 'myRef.current' will likely have changed by the time ` +
+            `this effect cleanup function runs. If this ref points to a node ` +
+            `rendered by React, copy 'myRef.current' to a variable inside the effect, ` +
+            `and use that variable in the cleanup function.`,
+          suggestions: undefined,
+        },
+      ],
+    },
     {
       code: normalizeIndent`
         function MyComponent() {
@@ -4608,6 +4726,51 @@ const tests = {
         },
       ],
     },
+    {
+      code: normalizeIndent`
+        import MutableStore from 'store';
+        let z = {};
+
+        function MyComponent(props) {
+          let x = props.foo;
+          {
+            let y = props.bar;
+            const fn = useCallback(() => {
+              // nothing
+            }, [MutableStore?.hello?.world, props.foo, x, y, z, global?.stuff]);
+          }
+        }
+      `,
+      errors: [
+        {
+          message:
+            'React Hook useCallback has unnecessary dependencies: ' +
+            "'MutableStore.hello.world', 'global.stuff', 'props.foo', 'x', 'y', and 'z'. " +
+            'Either exclude them or remove the dependency array. ' +
+            "Outer scope values like 'MutableStore.hello.world' aren't valid dependencies " +
+            "because mutating them doesn't re-render the component.",
+          suggestions: [
+            {
+              desc: 'Update the dependencies array to be: []',
+              output: normalizeIndent`
+                import MutableStore from 'store';
+                let z = {};
+
+                function MyComponent(props) {
+                  let x = props.foo;
+                  {
+                    let y = props.bar;
+                    const fn = useCallback(() => {
+                      // nothing
+                    }, []);
+                  }
+                }
+              `,
+            },
+          ],
+        },
+      ],
+    },
     {
       // Every almost-static function is tainted by a dynamic value.
       code: normalizeIndent`
@@ -6168,6 +6331,40 @@ const tests = {
         },
       ],
     },
+    {
+      code: normalizeIndent`
+        function Podcasts({ fetchPodcasts, id }) {
+          let [podcasts, setPodcasts] = useState(null);
+          useEffect(() => {
+            console.log(fetchPodcasts);
+            fetchPodcasts?.(id).then(setPodcasts);
+          }, [id]);
+        }
+      `,
+      errors: [
+        {
+          message:
+            `React Hook useEffect has a missing dependency: 'fetchPodcasts'. ` +
+            `Either include it or remove the dependency array. ` +
+            `If 'fetchPodcasts' changes too often, ` +
+            `find the parent component that defines it and wrap that definition in useCallback.`,
+          suggestions: [
+            {
+              desc: 'Update the dependencies array to be: [fetchPodcasts, id]',
+              output: normalizeIndent`
+                function Podcasts({ fetchPodcasts, id }) {
+                  let [podcasts, setPodcasts] = useState(null);
+                  useEffect(() => {
+                    console.log(fetchPodcasts);
+                    fetchPodcasts?.(id).then(setPodcasts);
+                  }, [fetchPodcasts, id]);
+                }
+              `,
+            },
+          ],
+        },
+      ],
+    },
     {
       // The mistake here is that it was moved inside the effect
       // so it can't be referenced in the deps array.

packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js

@@ -588,8 +588,8 @@ export default {
             if (
               parent != null &&
               // ref.current
-              (parent.type === 'MemberExpression' ||
-                parent.type === 'OptionalMemberExpression') &&
+              // Note: no need to handle OptionalMemberExpression because it can't be LHS.
+              parent.type === 'MemberExpression' &&
               !parent.computed &&
               parent.property.type === 'Identifier' &&
               parent.property.name === 'current' &&
@@ -1452,8 +1452,8 @@ function getDependency(node) {
   ) {
     return getDependency(node.parent);
   } else if (
-    (node.type === 'MemberExpression' ||
-      node.type === 'OptionalMemberExpression') &&
+    // Note: we don't check OptionalMemberExpression because it can't be LHS.
+    node.type === 'MemberExpression' &&
     node.parent &&
     node.parent.type === 'AssignmentExpression'
   ) {