Skip to content

Commit 03ad9c7

Browse files
authored
[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
1 parent eb6247a commit 03ad9c7

File tree

2 files changed

+84
-7
lines changed

2 files changed

+84
-7
lines changed

packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -949,6 +949,29 @@ const tests = {
949949
}
950950
`,
951951
},
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+
},
952975
],
953976
invalid: [
954977
{
@@ -2304,7 +2327,7 @@ const tests = {
23042327
errors: [
23052328
"React Hook useEffect has a missing dependency: 'state'. " +
23062329
'Either include it or remove the dependency array. ' +
2307-
`You can also write 'setState(state => ...)' ` +
2330+
`You can also write 'setState(s => ...)' ` +
23082331
`if you only use 'state' for the 'setState' call.`,
23092332
],
23102333
},
@@ -2335,7 +2358,7 @@ const tests = {
23352358
errors: [
23362359
"React Hook useEffect has a missing dependency: 'state'. " +
23372360
'Either include it or remove the dependency array. ' +
2338-
`You can also write 'setState(state => ...)' ` +
2361+
`You can also write 'setState(s => ...)' ` +
23392362
`if you only use 'state' for the 'setState' call.`,
23402363
],
23412364
},
@@ -3933,7 +3956,7 @@ const tests = {
39333956
errors: [
39343957
"React Hook useEffect has a missing dependency: 'count'. " +
39353958
'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 ` +
39373960
`only use 'count' for the 'setCount' call.`,
39383961
],
39393962
},
@@ -3971,7 +3994,7 @@ const tests = {
39713994
errors: [
39723995
"React Hook useEffect has missing dependencies: 'count' and 'increment'. " +
39733996
'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 ` +
39753998
`only use 'count' for the 'setCount' call.`,
39763999
],
39774000
},
@@ -4379,6 +4402,35 @@ const tests = {
43794402
`Either exclude it or remove the dependency array.`,
43804403
],
43814404
},
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+
},
43824434
],
43834435
};
43844436

packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,28 @@ export default {
106106
return;
107107
}
108108

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+
109131
// Get the current scope.
110132
const scope = context.getScope();
111133

@@ -890,9 +912,12 @@ export default {
890912
break;
891913
case 'updater':
892914
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 '${
896921
setStateRecommendation.missingDep
897922
}'` + ` for the '${setStateRecommendation.setter}' call.`;
898923
break;

0 commit comments

Comments
 (0)