[ESLint] Tweak setState updater message and add useEffect(async) warning (#15055)
* Use first letter in setCount(c => ...) suggestion In-person testing showed using original variable name confuses people. * Warn about async effects
Dan Abramov committed
Mar 7, 2019 at 19:40 UTC
03ad9c73e468cafa9cafbd9a51a0c3a16ed3f362
2 files changed
+84
-7
packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
+56
-4
@@ -949,6 +949,29 @@ const tests = {
949
}
950
`,
951
},
952
+ {
953
+ code: `
954
+ function App() {
955
+ const [query, setQuery] = useState('react');
956
+ const [state, setState] = useState(null);
957
+ useEffect(() => {
958
+ let ignore = false;
959
+ fetchSomething();
960
+ async function fetchSomething() {
961
+ const result = await (await fetch('http://hn.algolia.com/api/v1/search?query=' + query)).json();
962
+ if (!ignore) setState(result);
963
+ }
964
+ return () => { ignore = true; };
965
+ }, [query]);
966
+ return (
967
+ <>
968
+ <input value={query} onChange={e => setQuery(e.target.value)} />
969
+ {JSON.stringify(state)}
970
+ </>
971
+ );
972
+ }
973
+ `,
974
+ },
975
],
976
invalid: [
977
{
@@ -2304,7 +2327,7 @@ const tests = {
2327
errors: [
2328
"React Hook useEffect has a missing dependency: 'state'. " +
2329
'Either include it or remove the dependency array. ' +
2307
- `You can also write 'setState(state => ...)' ` +
2330
+ `You can also write 'setState(s => ...)' ` +
2331
`if you only use 'state' for the 'setState' call.`,
2332
],
2333
},
@@ -2335,7 +2358,7 @@ const tests = {
2358
errors: [
2359
"React Hook useEffect has a missing dependency: 'state'. " +
2360
'Either include it or remove the dependency array. ' +
2338
- `You can also write 'setState(state => ...)' ` +
2361
+ `You can also write 'setState(s => ...)' ` +
2362
`if you only use 'state' for the 'setState' call.`,
2363
],
2364
},
@@ -3933,7 +3956,7 @@ const tests = {
3956
errors: [
3957
"React Hook useEffect has a missing dependency: 'count'. " +
3958
'Either include it or remove the dependency array. ' +
3936
- `You can also write 'setCount(count => ...)' if you ` +
3959
+ `You can also write 'setCount(c => ...)' if you ` +
3960
`only use 'count' for the 'setCount' call.`,
3961
],
3962
},
@@ -3971,7 +3994,7 @@ const tests = {
3994
errors: [
3995
"React Hook useEffect has missing dependencies: 'count' and 'increment'. " +
3996
'Either include them or remove the dependency array. ' +
3974
- `You can also write 'setCount(count => ...)' if you ` +
3997
+ `You can also write 'setCount(c => ...)' if you ` +
3998
`only use 'count' for the 'setCount' call.`,
3999
],
4000
},
@@ -4379,6 +4402,35 @@ const tests = {
4402
`Either exclude it or remove the dependency array.`,
4403
],
4404
},
4405
+ {
4406
+ code: `
4407
+ function Thing() {
4408
+ useEffect(async () => {}, []);
4409
+ }
4410
+ `,
4411
+ output: `
4412
+ function Thing() {
4413
+ useEffect(async () => {}, []);
4414
+ }
4415
+ `,
4416
+ errors: [
4417
+ `Effect callbacks are synchronous to prevent race conditions. ` +
4418
+ `Put the async function inside:\n\n` +
4419
+ `useEffect(() => {\n` +
4420
+ ` let ignore = false;\n` +
4421
+ ` fetchSomething();\n` +
4422
+ `\n` +
4423
+ ` async function fetchSomething() {\n` +
4424
+ ` const result = await ...\n` +
4425
+ ` if (!ignore) setState(result);\n` +
4426
+ ` }\n` +
4427
+ `\n` +
4428
+ ` return () => { ignore = true; };\n` +
4429
+ `}, []);\n` +
4430
+ `\n` +
4431
+ `This lets you handle multiple requests without bugs.`,
4432
+ ],
4433
+ },
4434
],
4435
};
4436
packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
+28
-3
@@ -106,6 +106,28 @@ export default {
106
return;
107
}
108
109
+ if (isEffect && node.async) {
110
+ context.report({
111
+ node: node,
112
+ message:
113
+ `Effect callbacks are synchronous to prevent race conditions. ` +
114
+ `Put the async function inside:\n\n` +
115
+ `useEffect(() => {\n` +
116
+ ` let ignore = false;\n` +
117
+ ` fetchSomething();\n` +
118
+ `\n` +
119
+ ` async function fetchSomething() {\n` +
120
+ ` const result = await ...\n` +
121
+ ` if (!ignore) setState(result);\n` +
122
+ ` }\n` +
123
+ `\n` +
124
+ ` return () => { ignore = true; };\n` +
125
+ `}, []);\n` +
126
+ `\n` +
127
+ `This lets you handle multiple requests without bugs.`,
128
+ });
129
+ }
130
+
131
// Get the current scope.
132
const scope = context.getScope();
133
@@ -890,9 +912,12 @@ export default {
912
break;
913
case 'updater':
914
extraWarning =
893
- ` You can also write '${setStateRecommendation.setter}(${
894
- setStateRecommendation.missingDep
895
- } => ...)' if you only use '${
915
+ ` You can also write '${
916
+ setStateRecommendation.setter
917
+ }(${setStateRecommendation.missingDep.substring(
918
+ 0,
919
+ 1,
920
+ )} => ...)' if you only use '${
921
setStateRecommendation.missingDep
922
}'` + ` for the '${setStateRecommendation.setter}' call.`;
923
break;