Skip to content

Commit 0f84b0f

Browse files
authored
Fix ExhaustiveDeps ESLint rule throwing with optional chaining (facebook#19260)
Certain code patterns using optional chaining syntax causes eslint-plugin-react-hooks to throw an error. We can avoid the throw by adding some guards. I didn't read through the code to understand how it works, I just added a guard to every place where it threw, so maybe there is a better fix closer to the root cause than what I have here. In my test case, I noticed that the optional chaining that was used in the code was not included in the suggestions description or output, but it seems like it should be. This might make a nice future improvement on top of this fix, so I left a TODO comment to that effect. Fixes facebook#19243
1 parent 670c037 commit 0f84b0f

File tree

2 files changed

+41
-3
lines changed

2 files changed

+41
-3
lines changed

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

+38
Original file line numberDiff line numberDiff line change
@@ -6628,6 +6628,44 @@ const testsTypescript = {
66286628
},
66296629
],
66306630
},
6631+
{
6632+
// https://github.com/facebook/react/issues/19243
6633+
code: normalizeIndent`
6634+
function MyComponent() {
6635+
const pizza = {};
6636+
6637+
useEffect(() => ({
6638+
crust: pizza.crust,
6639+
toppings: pizza?.toppings,
6640+
}), []);
6641+
}
6642+
`,
6643+
errors: [
6644+
{
6645+
message:
6646+
"React Hook useEffect has missing dependencies: 'pizza.crust' and 'pizza.toppings'. " +
6647+
'Either include them or remove the dependency array.',
6648+
suggestions: [
6649+
{
6650+
// TODO the description and suggestions should probably also
6651+
// preserve the optional chaining.
6652+
desc:
6653+
'Update the dependencies array to be: [pizza.crust, pizza.toppings]',
6654+
output: normalizeIndent`
6655+
function MyComponent() {
6656+
const pizza = {};
6657+
6658+
useEffect(() => ({
6659+
crust: pizza.crust,
6660+
toppings: pizza?.toppings,
6661+
}), [pizza.crust, pizza.toppings]);
6662+
}
6663+
`,
6664+
},
6665+
],
6666+
},
6667+
],
6668+
},
66316669
],
66326670
};
66336671

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -1016,11 +1016,11 @@ export default {
10161016
// Is this a variable from top scope?
10171017
const topScopeRef = componentScope.set.get(missingDep);
10181018
const usedDep = dependencies.get(missingDep);
1019-
if (usedDep.references[0].resolved !== topScopeRef) {
1019+
if (usedDep && usedDep.references[0].resolved !== topScopeRef) {
10201020
return;
10211021
}
10221022
// Is this a destructured prop?
1023-
const def = topScopeRef.defs[0];
1023+
const def = topScopeRef && topScopeRef.defs[0];
10241024
if (def == null || def.name == null || def.type !== 'Parameter') {
10251025
return;
10261026
}
@@ -1062,7 +1062,7 @@ export default {
10621062
return;
10631063
}
10641064
const usedDep = dependencies.get(missingDep);
1065-
const references = usedDep.references;
1065+
const references = usedDep ? usedDep.references : [];
10661066
let id;
10671067
let maybeCall;
10681068
for (let i = 0; i < references.length; i++) {

0 commit comments

Comments
 (0)