1

I have a question about unsubscribing Outputs in Angular. I know that EventEmitter gets cleaned up automatically, but the last time I needed use Observable as my Output. I mean, I wanted take Output which emit maximum once event per second. So my code looked like:

@Output() loadNextEvent: Observable<any>;
loadNextSubject = new Subject();

constructor(private el: ElementRef) {
    this.loadNextEvent = this.loadNextSubject.asObservable()
        .throttleTime(1000); // once per second
    }

emit() {
    this.loadNextSubject.next('new event');
}

Ok, it works pretty fine - I thought. The only problem is unsubscribing. I found one solution on StackOverflow, but I'm still not sure, how I should do it correctly. In my case:

@Output() loadNextEvent: Observable<any>;
loadNextSubject = new Subject();

constructor(private el: ElementRef) {
    this.loadNextEvent = this.loadNextSubject.asObservable()
        .takeUntil(this.loadNextSubject) //does it make any sense?
        .throttleTime(1000); // once per second
    }

emit() {
    this.loadNextSubject.next('new event');
}

ngOnDestroy() {
    this.loadNextSubject.complete();
}

Question: Is this the correct unsubscribing of observable Output? .takeUntil(this.loadNextSubject) make any sense? Or maybe .asObservable() give ensurance that Observable was cleaned, when Subject is completed? Anyone know answer for my problem? or maybe, there is a better solution, then use Observable as Output, for my case?

martin
  • 93,354
  • 25
  • 191
  • 226
Jaroslaw K.
  • 5,224
  • 2
  • 39
  • 65

1 Answers1

3

If you want to use takeUntil the notifier Observable needs to emit a next notification and not just complete. So the correct way to do this would be like this:

ngOnDestroy() {
    this.loadNextSubject.next();
    this.loadNextSubject.complete();
}

Also you don't need to use asObservable() in this case. Subjects are Observables so using asObservable() will just wrap it with another Observable which is unnecessary.

One more thing. In Angular 5 there's a bug that is creating memory leaks when chaining operators with EventEmitter (the chain wasn't properly disposed). For more details you can have a look here https://github.com/angular/angular/issues/21999.

This was fixed here https://github.com/angular/angular/pull/22016 and should be available since Angular 6.

And I think this is exactly what you're doing right now. An easy way to avoid this until Angular 6 is actually using asObservable() before adding any operators (see this comment https://github.com/angular/angular/issues/21999#issuecomment-362921475).

martin
  • 93,354
  • 25
  • 191
  • 226
  • so if I will not use `.takeUntil(this.loadNextSubject)`, can I add only `this.loadNextSubject.complete();` in `ngOnDestroy`? Or `.takeUntil(this.loadNextSubject)` is required for this structure and I have to add `this.loadNextSubject.next();` too (before .complete). – Jaroslaw K. Mar 15 '18 at 14:19
  • 1
    If you want to use `.takeUntil(this.loadNextSubject)` you have to `this.loadNextSubject.next();`. Without it it wont work. – martin Mar 15 '18 at 14:23