Skip to content

Need an ESLint rule that restricts optional chaining with negation. #21984

Closed

Description

@sainthkh

Copied from #19931(comment)


I guess as of #19831 I could simplify this using optional chaining:

wrapperNode.current &&
wrapperNode.current.contains( document.activeElement )
// =>
wrapperNode.current?.contains( document.activeElement )

What's interesting to me is looking at the other, negated instance of this condition, I sense there's going to be a very common pitfall associated with optional chaining:

wrapperNode.current &&
! wrapperNode.current.contains( document.activeElement )

If someone treats optional chaining as a way to get to the value by any means necessary, it might be easy to overlook that negating would produce true if the optional chaining aborts early:

const wrapperNode = { current: undefined };
! wrapperNode.current?.contains( document.activeElement );
// true

😯 It's definitely not the expected result.

It seems like in the same category of issues we restrict with this ESLint rule:

https://eslint.org/docs/rules/no-unsafe-negation

I could imagine some improvement to that rule which forbids negation of optional chains.


! wrapperNode.current?.contains() is ! ( wrapperNode.current && wrapperNode.current.contains() ).

It's not wrapperNode.current && ! wrapperNode.current.contains().

We need an eslint rule to help us to not fall into this pitfall.

Activity

  1. self-assigned this
    on May 1, 2020
  2. aduth commented on May 13, 2020

    @aduth
    Member

    I proposed a new rule for this at #22041. I'd welcome feedback, but at this time I have a feeling there is likely going to be enough legitimate use-cases for the behavior that creating a rule will be counter-productive.

    Going to close the issue for this reason. It can be revisited in the future if continued usage surfaces new insight.

  3. sainthkh commented on May 14, 2020

    @sainthkh
    ContributorAuthor

    When I saw the real-world use cases you mentioned in #22041, I also felt that the warning in coding guideline would be sufficient.

    Thank you for spending time on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions