Fix ExhaustiveDeps ESLint rule throwing with optional chaining (#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 #19243
Joe Lencioni committed
Jul 6, 2020 at 14:52 UTC
0f84b0f02b5579d780a9f54497007c4c84aaebb7
2 files changed
+41
-3
packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
+38
@@ -6628,6 +6628,44 @@ const testsTypescript = {
6628
},
6629
],
6630
},
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
+ },
6669
],
6670
};
6671
packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
+3
-3
@@ -1016,11 +1016,11 @@ export default {
1016
// Is this a variable from top scope?
1017
const topScopeRef = componentScope.set.get(missingDep);
1018
const usedDep = dependencies.get(missingDep);
1019
- if (usedDep.references[0].resolved !== topScopeRef) {
1019
+ if (usedDep && usedDep.references[0].resolved !== topScopeRef) {
1020
return;
1021
}
1022
// Is this a destructured prop?
1023
- const def = topScopeRef.defs[0];
1023
+ const def = topScopeRef && topScopeRef.defs[0];
1024
if (def == null || def.name == null || def.type !== 'Parameter') {
1025
return;
1026
}
@@ -1062,7 +1062,7 @@ export default {
1062
return;
1063
}
1064
const usedDep = dependencies.get(missingDep);
1065
- const references = usedDep.references;
1065
+ const references = usedDep ? usedDep.references : [];
1066
let id;
1067
let maybeCall;
1068
for (let i = 0; i < references.length; i++) {