Alphabetize the autofix

facebook · Feb 20, 2019 · 0c722bb · 0c722bb
1 parent 6fd2d12
commit 0c722bb
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 11 deletions.
diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleReactiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleReactiveDeps-test.js
@@ -174,6 +174,17 @@ const tests = {
       }
     `,
     },
+    {
+      // TODO: we might want to forbid dot-access in deps.
+      code: `
+      function MyComponent(props) {
+        useEffect(() => {
+          console.log(props.foo);
+          console.log(props.bar);
+        }, [props.bar, props.foo]);
+      }
+    `,
+    },
     {
       // TODO: we might want to forbid dot-access in deps.
       code: `
@@ -991,11 +1002,34 @@ const tests = {
           useEffect(() => {
             console.log(props.foo);
             console.log(props.bar);
-          }, [props.foo, props.bar]);
+          }, [props.bar, props.foo]);
+        }
+      `,
+      errors: [
+        "React Hook useEffect has missing dependencies: 'props.bar' and 'props.foo'. " +
+          'Either include them or remove the dependency array.',
+      ],
+    },
+    {
+      code: `
+        function MyComponent(props) {
+          let a, b, c, d, e, f, g;
+          useEffect(() => {
+            console.log(b, e, d, c, a, g, f);
+          }, [c, a, g]);
+        }
+      `,
+      // Alphabetize during the autofix.
+      output: `
+        function MyComponent(props) {
+          let a, b, c, d, e, f, g;
+          useEffect(() => {
+            console.log(b, e, d, c, a, g, f);
+          }, [a, b, c, d, e, f, g]);
         }
       `,
       errors: [
-        "React Hook useEffect has missing dependencies: 'props.foo' and 'props.bar'. " +
+        "React Hook useEffect has missing dependencies: 'b', 'd', 'e', and 'f'. " +
           'Either include them or remove the dependency array.',
       ],
     },
@@ -1019,11 +1053,11 @@ const tests = {
             console.log(props.foo);
             console.log(props.bar);
             console.log(local);
-          }, [props.foo, props.bar, local]);
+          }, [local, props.bar, props.foo]);
         }
       `,
       errors: [
-        "React Hook useEffect has missing dependencies: 'props.foo', 'props.bar', and 'local'. " +
+        "React Hook useEffect has missing dependencies: 'local', 'props.bar', and 'props.foo'. " +
           'Either include them or remove the dependency array.',
       ],
     },
@@ -1045,7 +1079,7 @@ const tests = {
             console.log(props.foo);
             console.log(props.bar);
             console.log(local);
-          }, [props, local]);
+          }, [local, props]);
         }
       `,
       errors: [
@@ -1260,11 +1294,11 @@ const tests = {
             console.log(ref2.current.textContent);
             alert(props.someOtherRefs.current.innerHTML);
             fetch(props.color);
-          }, [ref1, ref2, props.someOtherRefs, props.color]);
+          }, [props.color, props.someOtherRefs, ref1, ref2]);
         }
       `,
       errors: [
-        "React Hook useEffect has missing dependencies: 'props.someOtherRefs' and 'props.color'. " +
+        "React Hook useEffect has missing dependencies: 'props.color' and 'props.someOtherRefs'. " +
           'Either include them or remove the dependency array.',
       ],
     },
@@ -1290,7 +1324,7 @@ const tests = {
             console.log(ref2.current.textContent);
             alert(props.someOtherRefs.current.innerHTML);
             fetch(props.color);
-          }, [props.someOtherRefs, props.color, ref1, ref2]);
+          }, [props.color, props.someOtherRefs, ref1, ref2]);
         }
       `,
       errors: [
@@ -1365,12 +1399,12 @@ const tests = {
             if (props.onChange) {
               props.onChange();
             }
-          }, [props.onChange, props]);
+          }, [props, props.onChange]);
         }
       `,
       errors: [
         // TODO: reporting props separately is superfluous. Fix to just props.onChange.
-        "React Hook useEffect has missing dependencies: 'props.onChange' and 'props'. " +
+        "React Hook useEffect has missing dependencies: 'props' and 'props.onChange'. " +
           'Either include them or remove the dependency array.',
       ],
     },

diff --git a/packages/eslint-plugin-react-hooks/src/ReactiveDeps.js b/packages/eslint-plugin-react-hooks/src/ReactiveDeps.js
@@ -341,6 +341,8 @@ export default {
           // Already did that. Do nothing.
         }
       });
+      // Alphabetize for the autofix.
+      suggestedDependencies.sort();
 
       const problemCount =
         duplicateDependencies.size +
@@ -376,7 +378,11 @@ export default {
           ' ' +
           (set.size > 1 ? 'dependencies' : 'dependency') +
           ': ' +
-          join(Array.from(set).map(quote)) +
+          join(
+            Array.from(set)
+              .sort()
+              .map(quote),
+          ) +
           `. Either ${fixVerb} ${
             set.size > 1 ? 'them' : 'it'
           } or remove the dependency array.`