Is this the right way to use a BehaviorSubject with a paginated list in Angular 9?

Question

I have a list of products coming from an API. This list is shown and paginated in a Component. A pagination change doesn't trigger a URL change or reload the component – but it does load a new set of products.

I need to get the list in the Component because I have to extract/modify some of its values. So it's not sufficient to simply use the AsyncPipe in the template.

The solution I came up with is using a BehaviorSubject. I wonder whether this approach is correct.

This is the Service:

export class ProductService {
  public list$ = new BehaviorSubject<Product[]>(null);

  getAll(criteria: any): Subscription {
    const path = '/api';

    return this.http.post<any>(path, criteria).pipe(
      map((response: any) => {
        // some mapping …
        return response;
      })
    ).subscribe(response => this.list$.next(response));
  }
}

This is the Component:

export class ProductComponent implements OnInit, OnDestroy {
  products: Product[];
  page: number = 1;

  constructor(
    productService: ProductService
  ) { }

  ngOnInit() {
    this.productService.list$.subscribe(products => {
      this.products = products;
    });

    this.loadProducts();
  }

  ngOnDestroy() {
    this.productService.list$.unsubscribe();
  }

  loadProducts() {
    this.productService.getAll({page: this.page});
  }

  onPageChange(page: number) {
    this.page = page;
    this.loadProducts();
  }
}

My questions are:

• Is there a better way of doing this?
• Is this the right way?
• Is everything subscribed and unsubscribed correctly?
• Does this fail, if I would load two lists with different criterias in the Controller? If yes, how can I fix this?

Answer 1

Is there a better way of doing this?

Yes, I believe there is.

Is this the right way?

No, I am afraid not.

Is everything subscribed and unsubscribed correctly?

Nope, each time you call ProductService.getAll method, you create a new subscription without unsubscribing.

Does this fail, if I would load two lists with different criterias in the Controller? If yes, how can I fix this?

Yes it would fail, because service is pushing values to one single subject. You can fix this by making service stateless.

---

A better way imho:

export class ProductService {

  // query would be a better name, because it doesn't literally get all.
  query(criteria: any): Observable<Product[]> {
    const path = '/api';
    // This way service stays stateless.
    return this.http.post<Product[]>(path, criteria).pipe(
      map((response: any) => {
        // some mapping …
        return response;
      })
    );
  }
}

Then in the component, you can keep the page number in a subject and pipe for changes. Using async pipe together with some mapping could still be an option, but not a must have.

export class ProductComponent implements OnInit, OnDestroy {
  products: Product[];
  page$ = new BehaviourSubject<number>(1);
  products$: Observable<Product[]>;
  destroyed$ = new Subject<void>();

  constructor(
    productService: ProductService
  ) { }

  ngOnInit() {
    this.products$ = page$
      .pipe(
        switchMap((page: number) => this.productService.query({page}))
      );
    this.products$
      .pipe(takeUntil(this.destroyed$))
      .subscribe((products: Product[]) => this.products = products);
  }

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

  onPageChange(page: number) {
    this.page$.next(page);
  }
}

---

As a bonus, you can extract a base class with destroyed$ subject, so you can share it among components:

export class BaseComponent implements OnInit {
  readonly destroyed$ = new Subject<void>();

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

Then your component would be:

export class ProductComponent extends BaseComponent implements OnInit {
  products: Product[];
  page$ = new BehaviourSubject<number>(1);
  products$: Observable<Product[]>;

  constructor(productService: ProductService) {
    super();
  }

  ngOnInit() {
    this.products$ = page$
      .pipe(
        switchMap((page: number) => this.productService.query({page}))
      );
    this.products$
      .pipe(takeUntil(this.destroyed$))
      .subscribe((products: Product[]) => this.products = products);
  }

  onPageChange(page: number) {
    this.page$.next(page);
  }
}

Answer 2

Is there a better way of doing this?

I wouldn't say this is a better way, it's just personal preference. With this approach you can avoid the explicit subscription in your component:

product.service.ts

class ProductService {
  private listSrc = new Subject();
  private path = '/api';

  // Available for data consumers
  list$ = this.listSrc.pipe(
    mergeMap(
      criteria => this.http.post(this.path, criteria)
        .pipe(
          map(/* ... */),
          // You can also handle errors here
          // catchError()
        )
    ),
    // Might want to add a multicast operator in case `list$` is used in multiple places in the template
  );

  getAll (criteria) {
    this.listSrc.next(criteria);
  }
}

app.component.ts

class ProductComponent {

  get list$ () {
    return this.productService.list$;
  }

  constructor (/* ... */) { }

  ngOnInit() {
    this.loadProducts();
  }

  loadProducts () {
    this.productService.getAll({page: this.page});
  }
}

And now you can consume list$ with the async pipe.

Does this fail, if I would load two lists with different criterias in the Controller? If yes, how can I fix this?

In that case, if that list is still related to products, I'd just add another property in the productService, that follows the same pattern:

anotherProp$ = this.anotherPropSrc.pipe(
  /* ... */
)

Answer 3

I think what you're doing is correct. If you use async pipe to subscribe to list$ it will unsubscribe automatically. You probably don't need to unsubscribe from getAll() because it makes a single HTTP request but if you want to make super sure nothing is left pending after you destroy the component you should keep the subscription in a property and unsubscribe in ngOnDestroy().

You could maybe put everything into a single chain to avoid that:

private refresh$ = new Subject();
public list$ = this.refresh$
  .pipe(
    switchMap(criteria => this.http.post<any>('/api', criteria)),
    share(),
  );

...

getAll(criteria: any) {
  this.refresh$.next(criteria);
}