no-unsafe-takeuntil: Accept object properties as operators

[alwonder]

Motivation

I wanted to add the rule for the custom takeUntilDestroy operator in our working project. The operator is the method of an angular component, which fires when ngOnDestroy is called. But I stumbled upon the issue that the rule ignores my method:

// .eslintrc.json

"rxjs/no-unsafe-takeuntil": [
  "error",
  {
    "alias": ["takeUntilDestroy"]
  }
]

// My method

of(null)
  .pipe(
    this.takeUntilDestroy(), // It doesn't say anything
    tap(() => {}),
  )
  .subscribe(() => {});

Solution

I added the new option acceptObjectProperties (false by default) which allows adding not only functions but object properties and class members. So with this config:

// .eslintrc.json

"rxjs/no-unsafe-takeuntil": [
  "error",
  {
    "alias": ["takeUntilDestroy"],
    "acceptObjectProperties": true
  }
]

eslint would highlight the problem method:

// My method

of(null)
  .pipe(
    this.takeUntilDestroy(), // Applying operators after takeUntil is forbidden.
    tap(() => {}),
  )
  .subscribe(() => {});

[cartant]

I think we could make this change without introducing the acceptObjectProperties option. That is, checking for a call via a member expression would be sensible default behaviour - as calls made via namespaced imports are represented as member expressions.

ATM - i.e. without your change - this wouldn't behave the way it should:

import * as rxjs from "rxjs";
rxjs.of(null)
  .pipe(
    rxjs.takeUntil(rxjs.NEVER),
    rxjs.tap(() => {}),
  )
  .subscribe(() => {});

So your change is more of a general fix, I think.

[alwonder]

Yes, I think you're right. In that case, I'll delete the option.

[cartant]

Thanks.

[cartant]

This will match calls via member expressions that are outside of pipe, as the , separates the member-expression selector from the pipe selector.

If you're curious, you can verify that this is how the selectors behave using http://estools.github.io/esquery/ with:

of(null).pipe(foo())
x.bar()

as the source and

CallExpression[callee.property.name='pipe'] > CallExpression[callee.name='foo'], CallExpression[callee.property.name='bar']

as the selector.

I'm going to merge this and fix it with a subsequent commit. My preference, in these situations, is to have two separate selectors/properties calling shared function.

[cartant]

Your change should be in 4.0.4 - which has just been published.

[alwonder]

Great, thanks!