fix(router-plugin): cleanup subscriptions when the root view is destr…

…oyed (ngxs#1754)

CHANGELOG.md

@@ -8,6 +8,7 @@ $ npm install @ngxs/store@dev
 
 - Fix: Do not re-use the global `Store` instance between different apps [#1740](https://github.com/ngxs/store/pull/1740)
 - Fix: Do not run `Promise.then` within synchronous tests when decorating factory [#1753](https://github.com/ngxs/store/pull/1753)
+- Fix: Router Plugin - Cleanup subscriptions when the root view is destroyed [#1754](https://github.com/ngxs/store/pull/1754)
 - Performance: Tree-shake errors and warnings [#1732](https://github.com/ngxs/store/pull/1732)
 - Performance: Router Plugin - Tree-shake `isAngularInTestMode()` [#1738](https://github.com/ngxs/store/pull/1738)
 - Performance: Tree-shake `isAngularInTestMode()` [#1739](https://github.com/ngxs/store/pull/1739)

bundlesize.config.json

@@ -102,7 +102,7 @@
       "path": "./@ngxs/router-plugin/fesm2015/ngxs-router-plugin.js",
       "package": "@ngxs/router-plugin",
       "target": "es2015",
-      "maxSize": "21.525KB",
+      "maxSize": "21.55KB",
       "compression": "none"
     },
     {

packages/router-plugin/src/router.state.ts

@@ -1,4 +1,4 @@
-import { NgZone, Injectable } from '@angular/core';
+import { NgZone, Injectable, OnDestroy } from '@angular/core';
 import {
   NavigationCancel,
   NavigationError,
@@ -13,6 +13,7 @@ import {
 import { LocationStrategy, Location } from '@angular/common';
 import { Action, Selector, State, StateContext, Store } from '@ngxs/store';
 import { isAngularInTestMode } from '@ngxs/store/internals';
+import { Subscription } from 'rxjs';
 import { first } from 'rxjs/operators';
 
 import {
@@ -48,7 +49,7 @@ declare const ngDevMode: boolean;
   }
 })
 @Injectable()
-export class RouterState {
+export class RouterState implements OnDestroy {
   /**
    * Determines how navigation was performed by the `RouterState` itself
    * or outside via `new Navigate(...)`
@@ -67,6 +68,8 @@ export class RouterState {
 
   private _lastRoutesRecognized: RoutesRecognized = null!;
 
+  private _subscription = new Subscription();
+
   @Selector()
   static state<T = RouterStateSnapshot>(state: RouterStateModel<T>) {
     return state && state.state;
@@ -91,6 +94,10 @@ export class RouterState {
     this.checkInitialNavigationOnce();
   }
 
+  ngOnDestroy(): void {
+    this._subscription.unsubscribe();
+  }
+
   @Action(Navigate)
   navigate(_: StateContext<RouterStateModel>, action: Navigate) {
     return this._ngZone.run(() =>
@@ -115,13 +122,17 @@ export class RouterState {
   }
 
   private setUpStoreListener(): void {
-    this._store.select(RouterState).subscribe((state: RouterStateModel | undefined) => {
-      this.navigateIfNeeded(state);
-    });
+    const subscription = this._store
+      .select(RouterState)
+      .subscribe((state: RouterStateModel | undefined) => {
+        this.navigateIfNeeded(state);
+      });
+
+    this._subscription.add(subscription);
   }
 
   private setUpRouterEventsListener(): void {
-    this._router.events.subscribe(event => {
+    const subscription = this._router.events.subscribe(event => {
       if (event instanceof NavigationStart) {
         this.navigationStart();
       } else if (event instanceof RoutesRecognized) {
@@ -139,6 +150,8 @@ export class RouterState {
         this.reset();
       }
     });
+
+    this._subscription.add(subscription);
   }
 
   private navigationStart(): void {
@@ -250,7 +263,7 @@ export class RouterState {
       return;
     }
 
-    this._router.events
+    const subscription = this._router.events
       .pipe(first((event): event is RoutesRecognized => event instanceof RoutesRecognized))
       .subscribe(({ url }) => {
         // `location.pathname` always equals manually entered URL in the address bar
@@ -282,5 +295,7 @@ export class RouterState {
           this._router.navigateByUrl(currentUrl);
         }
       });
+
+    this._subscription.add(subscription);
   }
 }

packages/router-plugin/tests/router-state-cleanup.spec.ts

@@ -0,0 +1,55 @@
+import { APP_BASE_HREF } from '@angular/common';
+import { Component, NgModule } from '@angular/core';
+import { BrowserModule } from '@angular/platform-browser';
+import { Router, RouterEvent, RouterModule } from '@angular/router';
+import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';
+
+import { Subject } from 'rxjs';
+import { NgxsModule } from '@ngxs/store';
+import { freshPlatform } from '@ngxs/store/internals/testing';
+
+import { NgxsRouterPluginModule, RouterState } from '../';
+
+describe('RouterState cleanup', () => {
+  @Component({
+    selector: 'app-root',
+    template: ''
+  })
+  class TestComponent {}
+
+  @NgModule({
+    imports: [
+      BrowserModule,
+      RouterModule.forRoot([], { initialNavigation: false }),
+      NgxsModule.forRoot([]),
+      NgxsRouterPluginModule.forRoot()
+    ],
+    declarations: [TestComponent],
+    providers: [{ provide: APP_BASE_HREF, useValue: '/' }],
+    bootstrap: [TestComponent]
+  })
+  class TestModule {}
+
+  it(
+    'should cleanup subscriptions when the root view is destroyed',
+    freshPlatform(async () => {
+      // Arrange & act
+      const ngModuleRef = await platformBrowserDynamic().bootstrapModule(TestModule);
+
+      const router = ngModuleRef.injector.get(Router);
+      const events = router.events as Subject<RouterEvent>;
+      const routerState = ngModuleRef.injector.get(RouterState);
+      const spy = jest.spyOn(routerState, 'ngOnDestroy');
+
+      try {
+        // Assert
+        expect(events.observers.length).toBeGreaterThan(0);
+        ngModuleRef.destroy();
+        expect(spy).toHaveBeenCalled();
+        expect(events.observers.length).toEqual(0);
+      } finally {
+        spy.mockRestore();
+      }
+    })
+  );
+});