`ngOnDestroy` is not implemented raised by prefer-takeuntil rule when ngOnDestroy is in a super class

[ldulary]

Considering this super class :

import { Directive, OnDestroy } from '@angular/core';
import { Subject } from 'rxjs';

@Directive()
export class Destroy implements OnDestroy {
    protected destroyed$ = new Subject();

    public ngOnDestroy() {
        this.destroyed$.next();
        this.destroyed$.unsubscribe();
    }
}

In the components extending Destroy, having takeUntil(this.destroyed$) before each subscribe should be enough to fullfill prefer-takeuntil rule. But there is the following issue : error `ngOnDestroy` is not implemented rxjs-angular/prefer-takeuntil

[cartant]

See this comment on the TSLint rule equivalent:

I imagine this would be possible, but complicated and I can't see myself having the time to do this anytime soon.

[ldulary]

Thanks for your answer. Meanwhile I will just set checkDestroy to false.

[silveoj]

It's safer use

// eslint-disable-next-line rxjs-angular/prefer-takeuntil
export class MyComponent ...

or

  // eslint-disable-next-line rxjs-angular/prefer-takeuntil
  public ngOnDestroy() {
    super.ngOnDestroy();
  }

[SamuelT-Beslogic]

See this comment on the TSLint rule equivalent:

I imagine this would be possible, but complicated and I can't see myself having the time to do this anytime soon.

For both prefer-composition and prefer-takeuntil, May I suggest configuration to ignore if extended from a certain class? (and ngOnDestroy is not reimplemented in the child class). In my case I'd configure it for "BaseComponent". The rules would still naturally enforce ngOnDestroy and takeUntil for the BaseComponent anyway (since it doesn't extend itself).

@Component({
  selector: 'app-footer',
  templateUrl: './footer.component.html',
  styleUrls: ['./footer.component.less'],
})
export class FooterComponent extends BaseComponent implements OnInit {
  // Some code
}

Or even simpler as an easy / naive way to add support: let us disable the ngOnDestroy is not implemented message, while keeping the Forbids calling subscribe without an accompanying takeUntil one.

[cartant]

@Samuel-Beslogic I just don't have the time to mess with these rules and I've not used them myself lately - as I've not used Angular for years. Rather than messing with them to work with base-class/component shenanigans, maybe the plain vanilla RxJS rules would be a better fit:

• https://github.com/cartant/eslint-plugin-rxjs/blob/main/docs/rules/no-ignored-subscription.md
• https://github.com/cartant/eslint-plugin-rxjs/blob/main/docs/rules/no-unsafe-takeuntil.md

[Avasam]

@cartant Part of your rule does something important that neither of those two do: it ensures takeuntil used on subscriptions should properly unsubscribe when the component is destroyed.

It's just that there's part of the rule that wrongly assumes all components need to implement OnDestroy by themselves.

Even just splitting the takeUntil rule in two (or being able to disable the check for implementing OnDestroy) would be sufficient. Forgetting takeUntil happens often. But when typing takeUntil(this.unsubscribe) it's not hard to see that I just need to extend our BaseComponent if Typescript tells me that this.unsubscribe does not exist.

I'm willing to spend some time on this and create a PR if I have a working solution. The AST Selector for the extend class would look something like this ClassDeclaration[superClass.name='${baseComponent}']

[Avasam]

@ldulary #10 You can install github:Avasam/eslint-plugin-rxjs-angular#dist in the mean time with the following configuration:

"rxjs-angular/prefer-composition": [
  "error",
  {
    "superClass": [
      "BaseComponent"
    ]
  }
],
"rxjs-angular/prefer-takeuntil": [
  "error",
  {
    "superClass": [
      "BaseComponent"
    ]
  }
],