How to fix subscription leak Angular ngrx store

Question

I have a ponyracer app, which implements ngrx store. Very basic functionality: start race, add pony, remove pony, table of scores. The issue comes when I remove or add pony - basically this functionality refers to creating or destroying a pony component. So any added Pony stores somewhere for some reason, and even after destroying a component - app still has an access to all added and deleted ponies, and iterates them for example when I need to view scores only for existing ponies - but all (even previously removed) ponies are shown

I use store.select.pipe(...).subscribe(), and there is no need to use onDestroy and unsubscribe()

races.component.ts

export class RacesComponent implements OnInit {
    racesState: Observable<fromPonyRacer.State>;

    constructor(private store: Store<fromApp.AppState>) { }

    ngOnInit() {
        this.racesState = this.store.select("raceList");
        this.racesState.subscribe()
    }
}

races.component.html

<pr-race *ngFor="let race of (racesState | async).races; let i = index" 
         [race]="race" [index]="i"> 
</pr-race>

race.component.ts

export class RaceComponent implements OnInit {
    @Input() race: RaceModel;
    @Input() index: number;
    racesState: Observable<fromPonyRacer.State>;

    constructor(private store: Store<fromApp.AppState>) { }

    ngOnInit() {
    this.racesState = this.store.select("raceList");
    this.racesState.pipe(
        tap(races => {
            if(races.raceStatus) this.movePony();
        })
    )
    .subscribe()
    }
}

race.component.html

<div class="race">
    <img class="pony-img" [src]="race.src"> 
</div>

When a pony reaches finish, action is dispatched and pony is added to the store property called currentRaces, so store tracks which pony finished first, which is second and so on.

const initialState: State = {
    races: [
        new RaceModel(some data),
        new RaceModel(some another data),
        new RaceModel(some more data)
      ],
    raceStatus: false,
    raceCount: 0,
    isNewrace: false,
    poniesAreAboutToFinish: null,
    currentRaces: []
}

But this action is also dispatched for removed ponies, and they are also getting to the store.currentRaces and are shown in score table. And I cannot figure out where these removed ponies are coming from. Because state.races are always actual and correct (according to redux dev tools), and races.component takes state.races for rendering view for each pony, and it is always correct and fresh

movePony() {
    some code for moving a pony
      if(ponyReachedFinish) {
      this.store.dispatch(new PonyRacerActions.StopRace({name: this.race.name, place, points})
    }
}

Here you can find the whole code if more details are needed: https://github.com/joistick1/pr2

UPDATE: Issue solved. The problem occurred due subscription leak, as I assumed initially. I applied OnDestroy and in this method applied one line of code this.subscription.unsubscribe();

If you use subscribe on hot observables, why is that there is no need to use onDestroy and unsubscribe() ? This should be mandatory. — user4676340
I second @trichetriche. If you're subscribing to the observable from the store, you need to either be using the async pipe to subscribe, or explicitly call .unsubscribe() in your component. — Brandon
@trichetriche the fact the stream is hot isnt really relevant to the issue, whereas the stream being infinite is the cause of it. — Jota.Toledo
Yeah, even though subscribing to the Observable is a bad practice, it definitely isn't the cause of the bug. Post the code of your reducer where you remove a pony. — Christian
@Jota.Toledo Whether the async pipe subscribes to the Observable, or you do in the component will not change the value emitted. It's clear his error is somewhere in his reducers which actually determine his app's state. — Christian

Christian Christian · Accepted Answer · 2019-02-15T20:03:46

In your reducer when you delete a pony, you're directly mutating the state instead of returning new immutable state. Try applying the following changes:

case PonyRacerActions.DELETE_PONY:
            return {
                ...state,
                races: state.races.filter(race => race != action.payload),
                currentRaces: []
            }

How to fix subscription leak Angular ngrx store

1 Answers