Skip to content

Commit

Permalink
refactor: use ReplaySubject for take until notifiers (#2055)
Browse files Browse the repository at this point in the history
This commit updates all `destroy$` subjects to be replayed subjects for
consistency across implementations. This change ensures that no more subscriptions
occur after any injectee is destroyed.
  • Loading branch information
arturovt authored Sep 16, 2023
1 parent 4a74d16 commit db96f65
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 18 deletions.
4 changes: 2 additions & 2 deletions packages/form-plugin/src/directive.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ChangeDetectorRef, Directive, Input, OnDestroy, OnInit } from '@angular/core';
import { FormGroup, FormGroupDirective } from '@angular/forms';
import { Actions, getValue, ofActionDispatched, Store } from '@ngxs/store';
import { Observable, Subject } from 'rxjs';
import { Observable, ReplaySubject } from 'rxjs';
import { debounceTime, distinctUntilChanged, filter, takeUntil } from 'rxjs/operators';
import {
ResetForm,
Expand Down Expand Up @@ -37,7 +37,7 @@ export class NgxsFormDirective implements OnInit, OnDestroy {

private _updating = false;

private readonly _destroy$ = new Subject<void>();
private readonly _destroy$ = new ReplaySubject<void>(1);

constructor(
private _actions$: Actions,
Expand Down
25 changes: 11 additions & 14 deletions packages/router-plugin/src/router.state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
NgxsRouterPluginOptions,
ɵNGXS_ROUTER_PLUGIN_OPTIONS
} from '@ngxs/router-plugin/internals';
import { Subscription } from 'rxjs';
import { ReplaySubject } from 'rxjs';
import { takeUntil } from 'rxjs/operators';

import {
Navigate,
Expand Down Expand Up @@ -71,10 +72,10 @@ export class RouterState implements OnDestroy {

private _lastEvent: Event | null = null;

private _subscription = new Subscription();

private _options: NgxsRouterPluginOptions | null = null;

private _destroy$ = new ReplaySubject<void>(1);

@Selector()
static state<T = RouterStateSnapshot>(state: RouterStateModel<T>) {
return state && state.state;
Expand All @@ -100,7 +101,7 @@ export class RouterState implements OnDestroy {
}

ngOnDestroy(): void {
this._subscription.unsubscribe();
this._destroy$.next();
}

@Action(Navigate)
Expand Down Expand Up @@ -133,13 +134,10 @@ export class RouterState implements OnDestroy {
}

private _setUpStoreListener(): void {
const subscription = this._store
.select(RouterState)
.subscribe((state: RouterStateModel | undefined) => {
this._navigateIfNeeded(state);
});

this._subscription.add(subscription);
const routerState$ = this._store.select(RouterState).pipe(takeUntil(this._destroy$));
routerState$.subscribe((state: RouterStateModel | undefined) => {
this._navigateIfNeeded(state);
});
}

private _navigateIfNeeded(routerState: RouterStateModel | undefined): void {
Expand Down Expand Up @@ -171,7 +169,8 @@ export class RouterState implements OnDestroy {

let lastRoutesRecognized: RoutesRecognized;

const subscription = this._router.events.subscribe(event => {
const events$ = this._router.events.pipe(takeUntil(this._destroy$));
events$.subscribe(event => {
this._lastEvent = event;

if (event instanceof NavigationStart) {
Expand Down Expand Up @@ -199,8 +198,6 @@ export class RouterState implements OnDestroy {
this._reset();
}
});

this._subscription.add(subscription);
}

/** Reacts to `NavigationStart`. */
Expand Down
4 changes: 2 additions & 2 deletions packages/store/src/internal/lifecycle-state-manager.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Injectable, OnDestroy } from '@angular/core';
import { NgxsBootstrapper } from '@ngxs/store/internals';
import { EMPTY, Subject } from 'rxjs';
import { EMPTY, ReplaySubject } from 'rxjs';
import {
catchError,
filter,
Expand All @@ -21,7 +21,7 @@ import { NgxsLifeCycle, NgxsSimpleChange, StateContext } from '../symbols';

@Injectable({ providedIn: 'root' })
export class LifecycleStateManager implements OnDestroy {
private readonly _destroy$ = new Subject<void>();
private readonly _destroy$ = new ReplaySubject<void>(1);

constructor(
private _store: Store,
Expand Down

0 comments on commit db96f65

Please sign in to comment.