Not so opinionated analog of `rxjs-prefer-angular-takeuntil`, please

[anotherpit]

Not so opinionated analog of `rxjs-prefer-angular-takeuntil`, please

[cartant]

Yeah, that sounds pretty reasonable. 4.30.0 has been published and the rule now has alias and checkDestroy options.

[anotherpit]

@cartant

I've just checked out new version, and tried to run against my repo. Experiencing some issues. Even if checkDestroy is set to false the rule does still check subject and is still opinionated about operator signature. Let me adapt code from your tests to demonstrate:

// Alias accepts component instance, not destroy subject
const someAlias = (cmp) => takeUntil(cmp.destroy);

@Component({
  selector: "component-with-alias"
})
class CorrectComponent implements OnDestroy {
  private destroy = new Subject<void>();
  someMethod() {
    a.pipe(
      switchMap(_ => b),
      someAlias(this)
    ).subscribe();
  }
  ngOnDestroy() {
    this.destroy.next();
    this.destroy.complete();
  }
}

{
  "rules": {
    "rxjs-prefer-angular-takeuntil": {
      "options": [{
        "alias": ["someAlias"],
        "checkDestroy": false
      }]
    }
  }
}

My suggestion is to disable any semantic checks and any assumptions about operator's signature when checkDestroy is set to false.

[cartant]

@anotherpit This should be fixed in 4.30.1 and checkDestroy should default to false if an alias is specified.

[cartant]

@anotherpit Added a test with your snippet and config - https://github.com/cartant/rxjs-tslint-rules/tree/master/test/v6/fixtures/issues/115 - and AFAICT it passes, so I'm not sure what the issue is.

[anotherpit]

Well, it actually passes even without any takeUntil operator at all. I'm doing further investigations.

[anotherpit]

@cartant The test always passed because of missing declarations of a and b

Here's a fixture replacement to make it fail:

import { Component } from "@angular/core";
import { of, Subject } from "rxjs";
import { switchMap, takeUntil } from "rxjs/operators";

const a = of("a");
const b = of("b");

const someAlias = (cmp) => takeUntil(cmp.destroy);

@Component({
  selector: "component-with-alias"
})
class CorrectComponent implements OnDestroy {
  private destroy = new Subject<void>();
  someMethod() {
    a.pipe(
      switchMap(_ => b),
      someAlias(this)
    ).subscribe();
  }
  ngOnDestroy() {
    this.destroy.next();
    this.destroy.complete();
  }
}

[prefer-takeuntil]: Subscribing without takeUntil is forbidden

Would you mind reopening this issue?

[anotherpit]

And here's some extreme case that I suppose should also pass when checkDestroy is false:

import { Component } from "@angular/core";
import { of } from "rxjs";
import { switchMap } from "rxjs/operators";

const a = of("a");
const b = of("b");

const someAlias = (nevermind?: string, the?: boolean, signature?: number) => {}

@Component({
  selector: "component-with-alias"
})
class CorrectComponent {
  someMethod() {
    a.pipe(
      switchMap(_ => b),
      someAlias()
    ).subscribe();
  }
}

[prefer-takeuntil]: Subscribing without takeUntil is forbidden

[anotherpit]

@cartant I do confirm: now it works on my repo. Thanks.

[cartant]

@anotherpit No worries.