diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleReactiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleReactiveDeps-test.js index cea8e472817c..6c08930778aa 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleReactiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleReactiveDeps-test.js @@ -1019,7 +1019,30 @@ const tests = { }, [c, a, g]); } `, - // Alphabetize during the autofix. + // Don't alphabetize if it wasn't alphabetized in the first place. + output: ` + function MyComponent(props) { + let a, b, c, d, e, f, g; + useEffect(() => { + console.log(b, e, d, c, a, g, f); + }, [c, a, g, b, e, d, f]); + } + `, + errors: [ + "React Hook useEffect has missing dependencies: 'b', 'd', 'e', and 'f'. " + + '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); + }, [a, c, g]); + } + `, + // Alphabetize if it was alphabetized. output: ` function MyComponent(props) { let a, b, c, d, e, f, g; @@ -1033,6 +1056,29 @@ const tests = { '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); + }, []); + } + `, + // Alphabetize if it was empty. + 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: 'a', 'b', 'c', 'd', 'e', 'f', and 'g'. " + + 'Either include them or remove the dependency array.', + ], + }, { code: ` function MyComponent(props) { @@ -1324,7 +1370,7 @@ const tests = { console.log(ref2.current.textContent); alert(props.someOtherRefs.current.innerHTML); fetch(props.color); - }, [props.color, props.someOtherRefs, ref1, ref2]); + }, [props.someOtherRefs, props.color, ref1, ref2]); } `, errors: [ diff --git a/packages/eslint-plugin-react-hooks/src/ReactiveDeps.js b/packages/eslint-plugin-react-hooks/src/ReactiveDeps.js index ef01c86097ff..b947ca7661a3 100644 --- a/packages/eslint-plugin-react-hooks/src/ReactiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ReactiveDeps.js @@ -341,8 +341,20 @@ export default { // Already did that. Do nothing. } }); - // Alphabetize for the autofix. - suggestedDependencies.sort(); + + function areDeclaredDepsAlphabetized() { + if (declaredDependencies.length === 0) { + return true; + } + const declaredDepKeys = declaredDependencies.map(dep => dep.key); + const sortedDeclaredDepKeys = declaredDepKeys.slice().sort(); + return declaredDepKeys.join(',') === sortedDeclaredDepKeys.join(','); + } + + if (areDeclaredDepsAlphabetized()) { + // Alphabetize the autofix, but only if deps were already alphabetized. + suggestedDependencies.sort(); + } const problemCount = duplicateDependencies.size +