Support Angular's takeUntilDestroyed operator

[dkimmich-onventis]

Angular 16 introduces a new takeUntilDestroyed() operator for destroying subscriptions, see their v16 blog post. The prefer-takeuntil rule should by default not raise a lint error when this operator is used on a subscription.

In the meantime we can add this operator to the alias option:

'rxjs-angular/prefer-takeuntil': [
  'error',
  {
    alias: ['takeUntilDestroyed'],
  },
],

[MCFreddie777]

+1 to this.

Also, there's no need for ngOnDestroy on component which uses angular's destroyRef, but the lint rule still gives "ngOnDestroy" is not implemented warning.

@cartant @Michsior14 @LayZeeDK

[jmeinlschmidt]

@MCFreddie777

not perfect, but at least some something:

'rxjs-angular/prefer-takeuntil': [
  'warn',
  {
    alias: ['takeUntilDestroyed'],
    checkDestroy: false,
    checkComplete: false,
  }
],

[negberts]

For me the alias is not working....

"rxjs-angular/prefer-takeuntil": [
    "error",
    {
        "alias": ['takeUntilDestroyed'],
        "checkComplete": true,
        "checkDecorators": ["Directive", "Component"],
        "checkDestroy": true
    }
]

But still getting errors:

132:155 error Forbids calling subscribe without an accompanying takeUntil rxjs-angular/prefer-takeuntil
136:155 error Forbids calling subscribe without an accompanying takeUntil rxjs-angular/prefer-takeuntil
180:70 error Forbids calling subscribe without an accompanying takeUntil rxjs-angular/prefer-takeuntil

[rgant]

@negberts without code that causes the issue it is hard to diagnose. One thing to check is that the takeUntilDestroyed operator is called in the same operation as the .subscribe(.

Here is my config working with eslint-plugin-rxjs-angular@2.0.1

        "rxjs-angular/prefer-takeuntil": [
          "error",
          {
            "alias": ["takeUntilDestroyed"],
            "checkComplete": false, // Until https://github.com/cartant/eslint-plugin-rxjs-angular/issues/16 is implemented
            "checkDecorators": [ "Component", "Directive", "Pipe", "Service" ],
            "checkDestroy": false
          }
        ],

[negberts]

@rgant

this.stamgegevensService.getNominaleKorreldiameters()
    .pipe(takeUntilDestroyed()).subscribe((nominaleKorreldiameters: NominaleKorreldiameter[]) => {
        this.bestekspostLijstVM.NominaleKorrelDiameterWaardes = nominaleKorreldiameters;
    });

And the getNominaleKorreldiameters method is like this in the service (doing a HTTP call)

getNominaleKorreldiameters(): Observable<NominaleKorreldiameter[]> {
    const url = `${this.baseUrl}/stamgegevens/nominalekorreldiameters`;
    return this.http.get<NominaleKorreldiameter[]>(url)
        .pipe(map(res => res), catchError((error: any) => this.handleError(error, 'Het ophalen van de nominale korreldiameters is mislukt.')));
}

[rgant]

Make sure you are on the version of eslint-plugin-rxjs-angular that supports the alias.

IIRC, you have to turn off "checkComplete" and "checkDestroy" for this to work, but that is after this issue is resolved.

And finally, you are getting data out of an Observable in Angular. You should not be using takeUntilDestroyed & .subscribe(, you should be using the AsyncPipe. Its a framework, generally best to let the framework handle the boilerplate code IMPO. If you care to see my advice on how to use the AsyncPipe you can email me.

[negberts]

@rgant we're using the latest version. I am aware of the AsyncPipe but that would be a massive overhaul of the application.

[xDivisionByZerox]

Is this feature request under considerations from the maintainers? While the provided workaround works, having a separate rule would likely encourage people to adopt the takeUntilDestroyed() operator more readily, as it would be officially recognized by the linter. This could propably lead to more consistent and safer code practices within Angular applications of the community.

I would be willing to provide a PR for this feature, if that would help form a decision. Seeing the latest commit being over a year ago and other PRs not getting any attention, I first wanted to reach out if that is even desired from the maintainers (as well as the communities) side.

[skynetigor]

The alias workaround is not good for takeUntilDestroy because that way the plugin matches both takeUntil and takeUntilDestroy. So lint passes if your subscription contains either takeUntil or takeUntilDestroy that is not great. In case you forgot to use takeUntilDestroy and instead used takeUntil approach, lint rule stops protecting you because you might forget to call detach.next() in ngOnDestroy or forget to declare ngOnDestroy at all.

Out of topic:
Another issue I found, is that the plugin skips directives (maybe pipes as well) at all. It performs checks only in files that contain @component decorator.
Maybe I'm missing something and using it wrong? Because I'm about to open the issue.

[xDivisionByZerox]

Out of topic:
Another issue I found, is that the plugin skips directives (maybe pipes as well) at all. It performs checks only in files that contain https://github.com/component decorator.
Maybe I'm missing something and using it wrong? Because I'm about to open the issue.

From the rules description:

The checkDecorators property is an array containing the names of the decorators that determine whether or not a class is checked. By default, checkDecorators is ["Component"].

So yeah, you need to provide ["Component", "Directive", "Pipe"] yourself if you want them to be checked as well.
I think the documentation is pretty clear on that, TBH. But I can see this being an honest mistake to make 😄

[skynetigor]

Thanks for the response! Yes, really, I missed this in docs. Many thanks.