-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Enhancement: [prefer-optional-chain] Also rewrite if (x) x.something()
#6309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
This would be a great addition - it's a really common pattern that could be simplified. Though I can see this being an option - I can see some people might prefer the more verbose form. Though it's worth noting that this would be non-trivial to build and handle all the cases. |
What about multiple statements? if (foo) {
foo.bar();
foo.baz();
} It could be simplified but some would see it as "non-performant". |
I think that it'd make sense as an auto fix of a single statement, but not multiple. If anything, if I see this I'd want it fixed in the opposite direction (separate rule): o?.foo()
o?.bar()
o?.baz() |
It's not technically safe to auto-fix multiple statements. An earlier statement might modify the value being optionally chained.
One shudders to imagine the use case for wanting to preserve the thrown error, but it does technically change code behavior. |
See also: ill-behaved getters 😁 I agree with not rewriting multistatement optional chains, but, I think that in general we should follow TS's lead in control flow modelling, where a function call, or, god forbid, a member access, does not invalidate narrowing. |
Before You File a Proposal Please Confirm You Have Done The Following...
My proposal is suitable for this project
Link to the rule's documentation
https://typescript-eslint.io/rules/prefer-optional-chain/
Description
The rule apparently only tries to prevent
a && a.b
, but another way to write that isif (a) a.b
Could the rule be updated to include
if
conditions?Fail
Pass
Additional Info
No response
The text was updated successfully, but these errors were encountered: