Do I need to `complete()` takeUntil Subject inside ngOnDestroy?

Question

To avoid Observable memory leaks inside Components, I am using takeUntil() operator before subscribing to Observable.

I write something like this inside my components:

private unsubscribe$ = new Subject();

ngOnInit(): void {
  this.http
    .get('test')
    .pipe(takeUntil(this.unsubscribe$))
    .subscribe((x) => console.log(x));
}

ngOnDestroy(): void {
  this.unsubscribe$.next();
  this.unsubscribe$.complete(); // <--- ??
}

Finally my question is following:

Do I need to write this.unsubscribe$.complete(); or next() is enough?

Is unsubscribe$ going to be grabbed by garbage collector without completing?

Please explain is there difference or doesn't matter. I don't want memory leaks in my components.

Answer 1

Short answer, no this is not needed, but it also doesn't hurt.

Long answer:

Unsubscribing / completing in angular is only needed when it prevents garbage collection because the subscription involves some subject that will outlive the component due to be collected. This is how a memory leak is created.

if I have service:

export class MyService {
  mySubject = new Subject();
}

which is provided in root and only root (which means it's a singleton and will never be destroyed after instantiation) and a component that injects this service and subscribes to it's subject

export class MyLeakyComponent {
  constructor(private myService: MyService) {
    this.myService.mySubject.subscribe(v => console.log(v));
  }
}

this is creating a memory leak. Why? because the subscription in MyLeakyComponent is referenced by the subject in MyService, so MyLeakyComponent can't be garbage collected so long as MyService exists and holds a reference to it, and MyService will exist for the life of the application. This compounds everytime you instantiate MyLeakyComponent. To fix this, you must either unsubscribe or add a terminating operator in the component.

however this component:

export class MySafeComponent {
  private mySubect = new Subject();
  constructor() {
    this.mySubject.subscribe(v => console.log(v));
  }
}

is completely safe and will be garbage collected without issue. No external persisting entity holds a reference to it. This is also safe:

@Component({
  providers: [MyService]
})
export class MyNotLeakyComponent {
  constructor(private myService: MyService) {
    this.myService.mySubject.subscribe(v => console.log(v));
  }
}

now the injected service is provided by the component, so the service and the component will be destroyed together and can be safely garbage collected as the external reference will be destroyed as well.

Also this is safe:

export class MyHttpService { // root provided
  constructor(private http: HttpClient) {}
  
  makeHttpCall() {
    return this.http.get('google.com');
  }
}

export class MyHttpComponent {
  constructor(private myhttpService: MyHttpService) {
    this.myhttpService.makeHttpCall().subscribe(v => console.log(v));
  }
}

because http calls are a class of observables that are self terminating, so they terminate naturally after the call completes, so no need to manually complete or unsubscribe, as the external reference is gone once it naturally completes.

As to your example: the unsubscribe$ subject is local to the component, thus it cannot possibly cause a memory leak. This is true of any local subject.

A note on best practices: Observables are COMPLEX. One that might look completely safe, could involve an external subject in a subtle manner. To be totally safe / if you're not extremely comfortable with observables, it is generally recommended that you unsubscribe from all non terminating observables. There isn't a downside other than your own time spent doing it. I personally find the unsubscribe$ signal method hacky and think it pollutes / confuses your streams. the easiest to me is something like this:

export class MyCleanedComponent implements OnDestroy {
  private subs: Subscription[] = [];
  constructor(private myService: MyService) {
    this.subs.push(
      this.myService.mySubject.subscribe(v => console.log(v)),
      this.myService.mySubject1.subscribe(v => console.log(v)),
      this.myService.mySubject2.subscribe(v => console.log(v))
    );
  }
  
  ngOnDestroy() {
    this.subs.forEach(s => s.unsubscribe());
  }
}

However, the single BEST method for preventing leaks is using the async pipe provided by angular as much as possible. It handles all subscription management for you.

Answer 2

I can see that you are subscribing to a http response. There is one important thing when using Angular HttpClient: You don't have to unsubscribe from it as it is done automatically!

You can test that using finalize operator - it is called when observable completes. (And when observable completes, it is automatically unsubscribed)

this.http
  .get('test')
  .pipe(takeUntil(this.unsubscribe$), finalize(() => console.log('I completed and unsubscribed')))
  .subscribe((x) => console.log(x));

If you are worried that your component might die while the HTTP request is still on it's way, and that code in callback might still execute, you could do something like this:

private subscription: Subscription;

ngOnInit(): void {
  this.subscription = this.http
    .get('test')
    .subscribe((x) => console.log(x));
}

ngOnDestroy(): void {
  // Here you should also do a check if subscription exists because in most cases it won't
  this.subscription.unsubscribe();
}

It is also worth checking the AsyncPipe

The AsyncPipe subscribes (and unsubscribes) for you automatically.