3

Following the best practices as described in this Stack Overflow post: Angular RxJS When Should I Unsubscribe From Subscription, consider this Angular component lifecycle hook:

private ngUnsubscribe: Subject<void> = new Subject();

ngOnInit() {
  this.fireAuth.authState
    .takeUntil(this.ngUnsubscribe)
    .mergeMap((user) => this.todoService.getUserTodos(user.uid))
    .takeUntil(this.ngUnsubscribe)
    .subscribe((todos) => this.userTodoArray = todos);
}

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

As authState() and mergeMap() both return Observables, I think I'm supposed to either

  1. Unsubscribe from both of them (as shown above) or
  2. Only call one of the takeUntil() operators, either from the outer Observable authState() or the inner Observable this.todoService.getUserTodos(user.uid).

Which method is correct to make sure all Observables are unsubscribed after the component is destroyed?

Stan
  • 476
  • 1
  • 5
  • 15
  • It's number 2: you only need one of the `takeUntil` operators and it doesn't matter which one you remove. – cartant Feb 21 '18 at 03:33
  • Thanks for your response. Any explanation or reference about why #2 is correct? – Stan Feb 21 '18 at 03:48
  • Whichever one you use will see the stream complete and the observables unsubscribed. If no one else offers a detailed explanation, I might add an answer later. – cartant Feb 21 '18 at 03:52
  • Ok, no prob. Appreciate it! – Stan Feb 21 '18 at 04:12
  • I came back to answer this and realised that my comments were wrong. It most likely does matter which you choose to remove, as the behaviours are different. – cartant Feb 22 '18 at 08:27

1 Answers1

16

You don't need both of the takeUntil operators. You only need one, but the behaviour will differ depending upon which one you remove.

If you use only the first takeUntil, like this:

this.fireAuth.authState
  .takeUntil(this.ngUnsubscribe)
  .mergeMap((user) => this.todoService.getUserTodos(user.uid))
  .subscribe((todos) => this.userTodoArray = todos);

When this.ngUnsubscribe emits a next notification, the takeUntil will unsubscribe from the this.fireAuth.authState source and will complete. As outlined in the Observable Contract, when an observable completes, its subscribers receive a complete notification and are automatically unsubscribed.

However, if a next notification was emitted from this.fireAuth.authState and the observable returned by getUserTodos in the mergeMap has not yet completed, the mergeMap implementation will not unsubscribe from the getUserTodos observable. And if the getUserTodos observable later emits a next notification, the notification will flow through to the subscribe.

If you use only the second takeUntil, like this:

this.fireAuth.authState
  .mergeMap((user) => this.todoService.getUserTodos(user.uid))
  .takeUntil(this.ngUnsubscribe)
  .subscribe((todos) => this.userTodoArray = todos);

When this.ngUnsubscribe emits a next notification, the takeUntil will unsubscribe from the mergeMap, which will unsubscribe from this.fireAuth.authState and from any returned getUserTodos observable.

So, after this.ngUnsubscribe emits, no notifications will flow through to the subscribe. It's likely that this is the behaviour you are looking for, so you should remove the first takeUntil operator.

cartant
  • 57,105
  • 17
  • 163
  • 197