CODEWITHSUNDEEP
angularrxjs
Zach Starnes
Zach Starnes

Reputation: 3198

Getting rid of nested observables

So I have a method that relies on the data from two observables and the only way I have found to get it to work is by nesting the subscriptions of the observables and then calling the method inside the second subscribe method. This seems bad and there has to be a better way of accomplishing this can I get some help on a better way to do this?

Here is the controller:

meta$: Subscription;
meta: any;
data$: Subscription;
data: any;

ngOnInit() {
  this.route.params.subscribe(params => this.id = params['id']);
  this.getPageInfo();
}

private getPageInfo(): void {
  this.meta$ = this.api.get(`${Endpoints.LOCATIONS}all/metadata`).subscribe(m => {
    this.meta = m;
    this.data$ = this.api.get(`${Endpoints.LOCATIONS}/${this.id}`).subscribe(d => {
      this.data = d;
      this.setSelectedAttributes(); // <-- relies on this.meta and this.data
    }, err => console.error(err));
  }, err => console.error(err));
}

setSelectedAttributes(): void {
  const viewableAttributes = this.meta.columns.filter(c => !c.hidden && c.type === 'String');
  const selectedAttributes: Array<any> = [];
  for (const attr of viewableAttributes) {
    if (this.data[attr.field]) {
      selectedAttributes.push({name: attr.title, value: this.data[attr.field]});
    }
  }
  this.selectedAttributes = selectedAttributes;
}

ngOnDestroy() {
  this.data$.unsubscribe();
  this.meta$.unsubscribe();
}

Upvotes: 0

Views: 767

Answers (4)

MoxxiManagarm
MoxxiManagarm

Reputation: 9124

Without any subscription you could do it like this:

id$: Observable<number> = this.route.params.pipe(
    map(params => +params.id),
);


data$: Observable<Data> = this.id$.pipe(
  switchMap(id => this.api.get(`${Endpoints.LOCATIONS}/${this.id}`)),
);

export interface MetaData {
  columns: Atribute[];
}

export interface Attribute {
  hidden: boolean;
  type: string;
  title: string;
  field: string;
}

viewableAttributes$: Observable<Attribute[]> = this.api.get<MetaData>(`${Endpoints.LOCATIONS}all/metadata`).pipe(
  map(meta => meta.columns),
  filter(column => !column.hidden && c.type === 'String'),
);


export interface AttributeSelection {
  name: string;
  value: string;
}

selectedAttributes$: Observable<AttributeSelection[]> = forkJoin([this.data$, this.viewableAttributes$]).pipe(
  map(([data, viewableAttributes]) => viewableAttributes.map(attr => ({ name: attr.title, value: data[attr.field]})).filter(attr => attr.value)),
);

You then only need to use async pipe on selectedAttributes$

Upvotes: 0

Fan Cheung
Fan Cheung

Reputation: 11345

Looks like those two api call can happen in parallel, use forkJoin() and tap() for side effect -> instance property assignment

private getPageInfo(): void {
 forkJoin(this.api.get(`${Endpoints.LOCATIONS}all/metadata`),this.api.get(`${Endpoints.LOCATIONS}/${this.id}`)).pipe(tap(([m,d])=>{
this.data$=d
this.meta$=m
this.setSelectedAttributes()
}))
}

Upvotes: 0

martin
martin

Reputation: 96891

It looks like you could use just concatMap and then map the second response to contain also the response from the first response and maybe you won't even need to use any properties:

this.api.get(`${Endpoints.LOCATIONS}all/metadata`).pipe(
  concatMap(response1 => this.api.get(`${Endpoints.LOCATIONS}/${this.id}`)).pipe(
    map(response2 => [response1, response2])
  )
).subscribe(([response1, response2]) => {
  // Whatever here. You can set `this.meta = response1` etc...
});

Upvotes: 2

Ling Vu
Ling Vu

Reputation: 5181

You can use flatMap(). This is the same as mergeMap() which is used to prevent nested subscriptions:

this.meta$ = this.api.get(...).map(
   flatMap((m) => {
      this.meta = m;
      ...
      return this.api.get(...)

   }),
   // You could also make another request if you need
   // flatMap((m) => {
   //    ...
   //    return this.api.get(...)
   // }),
).subscribe((d) => {
   this.data = d;
});

For more information: https://www.learnrxjs.io/operators/transformation/mergemap.html

Another tip:

If you want to destory everything in 1 variable use takeUntil() bound to an attribute to destory:

@Component({
  selector: 'my-app',
  templateUrl: './app.component.html',
  styleUrls: [ './app.component.css' ]
})
export class AppComponent  {
  destroy$ = new Subject<boolean>();

  request() {
     return this.apiService.get(...).pipe(
         takeUntil(this.destroy$)
     )
  }

  ngOnDestroy(): void {
    this.destroy$.next(true);
  }
}

Upvotes: -1

Related Questions