Skip to content

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

Open
@bradzacher

Description

@bradzacher

#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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    accepting prsGo ahead, send a pull request that resolves this issueenhancementNew feature or requestpackage: eslint-pluginIssues related to @typescript-eslint/eslint-plugin

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions