1

Let's say I have this component, in which the users can call getData() function multiple times, how to manage this correctly? As I understand, this will cause memory leaks because it's creating a new subscription for each getData() function call. Thanks in advance.

export class AnalyticsComponent {
  constructor(
    private http: HttpClient
  ) { }

  getData() {
    const url = `${environment.apiUrl}/protected`;
    this.http.get(url).pipe(
      map(response => console.log(response)),
      catchError(async error => console.error(error))
    ).subscribe();
  }
}
heaxyh
  • 573
  • 6
  • 20
sonEtLumiere
  • 4,461
  • 3
  • 8
  • 35
  • 1
    Note that in angular 16 we'll have the [`DestroyRef`](https://netbasal.com/getting-to-know-the-destroyref-provider-in-angular-9791aa096d77?gi=0cda3a08ddd3) provider – Pieterjan Apr 23 '23 at 05:39

2 Answers2

3

Couple things:

  1. Usually you don't perform fetch operations directly inside a component. There are exceptions, but usually it's better to let an injectable service do the fetching and let the component do the rendering. This improves reusability and responsibility segregation which are two factors that will make it easier to reason about and work on your code.
@Injectable()
export class DataService {
  getData() {}
}
@Component()
export class AnalyticsComponent {
  readonly data$ = inject(DataService).getData();
}
  1. Usually your service methods should return the Observable instead of calling .subscribe(). Returning the observable allows downstream consumers to continue modifying the data coming from the source and allows them to decide when and how to subscribe. This isn't possible if you subscribe for your consumers. Also, if you subscribe in the service method, you have to handle returning asynchronously since your observable might not complete before control is returned.
type MyType = { foo: number; bar: number; }

getData(): Observable<MyType> {
  return this.http.get<MyType>();
}
// We can pipe the returned observable
// to ignore properties we don't need and
// add ones that weren't provided.
data$ = inject(DataService).getData().pipe(
  map((data: MyType) => ({ foo, baz: data.foo + data.bar }))
);
  1. There isn't really a memory leak when you .subscribe() to an Observable returned from one of the .get()/.post()/.put()/.delete()/etc methods on HttpClient since the framework will automatically complete the observable when the request is finished. You can read more about this here: Is it necessary to unsubscribe from observables created by Http methods? The actual reason you want to share the result of your fetch is to prevent re-fetching when it's not necessary.

That out of the way, there are two general approaches: using the shareReplay() operator to only perform the fetch for the first subscriber (all subsequent subscribers will receive the same value the first subscriber got), or storing the result of the fetch in the service using a Subject or BehaviorSubject.

@Injectable()
export class DataService {
  readonly http = inject(HttpClient);

  getData() {
    return this.http.get<MyType>(url).pipe(
      shareReplay(1)
    );
  }
}
@Injectable()
export class DataService {
  readonly http = inject(HttpClient);

  private _data = new BehaviorSubject<MyType | null>(null);

  getData() {
    if (this._data.value === null) this.refreshData();
    return this._data.asObservable();
  }

  refreshData() {
    this.http.get<MyType>().subscribe(data => this._data.next(data));
  }
}
D M
  • 5,769
  • 4
  • 12
  • 27
2

It's hard to answer this reasonably well with out additional information here.

The first issue, the HttpClient should be in a service, not the component itself.

Then you can do something like:

//example a
ngOnInit() {
 this.data = this.apiService.getData.pipe(
  map(response => console.log(response)),
  catchError(async error => console.error(error))
)
}

And then perhaps an async pipe on the template. This isn't the only choice,

ngOnInit(){
  this.apiSerive.get().subscribe(s => {
    this.data = s;
  })
}

could also be entirely valid, as well with the operators you need.

Where memory leaks are likely to come in: If the component is often reused:

<div *ngFor="let chart of allAnalytics"><app-analytics [chart]="view"></app-analytics></div>

Potentially, each of those could have a stream that is just left, out there. Objects sitting in memory.

There are other potential issues too. Angular doesn't do great (yet) with a template calling a method. You might see additional calls for components being re-rendered as well as additional checks for updates, or updates creating more updates. Check this even just by logging everytime that method is called. You might see 2,4 or 20 messages logged when you expected 1.

If you went with example A instead of the original code, the mitigation strategy is pretty easy:

ngOnDestroy(){
 this.myDataSubscription.unsubscribe();
}

This basically just makes sure each component has a reference to the same subscription, and any objects you needed are destroyed when the component leaves the DOM.

Austin T French
  • 5,022
  • 1
  • 22
  • 40