23

In an Angular app, I'm managing my subscriptions by pushing them all into an array, then looping through it and unsubscribing during ngOnDestroy.

private subscriptions: Subscription[] = []
this.subscriptions.push(someObservable.subscribe(foo => bar))

My problem is that I haven't found a sufficiently clean way to handle the unsubscription. The ideal way would be a simple 

ngOnDestroy () {
    this.subscriptions.forEach(subscription => subscription.unsubscribe())
}

but this doesn't work. Notably, I'm still receiving Firebase permission errors after logging out (which doesn't happen with the "working" methods). Interestingly, the exact same method does work if I pull it out into a separate class:

export class Utils {
    public static unsubscribeAll (subObject: {subscriptions: Subscription[]}) {
        subObject.subscriptions.forEach(subscription => subscription.unsubscribe())
    }
}

// ---------- back to the component

ngOnDestroy () {
    Utils.unsubscribeAll({subscriptions: this.subscriptions}) // no Firebase errors
}

but I don't really like this solution, mostly because it only works if I wrap the array in an object so it passes as a reference. The other working method I found was to write it as a for loop:

ngOnDestroy () {
    /* tslint:disable */
    for (let i = 0; i < this.subscriptions.length; i++) {
        this.subscriptions[i].unsubscribe()
    }
    /* tslint:enable */
}

but aside from the unnecessary length, it also makes TSLint complain because it thinks I should be using a for-of loop instead (which doesn't work) so I have to throw in the extra comments every time.

Currently I'm using the Utils option as the "best" solution, but I'm still not happy with it. Is there a cleaner way to do this that I'm missing?

John Montgomery
  • 6,739
  • 9
  • 52
  • 68
  • 2
    weird, `subscriptions.forEach(subscription => subscription.unsubscribe())` is equivalent for `for (let i = 0; i < this.subscriptions.length; i++) {` – Max Koretskyi Jul 13 '17 at 17:28
  • @Maximus At first I assumed it was some under-the-hood distinction where it's copying instead of referencing the elements, but I can't figure out why pulling it out into a separate class would work in that case so I really have no idea what's going on here. – John Montgomery Jul 13 '17 at 17:45
  • can you tell a little bit more about your subscriptions? show them in code and what they do. also [read this article](https://medium.com/@benlesh/rxjs-dont-unsubscribe-6753ed4fda87) – Max Koretskyi Jul 13 '17 at 17:54
  • @Maximus It's mainly for subscriptions to Firebase data. If I don't unsubscribe manually, it throws permission errors when I log out. That link helped (I didn't realize you could add multiple subscriptions together which is pretty much what I was trying to do with the array) if you want to post that as an answer, though I'm still curious why it wasn't working as it was. – John Montgomery Jul 13 '17 at 18:21
  • 1
    I'm not really familiar with Angular, but noticed that your "working" examples refer to subscriptions as `this.subscriptions` while "not working" just as `subscriptions`, could it be related? Shouldn't `this` be used to access object members? Aside from that, `Rx.Subscription` allows to add new subscriptions and then unsubscribe from all of them at once, meaning that array is not needed: `const subscription = new Rx.Subscription(); subscription.add(source1.subscribe()); subscription.add(source2.subscribe()); subscription.unsubscribe();` – Sergey Karavaev Jul 13 '17 at 20:16
  • @SergeyKaravaev Sorry, that was a copying error not related to my problem (without `this` it doesn't compile at all). Your suggestion was what I ended up going with. – John Montgomery Jul 13 '17 at 20:53
  • You may want to take a look at [this question](https://stackoverflow.com/questions/42490265/rxjs-takeuntil-angular-components-ngondestroy). The top answer provides a way to unsubscribe in `ngOnDestroy` without having to keep track of a bunch of subscriptions. It isn't much easier but it is by just a hair. – Pace Jul 17 '17 at 22:02

2 Answers2

61

Since nobody wants to post their answer as an answer, I guess I'll do it.

If you're only using the array to group them for mass unsubscribing, you can accomplish the same thing by merging them into an existing subscription. For my setup, just change the empty array to an empty subscription and change push to add:

private subscriptions = new Subscription()
this.subscriptions.add(someObservable.subscribe(foo => bar))

Then, when you want to unsubscribe from them all, just unsubscribe from the container subscription and it will handle everything.

ngOnDestroy () {
    this.subscriptions.unsubscribe()
}

Note: you can also use takeUntil on each individual subscription, but I feel like this method is simpler and makes more sense for what I'm doing.

John Montgomery
  • 6,739
  • 9
  • 52
  • 68
  • How would you unsubscribe one specific subscription, though? After using "add" I mean. – Rafael May 30 '19 at 10:57
  • 2
    @Rafael You'd need to maintain a separate reference to it in that case, something like `this.foo = someObservable.subscribe(); this.subscriptions.add(this.foo)` and then you can either unsubscribe it on its own or with all the others. – John Montgomery May 30 '19 at 19:37
  • Don't use takeUntil, it tries to close the observable, not just the subscription. It created pain and havoc in my app due to subscriptions from Subjects still firing after being unsubscribed. – John White Feb 23 '21 at 09:58
-1
subs: Subscription[] = [];

ngOnInit(): void {
    this.subs.push(someObservable.subscribe(foo => bar));
}

ngOnDestroy(): void {
    this.subs.map(s => s.unsubscribe);
}
Vivek Jain
  • 2,730
  • 6
  • 12
  • 27
Hunzla Vlogs
  • 9