btroncone / ngrx-examples

@ngrx examples and resources
371 stars 68 forks source link

Shopping cart example question. Is it necessary to call store.unsubscribe? #17

Closed wiredprogrammer closed 8 years ago

wiredprogrammer commented 8 years ago

Looking in the shopping cart example I see unsubscribe being called on store during the OnDestroy hook of the component.
I'm getting a unsubscribe error when I attempt to do the same in my app. I don't see this being done in the brief component example on the ngrx/store readme.

Please forgive me if I'm missing something. I've been scaffolding my app based off this example and the main ngrx example project structure. https://github.com/ngrx/example-app

btroncone commented 8 years ago

Hello, If you have a copy of your source I could take a look. It should only be necessary to call unsubscribe when you are manually creating a subscription within your component. If using the AsyncPipe this will be handled for you. Hope this helps! 👍

wiredprogrammer commented 8 years ago

So are you calling "this.store.unsubscribe()" because of the "this.actions$.subscribe(store);".

A big difference between your code setup and mine is that I'm using the new beta component router. I am using the same set up you are to dispatch to the store to fire the effects to do a pull from the api.

I have components in my root app.component that I have to feed information from composed reducers. I set up something similar on each routed to page component with their own "actions$". The unsubscribe error happens when I move between routes. I'm calling the unsubscribe OnDestroy of each page component which I guess is unsubscribing everything and throwing errors on the navbar component's (above the router-outlet) async pipes.

So if this is only meant to be called on the app destruction (so one time and not during router component navigation) and I remove the unsubscribe from the page components OnDestroy lifecycle hooks then is there any potential for issues with the page components if they are doing similar "actions$" subscriptions to the store?

Please forgive me if I'm missing some sort of rxjs fundamental. I've been doing my best to grasp the concepts. Thanks so much for taking the time to answer!!

btroncone commented 8 years ago

No worries! There are a lot of concepts to wrap your head around.

You actually caught an unneeded unsubscribe. You should never have to call unsubscribe on store in your app, the only reason it wasn't causing an issue in this example was because this component was never being destroyed. This was a bug waiting to happen.

There are times, however, where you will need to unsubscribe within a component in ngOnDestroy. This will occur when you are creating a subscription within the component instead of using the AsyncPipe. For example:

this.store.select('myState').subscribe(s => this.something = s)

In this case you would need to unsubscribe within your component, like below:

this.subscription = this.store.select('myState').subscribe(s => this.something = s);

//later...
ngOnDestroy(){
  this.subscription.unsubscribe()
}

If you have multiple subscriptions within your component you can add them to a single subscription, and they will all be unsubscribed in one call, like so:

this.subscription = this.store.select('myState').subscribe(s => this.something = s);
const anotherSub =  this.store.select('anotherState').subscribe(s => this.someThingElse= s);
this.subscription.add(anotherSub)

//later...
ngOnDestroy(){
  this.subscription.unsubscribe()
}

I also have a free video here which explains this in more detail. I apologize for the confusion and I have updated that file. Hope this helps!

wiredprogrammer commented 8 years ago

Thanks for the clarification. Thanks for the examples too. They helped clarify my understanding of how to do certain things that didn't feel as clear looking at that one big ngrx example-app. Once I understood your examples it made it easier to understand what that example-app was doing.

btroncone commented 8 years ago

Awesome, no problem!