1

i have multiple functions that have subscriptions. First i have it on the ngOnInit(), then i have another function called onSelectVillain(). So my question is this possible that you could just use this.subscription.unsubscribe(). Or should i make declare another subscription?

subscription: Subscription;

    ngOnInit() {
      this.subscription = this.heroService.getHeroes()
                       .subscribe(
                         heroes => this.heroes = heroes,
                         error =>  this.errorMessage = <any>error);
    }

    onSelectVillain(event) {
      this.subscription = this.villainService.getVillains()
                       .subscribe(
                         .....
    }

    ngOnDestroy(){
     this.subscription.unsubscribe()
    }
  • using a separate subscription is best way to unsuscribe. using same variable provides you the last subscribed subscription not all of them. So when you unsubscribe you need reference to all of them to complete the unsubcription process. – Shubham Agarwal Bhewanewala Mar 24 '18 at 09:20

2 Answers2

3

It would be cleaner to use a seperate subscription - and if you use the same field you would never (manually) unsubscribe from the first subscription. Also if you don't want to clutter your component with lots of fields, that simply hold subscription references I would recommend to use a pattern, that involves using just one Subject, that is triggered at ngOnDestroy and before each subscribe you would use takeUntil. So your code could look like this:

private ngUnsubscribe = new Subject();

ngOnInit() {
  this.heroService.getHeroes()
                  .takeUntil(this.ngUnsubscribe)
                  .subscribe(
                     heroes => this.heroes = heroes,
                     error =>  this.errorMessage = <any>error);
}

onSelectVillain(event) {
  this.villainService.getVillains()
                     .takeUntil(this.ngUnsubscribe)
                     .subscribe(
                     .....
}

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

See this for further reference.

Note that subscriptions that are "finite", so where a complete state is called, don't neccessarily need to get unsubscribed manually. This is probably a good reference point for that.

Jay
  • 175
  • 1
  • 6
1

Once subscription value is replaced, previous subscription is lost, it's no different than any other value.

A cleaner way is to have different subscriptions that have meaningful names - heroesSubscription, villainsSubscription, etc:

heroesSubscription: Subscription;
villainsSubscription: Subscription;

ngOnInit() {
  this.heroesSubscription = this.heroService.getHeroes().subscribe(...);
}

onSelectVillain(event) {
  // possibly needs to unsubscribe previous subscription
  if (this.villainsSubscription)
    this.villainsSubscription.unsubscribe()

  this.villainsSubscription = this.villainService.getVillains().subscribe(...)
}

ngOnDestroy(){
 this.heroesSubscription.unsubscribe()
 this.villainsSubscription.unsubscribe()
}

If it's possible that onSelectVillain is called multiple times, previous subscription should be unsubscribed.

The code doesn't show the benefits of doing subscriptions manually. When observable values are consumed only in view, async pipe can be used instead because it does all subscription/unsubscription work automatically:

{{ heroService.getHeroes() | async }}
Estus Flask
  • 206,104
  • 70
  • 425
  • 565
  • @GraySingh It will work but it's not how it's usually done. – Estus Flask Mar 24 '18 at 13:26
  • I mean that the method suggested in another answer will work (additional subject for unsubscription). Whether an observable needs to be unsubscribed or not depends on observable itself. Usually it needs. – Estus Flask Mar 24 '18 at 13:34
  • I added the example, if this helps. – Estus Flask Mar 24 '18 at 13:39
  • Please check your on selectVillain. you called heroSubscription. It should be villainSubscription –  Mar 24 '18 at 13:40
  • You're welcome Note that you won't need to deal with subscriptions in case when it's surely synchronous, complete observable, like `Observable.of(1, 2)`. But this rarely happens in real app. – Estus Flask Mar 24 '18 at 13:43
  • Please check your on selectVillain(). you called heroSubscription. It should be villainSubscription – –  Mar 24 '18 at 13:44
  • Sure, that's a typo, fixed it. – Estus Flask Mar 24 '18 at 13:48
  • Theres an error when i have different variables in subscription. So i just need same variable subscription assign to the ngOnInit() and the onSelectVillain(). Just what i did above on my code. –  Mar 24 '18 at 15:30
  • This is not a valid approach. You lose one of those subscriptions and cannot unsubscribe from it. If you have specific problem with that, consider re-asking the question that explains what exact problem is. – Estus Flask Mar 24 '18 at 15:34
  • @estus What do you mean by "It will work but it's not how it's usually done"? It's a pattern that according to [the accepted answer to this question](https://stackoverflow.com/questions/38008334/angular-rxjs-when-should-i-unsubscribe-from-subscription) is the official/recommended way to do it. When you have more than just one subscription in a component I think it's a lot more readable and maintainable than having seperate variables for each subscription. – Jay Mar 25 '18 at 11:21
  • @Jay It's interesting to know that this pattern is at least used by some team members. But this doesn't make it 'official' way. As I know, it isn't recommended in docs, and it isn't recognized at all, I've seen it 2 or 3 times before. The pattern doesn't have real flaws, but IMO it is less straightforward and breaks KISS principle. If there's too much subscriptions in a class that need ngUnsubscribe and are tedious to unsubscribe one by one, this usually means that a class and its observables were designed inefficiently (bad observable composition, not using async pipe, etc). – Estus Flask Mar 25 '18 at 12:54