Skip to content
This repository was archived by the owner on Dec 4, 2017. It is now read-only.

Router params unsubscribing required or not? #3003

Closed
PEsteves8 opened this issue Dec 19, 2016 · 10 comments
Closed

Router params unsubscribing required or not? #3003

PEsteves8 opened this issue Dec 19, 2016 · 10 comments

Comments

@PEsteves8
Copy link

PEsteves8 commented Dec 19, 2016

Some time ago the docs recommended that logic to unsubscribe route params should be included in the onDestroy hook.

However, that bit was removed, leading one to believe that that's not required anymore.

That said, expressions like: "we'll feed the Observable directly into our template using the AsyncPipe, which will handle unsubscribing from the Observable for us when the component is destroyed" or "The router offers a Snapshot alternative that gives us the initial value of the route parameters. We don't need to subscribe or unsubscribe. It's much simpler to write and read" are still present.

My point is that these excerpts indicate that unsubscribing onDestroy is actually still required.

So assuming that there is no need to unsubscribe on destroy, wouldn't it make more sense to change "We don't need to subscribe or unsubscribe." to just "We don't need to subscribe.", since we don't need to unsubscribe anyway?

NOTE: the assumption that unsubscribing using the onDestroy hook isn't necessary came from discussions in the gitter channel, where I was told that by an angular dev.

@wardbell
Copy link
Contributor

The statements you cite are still correct. I'm not sure how you derive any conclusions about subscribing/unsubscribing other Observables from those statements.

You do subscribe to other Observables such as the ActivatedRoute and the returned values from Http. These particular observables are designed so you do not have to unsubscribe from them.

The ActivatedRoute evolved such that it unsubscribes all of its subscribers when the component is destroyed so you don't have to do it. That was not always the case ... as you recall in your issue.

You can always play it safe and unsubscribe ... and that may be a best practice; the jury is still out on that.

We have a guide coming soon that talks about RxJS in Angular and recommends sound patterns for these things.

Thanks for your thoughts. I'm closing because I don't think there is any action that needs to be taken ...that we aren't already taking elsewhere.

@PEsteves8
Copy link
Author

"The ActivatedRoute evolved such that it unsubscribes all of its subscribers when the component is destroyed so you don't have to do it." This is why I was a bit confused over the fact that there is an emphasis on how there is no need to unsubscribe using the snapshot alternative. Because from what I now understand, if we're getting the params from a subscription , unsubscribing happens anyway onDestroy.

Summarizing, indicating that the snapshot alternative allows us not to unsubscribe, leads one to believe that using the subscription version, requires us to unsubscribe (which isn't necessarily the case as unsubscribing happens onDestroy automatically).

Anyway, thank you very much for your clear answer. I'll await documentation updates.

@PEsteves8 PEsteves8 changed the title Router params unsubscription required or not? Router params unsubscribing required or not? Dec 20, 2016
@wardbell
Copy link
Contributor

I see your point. Not having to unsubscribe was a virtue of snapshot. Now it isn't. Not having to subscribe remains a snapshot virtue. I'll update prose accordingly.

@seangwright
Copy link

seangwright commented Dec 21, 2016

@wardbell I saw a question on StackOverflow connected to the need for direction when it comes to the requirement for unsubscribing.

I pieced together what information I could from sleuthing the web for answers and tried to provide some help there but I'm sure there are more nuanced suggestions and answers than what I was able to give.

I also wrote up a blog post mirroring some of this information and I've been using both of these sources for my team's project as an extension of the styleguide/docs when it comes to Angular and Observables.

I'm commenting here only because I was going to open up an issue (before I found this one) asking about any planned documentation around this area, if any assistance would be helpful (though I feel my rx-fu is beginner level) and if there were other none official sources to consult in the meantime.

For example I was interested in the unsubscribe() requirements for the example Misko recently wrote and what should be done when subscribing to Subjects instantiated inside components.

Thanks!

@wardbell
Copy link
Contributor

We have a guide on RxJS in Angular underway with some good guidance there (I believe)
Stay tuned.

@ScreamZ
Copy link

ScreamZ commented Dec 28, 2016

@wardbell This guide will be release on ng2 official documentation ? Thanks

@wardbell
Copy link
Contributor

wardbell commented Jan 2, 2017

Yes it will

@MaklaCof
Copy link

MaklaCof commented Apr 9, 2017

Is this guide already available? Where? Can someone post URL. Thank you.

@seangwright
Copy link

seangwright commented Apr 9, 2017

@MaklaCof

I talked with @wardbell at NGConf a couple days ago and he recommended using a private Subject that would next() and then complete() in the ngOnDestroy() call.

All observables would have a .takeUntil(privateSubject) directly before the .subscribe() call so everything gets cleaned up with the component is destroyed.

The docs are forthcoming and will recommend this approach as the best solution to this question, but if you want to see example code and a clearer explanation that matches what Ward explained to me, see my original answer on SO which I have updated.

@randallmeeker
Copy link

are the docs available yet?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants