Skip to content
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

[prefer-optional-chain] include mixed "nullish comparison style" chains in checks #7170

Open
bradzacher opened this issue Jul 7, 2023 · 0 comments
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@bradzacher
Copy link
Member

#6397 (comment)

In v6 we have rewritten the prefer-optional-chain rule from the ground up.
The new logic assesses the "nullish comparison style" of logical operand in relation to the logical operator to determine if it should be consider part of a chain. EG || will only assess "equal nullish" checks, and && will only assess "not equal to nullish" checks.

This does lead to one hole in the rule:

declare const foo: { bar: number } | null;
foo === null || foo.bar;
foo !== null && !foo.bar;

In the first case foo.bar is seen as a "truthy boolean" check (eg "not equal nullish") - which isn't a valid operand in an "OR" chain.
Similarly in the second case !foo.bar is seen as a "falsy boolean" check (eg "equal nullish") - which isn't a valid operand in an "AND" chain.

This means that the rule will ignore it and thus will not include it in the chain to report and fix.

This logic is correct and sound - however it doesn't properly encapsulate developer intent. Specifically when that "invalid" chain operand is the very last member in a chain - in certain cases we will actually want to include it as part of the chain.

A slightly longer example:

foo && foo.bar && foo.bar.baz === 1
// rule will currently fix to
foo?.bar && foo.bar.baz === 1
// but ideally it would fix it to
foo?.bar?.baz === 1

I think there's a subset of checks that it should include:

  • "boolean" checks (eg foo and !foo)
  • !==/===/!=/== comparing against anything except typeof
    • not typeof because foo && typeof foo.bar will return undefined | typeof foo.bar whereas typeof foo?.bar will return typeof foo.bar | 'undefined' - which is VERY different
  • should not include >/>=/</<= because TS will hard error if one of the operands is nullish.
@bradzacher bradzacher added enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin accepting prs Go ahead, send a pull request that resolves this issue labels Jul 7, 2023
@bradzacher bradzacher changed the title [prefer-optional-chain] include "mixed truthiness" chains in checks [prefer-optional-chain] include mixed "nullish comparison style" chains in checks Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

1 participant