Subscription still live after the parameter changed

Question

I have a subscription to get parameters come with the route and in that subscription(after it success) I am calling another subscription from the same service to get message details. the problem is: when the parameter changed the subscription that related with old parameter still alive.

this.routes.params.subscribe(param => {
        this.isLoading = true;
        this.messageId = param['id'];
        this.inboxService.getMessageDetail(toNumber(this.messageId)).subscribe((dat) => {
          this.initMessageDetails(dat);
        })
      })

Unsubscribe to the subscription after use. – Drashti Dobariya Jun 25 '21 at 08:00 — Drashti Dobariya, Jun 25 '21 at 08:00

score 0 · Answer 1 · answered Jun 25 '21 at 08:01

0

This is the exact reason why nested subscriptions are discouraged. It leads to multiple subscriptions of which some might be left unclosed.

You need to use an higher order mapping operator like swithcMap to map from one observable to another and take operator with argument 1 to close the first observable after it's first emission.

import { take, switchMap } from 'rxjs/operators';

this.routes.params.pipe(
  take(1),                // <-- complete after first emission
  switchMap(param => {    // <-- map to another observable
    this.isLoading = true;
    this.messageId = param['id'];
    return this.inboxService.getMessageDetail(toNumber(this.messageId));
  }
).subscribe(
  (dat) => {
    this.initMessageDetails(dat);
  },
  (error: any) => {
    // handle errors
  }
);

answered Jun 25 '21 at 08:01

ruth

29,535
4
30
57

If the route params are susceptible to change and that change needs to be detected, you need to remove the `take(1)` logic. Also, we really should discourage putting logic in the subscribe callback on any Observable. Everything goes in the `pipe`! Finally, I'd discourage modifying the local variables inside the `switchMap` function: those should really go in a `tap` to keep things tidy and easier to maintain. – Will Alexander Jun 25 '21 at 08:05
@WillAlexander: Most points need explanation. Why not put logic in the subscription callback? It's cleaner compared to performing the logic in `tap` which is meant for side-effects. "_Everything goes in the `pipe`!_" - Why? Do you mean to say having an empty `subscribe` is always better? Why? Also most of the time the first emission from the route is enough since routing to the same page later will trigger an individual subscription with it's own params. – ruth Jun 25 '21 at 08:21
@MichaelD thank you that's working good, but another problem occurred – James.mikael Jun 25 '21 at 08:30
OP: The point from @WillAlexander about closing the subscription using `takeUntil` is also important to avoiding open subscriptions after the component is destroyed. Please see here for more info: https://stackoverflow.com/a/60223749/6513921 – ruth Jun 25 '21 at 08:31
but the comment from @WillAlexander solved it, thanks for you all – James.mikael Jun 25 '21 at 08:32
@James.mikael: Please post what the issue was and how it was solved. It might help someone in the future. – ruth Jun 25 '21 at 08:32
@MichaelD: the modification of local variables is a side effect of Observable emissions, so `tap` is the ideal tool. Having all logic in the `pipe` allows you to subscribe to the Observable using other means (e.g. the `async` pipe) or to pipe other Observables onto it while maintaining the same behaviour. `subscribe` callbacks aren't clean because there is no separation of responsibility, and it takes longer to read and understand what's going on. A well-built `pipe` using appropriate operators makes for very readable and modular code. – Will Alexander Jun 25 '21 at 09:05
@WillAlexander: Most things you said are opinionated and subjective. Ofcourse doing everything using operators is required when using `async`, but not here where there is an empty subscribe call. If your logic is to go by, then most operator definitions are in the wrong. Eg. accd. to you [`take`](https://github.com/ReactiveX/rxjs/blob/7.1.0/src/internal/operators/take.ts#L53) mustn't subscribe to the source but must pipe `concatMap` and return either using `of` or `EMPTY`? – ruth Jun 25 '21 at 09:13
I don't understand your question about the `take` operator. The whole point of leaving everything out of the `subscribe` callback is to leave code as flexible as possible. When requirements change, the place where you subscribe to the final Observable may well change, so it's best if it encapsulates all of its logic. There is of course the separate point of not using local variables at all and maintaining the Observable chain all the way to the view. "Opinionated and subjective" perhaps, but generally speaking a best practice. – Will Alexander Jun 25 '21 at 09:22
@MichaelD the main problem which I asked for: I have a component retrieves a parameter from route and display data depending on that parameter, but when I render the component using a parameter and in the same time click to render the same component with different parameter , the component was rendering using new parameter and then render using the old parameter. – James.mikael Jun 25 '21 at 09:38
@MichaelD after I modified my code like you wrote, new problem happened which is: the component renders inside or using router outlet and after editing the code the component couldn't be rendered with routerLink and I needed to reload the page to open the component with new parameter, I solved it by removing (take) operator like willAlexender commented – James.mikael Jun 25 '21 at 09:41
@James.mikael meaning no disrespect to our colleague who posted this response, may I suggest you try the code from my answer and, if it works, consider making it the accepted answer to your question? That way other users with the same problem get the correct code without having to read the comments. – Will Alexander Jun 28 '21 at 05:51

score 0 · Accepted Answer · answered Jun 25 '21 at 08:04

This is a perfect use case for the switchMap operator! It subscribes to the child Observable for every emission from the parent, unsubscribing from the previous instance of that child Observable.

Wherever possible, you should avoid putting logic in your subscribe callbacks, especially if it involves subscribing to another Observable. When using RxJS Observables, use their pipe operator:

this.routes.params.pipe(
  tap(params => {
    this.isLoading = true;
    this.messageId = params['id'];
  }),
  switchMap(params => this.inboxService.getMessageDetail(toNumber(this.messageId))),
  tap(dat => this.initMessageDetails(dat))
).subscribe();

You should also implement some kind of unsubscribe logic to make sure your Observable dies when the component does (using the takeUntil operator and a Subject which emits in ngOnDestroy(), for example).

Subscription still live after the parameter changed

2 Answers2