William Martins
William Martins

Reputation: 442

Do I need to complete a Subject that is only used to compose another Observable?

Hi I have the following component:


export class PreviewReportComponent implements OnInit, OnChanges, OnDestroy {
  @Input() params: PreviewParams;
  reload$: Subject<void> = new Subject();
  preview$: Observable<SafeHtml>;
  constructor(private reportService: ReportService) {}

  ngOnInit() {
    this.preview$ = this.reload$.pipe(
      debounceTime(200),
      startWith([null]),
      switchMap(() => this.reportService.getPreview(this.params)),
    );
  }

  ngOnChanges() {
    this.reload$.next();
  }

  ngOnDestroy(){
    this.reload$.complete();
  }
}

Do I need to implements OnDestroy and call complete on reload$ subject?

Im already using AsyncPipe to subscribe to preview$ Observable.

Upvotes: 1

Views: 306

Answers (1)

BizzyBob
BizzyBob

Reputation: 14740

Do I need to implements OnDestroy and call complete on reload$ subject?

If your concern was memory leaks, then NO, it's not necessary to implement OnDestroy to complete your subject, since you are not subscribing in your component. Memory leaks occur when you have Subscriptions that are left open after the component is destroyed.

Since you are using AsyncPipe to handle subscriptions, you don't need to worry about it.


Actually, you don't even need OnInit:

export class PreviewReportComponent implements OnChanges {
  @Input() params: PreviewParams;

  private reload$ = new Subject<void>();

  public preview$ = this.reload$.pipe(
    startWith([null]),
    debounceTime(200),
    switchMap(() => this.reportService.getPreview(this.params)),
  );

  constructor(private reportService: ReportService) {}

  ngOnChanges() {
    this.reload$.next();
  }

}

You could also get rid of OnChanges as well, if you change the params to be a setter:

export class PreviewReportComponent {
  private params$ = new Subject<PreviewParams>();

  @Input() set params(params: PreviewParams) {
    this.params$.next(params);
  }

  public preview$ = this.params$.pipe(
    startWith([null]),
    debounceTime(200),
    switchMap(params => this.reportService.getPreview(params)),
  );

  constructor(private reportService: ReportService) {}
}

Upvotes: 2

Related Questions