Reputation: 829
When looking at code examples, I often see the case where observables inside services are not unsubscribed from.
Here is an example:
export class AuthGuard implements CanActivate {
private isLoggedIn: boolean;
private isLoggedIn$: Observable<boolean>;
constructor(private authService: AuthService, private router: Router) {
this.isLoggedIn$ = this.authService.isLoggedIn();
this.isLoggedIn$.subscribe(res => {
if (res) {
this.isLoggedIn = true;
}
else {
this.isLoggedIn = false;
}
});
}
canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) {
if (this.isLoggedIn) {
return true;
}
else {
this.router.navigate(['login']);
return false;
}
}
}
Is there a reason why you would not unsubscribe from the observable this.isLoggedIn$ in this case? Or is the above example just bad coding leading to memory leaks?
Upvotes: 6
Views: 5157
Reputation: 71961
For core services an unsubscribe is not really necessary, because the service lives as long as the application lives, and the service is a singleton. Problem is that you have to make sure you always use it as a singleton (for better solutions, read below)
For component services an unsubscribe should be placed inside the ngOnDestroy
of the service, because services actually can implement this NgOnDestroy
hook. A better way would be to use the takeUntil(this.destroy)
pipe and emit this on destroy.
An even better way would be to use async
pipe inside the template, and never really directly subscribe to things like these.
In your guard on the other hand, you can use the take(1)
pipe, this will take the first emit and immediately unsubscribes without the need for you to do it, this will change your example to this. As you can see, no subscribe inside the code:
export class AuthGuard implements CanActivate {
constructor(private authService: AuthService, private router: Router) {}
canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) {
return this.authService.isLoggedIn().pipe(
take(1),
tap((loggedIn) => {
if (!loggedIn) {
this.router.navigate(['login'])
}
})
);
}
}
Bottom line, try to keep your application a stream of Observables using the async inside templates and the rxjs pipe function, instead of subscribing and storing sub results. To make the isLoggedIn()
emit the last result when subscribed to, you can use a shareReplay()
pipe or make it a BehaviourSubject
to begin with.
Upvotes: 4
Reputation: 54821
When looking at code examples, I often see the case where observables inside services are not unsubscribed from.
You are looking at a bug.
Is there a reason why you would not unsubscribe from the observable this.isLoggedIn$ in this case?
If you wanted to leak memory.
Or is the above example just bad coding leading to memory leaks?
Yes, it's leaking memory. The subscribe function will continue to be executed after the object referenced by this
is destroyed. It might never happen if this object is never destroy, but it's an example of code that would fail a unit test where objects are created temporarily.
Objects tagged with @Injectable()
often behave like singletons or have weak references. It might be working for a while, but once you use it in a temporary case it'll leak memory.
Upvotes: 0