Refresh data after redirect (go back)

OK, maybe this would be less error-prone if we did this instead:

fetchAllProducts(): Observable<Product[]> {
  return this.http.get<Product[]>(API_URL).pipe(
    share(),
    tap(prods => this.allProducts$.next(prods));
}

That should make it harder to accidentally leak subscriptions, but keep the same basic behavior of “whoever calls fetchAllProducts, it causes all subscribers to watchAllProducts to receive the result”.

A few notes:

  • you shouldn’t need to be passing id here, it would typically be included in the Product. The fewer arguments you have to a function (I do my best to avoid having more than one, if I can), the less you have to worry about putting them in the proper order.
  • do everything in your power to avoid casts (res as Product), and you really shouldn’t need one here, because…
  • PUT requests shouldn’t be returning anything, IMHO. I find following this rule helps me avoid accidentally breaking the idempotance part of the fundamental contract of PUT on the server side: you should be able to make the same PUT request 1, 5, or 1000 times in a row and end up in the same place.
  • I would not call fetchAllProducts in updateProduct, because it’s too heavy. Instead, I would manually walk through allProducts$.value (or keep it in a hash instead of an array) and update the local notion of allProducts$ via next.

So, I would do something more like this:

updateProduct(newb: Product): Observable<Product> {
  return this.http.put(URL).pipe(map() => {
    let allprods = clone(this.allProducts$.value) || [];
    let nprods = allprods.length;
    let foundit = false;
    for (let i = 0; i < nprods; ++i) {
      if (allprods[i].id === newb.id) {
        allprods.splice(i, 1, newb);
        foundit = true;
        break;
      }
    }
    if (!foundit) {
      allprods.push(newb);
    }
    this.allProducts$.next(allprods);
    return newb;
  });
}