Bug: eslint(react-hooks/exhaustive-deps) When a property is accessed with and without optional chaining, exhaustive-deps' code suggestion will introduce an error #23248
Labels
Component: ESLint Rules
Status: Unconfirmed
A potential issue that we haven't yet confirmed as a bug
Type: Bug
Steps To Reproduce
StackBlitz demo
I've enabled
enableDangerousAutofixThisMayCauseInfiniteLoops
so you can runnpx eslint --fix index.js
and see the change thatreact-hooks/exhaustive-deps makes through the code suggestions API.
The current behavior
When you accept react-hooks/exhaustive-deps' code suggestions the following code:
Will be incorrectly fixed to the following.
The expected behavior
react-hooks/exhaustive-deps should be recommending the following:
Additional details
When you include both an optional chaining usage
foo?.bar
and a non-optional chaining usagefoo.bar
exhaustive-deps will use the version without the optional chaining for the deps array. i.e. It will recommend a deps array containingfoo.bar
, which becausefoo
may be undefined and the deps array is outside will result in a silent runtime error in JS or a TypeScript error in TS.If you only use
foo?.bar
it will recommendfoo?.bar
. So this may be an order of usage issue, i.e. preferring the last usage. I haven't checked. However that is a problem because the most common reason to access the same property with and without optional chaining is a case where you have an if condition which implicitly guarantees that the member you use optional chaining on is not nullish and can be accessed directly and thus do not need optional chaining within the if condition's body. And in that case the version without optional chaining will always be the latter usage.When you are already using
foo?.bar
will not try to autofix that. However this is made worse of an issue because if you are already using the correct version and you accept exhaustive-deps changes to fix a different issue (you added a new unrelated variable it needs to add) exhaustive-deps will override yourfoo?.bar
dep and turn it intofoo.bar
.If exhaustive-deps sees multiple versions of the same property access, it should recommend the version with the most optional chaining usage. Since anything less would create an error which would result in the optional chaining usage never even running.
The text was updated successfully, but these errors were encountered: