Harleyz
Harleyz

Reputation: 53

RXJS chain subscriptions anti-pattern with NGRX

I have the following code, which is working, but I've read that chaining subscriptions is an anti-pattern. I need to make sure that the user has a loggedin state of true, before making a call to the API which requires the users authtoken.

What would be the proper way to handle this in RXJS?

const subscrAuth = this.store
  .pipe(select(isLoggedIn))
  .subscribe(isLoggedIn => {
    console.log(isLoggedIn);
    if (isLoggedIn) {
      const subscrLoaded = this.store
        .pipe(select(hasCarriersLoaded))
        .subscribe(hasCarrierLoaded => {
          if (!hasCarrierLoaded) {
            this.store.dispatch(new CarriersPageRequested());
          }
        });
    }
  });
this.unsubscribe.push(subscrAuth);

Upvotes: 0

Views: 523

Answers (2)

Poul Kruijt
Poul Kruijt

Reputation: 71961

If it's just an API call, you wouldn't need to unsubscribe, because it completes by itself. You can use switchMap to chain and filter to.. filter :). And you have a nice chain of rxjs operators, which self completes

this.store.pipe(
  select(isLoggedIn),
  take(1),
  filter((isLoggedIn) => isLoggedIn),
  switchMapTo(this.store),
  select(hasCarriersLoaded),
  take(1),
  filter((hasCarriersLoaded) => !hasCarriersLoaded),
).subscribe(() => this.store.dispatch(new CarriersPageRequested()));

It feels like though that you can combine the two selects:

combineLatest([
  this.store.pipe(select(isLoggedIn)),
  this.store.pipe(select(hasCarriersLoaded))
]).pipe(
  take(1),
  filter(([loggedIn, hasCarriers]) => loggedIn && !hasCarriers)
).subscribe(() => this.store.dispatch(new CarriersPageRequested()));

If you however need to wait before the user is logged in, while handling this request, you can do the following:

combineLatest([
  this.store.pipe(select(hasCarriersLoaded)),
  this.store.pipe(
    select(isLoggedIn),
    filter((loggedIn) => loggedIn)
  )
]).pipe(
  take(1),
  filter(([hasCarriers]) => !hasCarriers)
).subscribe(() => this.store.dispatch(new CarriersPageRequested()));

If you also need to wait to be logged in to call hasCarriersLoaded, you should switch the filter with the take(1) in the first example

Upvotes: 2

tuckerjt07
tuckerjt07

Reputation: 922

Something like this?

const subscrAuth = this.store
.pipe(
  select(isLoggedIn),
  switchMap(isLoggedIn => {
    this.store.pipe(
      select(hasCarriersLoaded),
      tap(hasCarriersLoaded => {
        this.store.dispatch(new CarriersPageRequested());
      })
     )
  })
).subscribe();
this.unsubscribe.push(subscrAuth);

I'd also recommend either using the ngOnDestroy function to manage your subscriptions like so,

onDestroy$: Subject<null> = new Subject();

ngOnDestroy() {
    this.onDestroy$.next();
}

this.store
.pipe(
  takeUntil(this.onDestroy$),
  select(isLoggedIn),
  switchMap(isLoggedIn => {
    this.store.pipe(
      select(hasCarriersLoaded),
      tap(hasCarriersLoaded => {
        this.store.dispatch(new CarriersPageRequested());
      })
     )
  })
).subscribe();

or if you are only going to be making a single call you could also modify the pipe like so

this.store
.pipe(
  take(1),
  select(isLoggedIn),
  switchMap(isLoggedIn => {
    this.store.pipe(
      select(hasCarriersLoaded),
      tap(hasCarriersLoaded => {
        this.store.dispatch(new CarriersPageRequested());
      })
     )
  })
).subscribe();

Upvotes: 2

Related Questions