[ESLint] Tweak setState updater message and add useEffect(async) warning (#15055)

gaearon · web-flow · commit 03ad9c73e468 · 2019-03-07T19:40:23.000Z
* Use first letter in setCount(c =&gt; ...) suggestion

In-person testing showed using original variable name confuses people.

* Warn about async effects
diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
@@ -949,6 +949,29 @@ const tests = {
         }
       `,
     },
+    {
+      code: `
+        function App() {
+          const [query, setQuery] = useState('react');
+          const [state, setState] = useState(null);
+          useEffect(() => {
+            let ignore = false;
+            fetchSomething();
+            async function fetchSomething() {
+              const result = await (await fetch('http://hn.algolia.com/api/v1/search?query=' + query)).json();
+              if (!ignore) setState(result);
+            }
+            return () => { ignore = true; };
+          }, [query]);
+          return (
+            <>
+              <input value={query} onChange={e => setQuery(e.target.value)} />
+              {JSON.stringify(state)}
+            </>
+          );
+        }
+      `,
+    },
   ],
   invalid: [
     {
@@ -2304,7 +2327,7 @@ const tests = {
       errors: [
         "React Hook useEffect has a missing dependency: 'state'. " +
           'Either include it or remove the dependency array. ' +
-          `You can also write 'setState(state => ...)' ` +
+          `You can also write 'setState(s => ...)' ` +
           `if you only use 'state' for the 'setState' call.`,
       ],
     },
@@ -2335,7 +2358,7 @@ const tests = {
       errors: [
         "React Hook useEffect has a missing dependency: 'state'. " +
           'Either include it or remove the dependency array. ' +
-          `You can also write 'setState(state => ...)' ` +
+          `You can also write 'setState(s => ...)' ` +
           `if you only use 'state' for the 'setState' call.`,
       ],
     },
@@ -3933,7 +3956,7 @@ const tests = {
       errors: [
         "React Hook useEffect has a missing dependency: 'count'. " +
           'Either include it or remove the dependency array. ' +
-          `You can also write 'setCount(count => ...)' if you ` +
+          `You can also write 'setCount(c => ...)' if you ` +
           `only use 'count' for the 'setCount' call.`,
       ],
     },
@@ -3971,7 +3994,7 @@ const tests = {
       errors: [
         "React Hook useEffect has missing dependencies: 'count' and 'increment'. " +
           'Either include them or remove the dependency array. ' +
-          `You can also write 'setCount(count => ...)' if you ` +
+          `You can also write 'setCount(c => ...)' if you ` +
           `only use 'count' for the 'setCount' call.`,
       ],
     },
@@ -4379,6 +4402,35 @@ const tests = {
           `Either exclude it or remove the dependency array.`,
       ],
     },
+    {
+      code: `
+        function Thing() {
+          useEffect(async () => {}, []);
+        }
+      `,
+      output: `
+        function Thing() {
+          useEffect(async () => {}, []);
+        }
+      `,
+      errors: [
+        `Effect callbacks are synchronous to prevent race conditions. ` +
+          `Put the async function inside:\n\n` +
+          `useEffect(() => {\n` +
+          `  let ignore = false;\n` +
+          `  fetchSomething();\n` +
+          `\n` +
+          `  async function fetchSomething() {\n` +
+          `    const result = await ...\n` +
+          `    if (!ignore) setState(result);\n` +
+          `  }\n` +
+          `\n` +
+          `  return () => { ignore = true; };\n` +
+          `}, []);\n` +
+          `\n` +
+          `This lets you handle multiple requests without bugs.`,
+      ],
+    },
   ],
 };
 
diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
@@ -106,6 +106,28 @@ export default {
         return;
       }
 
+      if (isEffect && node.async) {
+        context.report({
+          node: node,
+          message:
+            `Effect callbacks are synchronous to prevent race conditions. ` +
+            `Put the async function inside:\n\n` +
+            `useEffect(() => {\n` +
+            `  let ignore = false;\n` +
+            `  fetchSomething();\n` +
+            `\n` +
+            `  async function fetchSomething() {\n` +
+            `    const result = await ...\n` +
+            `    if (!ignore) setState(result);\n` +
+            `  }\n` +
+            `\n` +
+            `  return () => { ignore = true; };\n` +
+            `}, []);\n` +
+            `\n` +
+            `This lets you handle multiple requests without bugs.`,
+        });
+      }
+
       // Get the current scope.
       const scope = context.getScope();
 
@@ -890,9 +912,12 @@ export default {
               break;
             case 'updater':
               extraWarning =
-                ` You can also write '${setStateRecommendation.setter}(${
-                  setStateRecommendation.missingDep
-                } => ...)' if you only use '${
+                ` You can also write '${
+                  setStateRecommendation.setter
+                }(${setStateRecommendation.missingDep.substring(
+                  0,
+                  1,
+                )} => ...)' if you only use '${
                   setStateRecommendation.missingDep
                 }'` + ` for the '${setStateRecommendation.setter}' call.`;
               break;
