From 5c6c48bbff32123d2a7973a762797cd00d2c4232 Mon Sep 17 00:00:00 2001
From: Dan Abramov <dan.abramov@gmail.com>
Date: Tue, 19 Feb 2019 16:23:48 +0000
Subject: [PATCH] Add special message for ref.current

---
 .../__tests__/ESLintRuleReactiveDeps-test.js  | 38 +++++++++++++++++--
 .../src/ReactiveDeps.js                       | 22 ++++++++++-
 2 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleReactiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleReactiveDeps-test.js
index ee96f02ffa1c9..cdb3f6c94133f 100644
--- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleReactiveDeps-test.js
+++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleReactiveDeps-test.js
@@ -1263,12 +1263,43 @@ const tests = {
           }, [ref1, ref2, props.someOtherRefs, props.color]);
         }
       `,
-      // TODO: special message for the ref case.
       errors: [
         "React Hook useEffect has missing dependencies: 'props.someOtherRefs' and 'props.color'. " +
           'Either include them or remove the dependency array.',
       ],
     },
+    {
+      code: `
+        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]);
+        }
+      `,
+      output: `
+        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, ref1, ref2]);
+        }
+      `,
+      errors: [
+        "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 their mutation doesn't re-render a component.",
+      ],
+    },
     {
       code: `
         function MyComponent() {
@@ -1286,10 +1317,11 @@ const tests = {
           }, [ref]);
         }
       `,
-      // TODO: special message for the ref case.
       errors: [
         "React Hook useEffect has an unnecessary dependency: 'ref.current'. " +
-          'Either exclude it or remove the dependency array.',
+          'Either exclude it or remove the dependency array. ' +
+          "Mutable values like 'ref.current' aren't valid dependencies " +
+          "because their mutation doesn't re-render a component.",
       ],
     },
     {
diff --git a/packages/eslint-plugin-react-hooks/src/ReactiveDeps.js b/packages/eslint-plugin-react-hooks/src/ReactiveDeps.js
index df1c5f3c6164a..1d56aa6a02b95 100644
--- a/packages/eslint-plugin-react-hooks/src/ReactiveDeps.js
+++ b/packages/eslint-plugin-react-hooks/src/ReactiveDeps.js
@@ -302,6 +302,7 @@ export default {
       let duplicateDependencies = new Set();
       let unnecessaryDependencies = new Set();
       let missingDependencies = new Set();
+      let showRefCurrentWarning = false;
 
       let actualDependencies = Array.from(dependencies.keys());
 
@@ -382,6 +383,24 @@ export default {
           } or remove the dependency array.`
         );
       };
+      let extraWarning = '';
+      if (unnecessaryDependencies.size > 0) {
+        let badRef = null;
+        Array.from(unnecessaryDependencies.keys()).forEach(key => {
+          if (badRef !== null) {
+            return;
+          }
+          if (key.endsWith('.current')) {
+            badRef = key;
+          }
+        });
+        if (badRef !== null) {
+          extraWarning =
+            ` Mutable values like '${badRef}' aren't valid dependencies ` +
+            "because their mutation doesn't re-render a component.";
+        }
+      }
+
       context.report({
         node: declaredDependenciesNode,
         message:
@@ -389,7 +408,8 @@ export default {
           // To avoid a long message, show the next actionable item.
           (list(missingDependencies, 'a', 'missing', 'include') ||
             list(unnecessaryDependencies, 'an', 'unnecessary', 'exclude') ||
-            list(duplicateDependencies, 'a', 'duplicate', 'omit')),
+            list(duplicateDependencies, 'a', 'duplicate', 'omit')) +
+          extraWarning,
         fix(fixer) {
           // TODO: consider keeping the comments?
           return fixer.replaceText(