Refactoring chained RxJs subscriptions

user_1708960 0
Viewed 123

I have a piece of code that I need to refactor because it's a hell of chained subscriptions.

ngOnInit(): void {
    this.dossierService.getIdTree()
        .subscribe(idTree => {
            this.bootstrappingService.refreshObligations(idTree)
                .subscribe(() => {
                    this.dossierPersonsService.retrieveDossierPersons(idTree)
                        .subscribe(debtors => {
                            this.retrieveObligations();
                            this.debtors = debtors;
                        });
                });
        });
}
  1. The first call dossierService.getIdTree() retrieves idTree which is used by other services except obligationsService.retrieveObligations().
  2. All service methods should be executed in the order they executed now. But retrieveDossierPersons and retrieveObligations can be executed in parallel.
  3. retrieveObligations() is a method that subscribes to another observable. This method is used in a few other methods.

I've refactored it and it seems to work. But did I refactor it in a proper way or my code can be improved?

   this.dossierService.getIdTree()
      .pipe(
        map(idTree => {
          this.idTree = idTree;
        }),
        switchMap(() => {
          return this.bootstrappingService.refreshObligations(this.idTree)
        }),
        switchMap(
          () => {
            return this.dossierPersonsService.retrieveDossierPersons(this.idTree)
          },
        )
      )
      .subscribe(debtors => {
        this.retrieveObligations();
        this.debtors = debtors;
      });
rxjs angular javascript
3 Answers

Something like this (not syntax checked):

  ngOnInit(): void {
    this.dossierService.getIdTree().pipe(
      switchMap(idTree =>
        this.bootstrappingService.refreshObligations(idTree)).pipe(
          switchMap(() => this.dossierPersonsService.retrieveDossierPersons(idTree).pipe(
            tap(debtors => this.debtors = debtors)
          )),
          switchMap(() => this.retrieveObligations())
        )
    ).subscribe();
  }

Using a higher-order mapping operator (switchMap in this case) will ensure that the inner observables are subscribed and unsubscribed.

In this example, you don't need to separately store idTree because you have access to it down the chained pipes.

user_16476040

You could try something like:

 ngOnInit(): void {
   const getIdTree$ = () => this.dossierService.getIdTree();
   const getObligations = idTree => this.bootstrappingService.refreshObligations(idTree);
   const getDossierPersons = idTree => this.dossierPersonsService.retrieveDossierPersons(idTree);

   getIdTree$().pipe(
     switchMap(idTree => forkJoin({
       obligations: getObligations(idTree)
       debtors: getDossierPersons(idTree),
     }))
   ).subscribe(({obligations, debtors}) => {
     // this.retrieveObligations(); // seems like duplicate of refreshObligations?
     this.debtors = debtors;
   });
 }

Depending on the rest of the code and on the template, you might also want to avoid unwrapping debtors by employing the async pipe instead

forkJoin will only complete when all of its streams have completed. You might want also want to employ some error handling by piping catchError to each inner observable.

Instead of forkJoin you might want to use mergeMap or concatMap (they take an array rather than an object) - this depends a lot on logic and the UI. concatMap will preserve the sequence, mergeMap will not - in both cases, data could be display accumulatively, as it arrives. With forkJoin when one request gets stuck, the whole stream will get stuck, so you won't be able to display anything until all streams have completed.

user_7766860

You can use switchMap or the best choice is concatMap to ensure orders of executions

obs1$.pipe(
 switchMap(data1 => obs2$.pipe(
   switchMap(data2 => obs3$)
 )
)
user_43992810
Related