mirror of
https://github.com/hanzoai/react.git
synced 2026-09-24 17:02:27 +00:00
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
This commit is contained in:
@@ -6628,6 +6628,44 @@ const testsTypescript = {
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
// https://github.com/facebook/react/issues/19243
|
||||
code: normalizeIndent`
|
||||
function MyComponent() {
|
||||
const pizza = {};
|
||||
|
||||
useEffect(() => ({
|
||||
crust: pizza.crust,
|
||||
toppings: pizza?.toppings,
|
||||
}), []);
|
||||
}
|
||||
`,
|
||||
errors: [
|
||||
{
|
||||
message:
|
||||
"React Hook useEffect has missing dependencies: 'pizza.crust' and 'pizza.toppings'. " +
|
||||
'Either include them or remove the dependency array.',
|
||||
suggestions: [
|
||||
{
|
||||
// TODO the description and suggestions should probably also
|
||||
// preserve the optional chaining.
|
||||
desc:
|
||||
'Update the dependencies array to be: [pizza.crust, pizza.toppings]',
|
||||
output: normalizeIndent`
|
||||
function MyComponent() {
|
||||
const pizza = {};
|
||||
|
||||
useEffect(() => ({
|
||||
crust: pizza.crust,
|
||||
toppings: pizza?.toppings,
|
||||
}), [pizza.crust, pizza.toppings]);
|
||||
}
|
||||
`,
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
|
||||
@@ -1016,11 +1016,11 @@ export default {
|
||||
// Is this a variable from top scope?
|
||||
const topScopeRef = componentScope.set.get(missingDep);
|
||||
const usedDep = dependencies.get(missingDep);
|
||||
if (usedDep.references[0].resolved !== topScopeRef) {
|
||||
if (usedDep && usedDep.references[0].resolved !== topScopeRef) {
|
||||
return;
|
||||
}
|
||||
// Is this a destructured prop?
|
||||
const def = topScopeRef.defs[0];
|
||||
const def = topScopeRef && topScopeRef.defs[0];
|
||||
if (def == null || def.name == null || def.type !== 'Parameter') {
|
||||
return;
|
||||
}
|
||||
@@ -1062,7 +1062,7 @@ export default {
|
||||
return;
|
||||
}
|
||||
const usedDep = dependencies.get(missingDep);
|
||||
const references = usedDep.references;
|
||||
const references = usedDep ? usedDep.references : [];
|
||||
let id;
|
||||
let maybeCall;
|
||||
for (let i = 0; i < references.length; i++) {
|
||||
|
||||
Reference in New Issue
Block a user