From 763023472b15f82a7551c9c4f8f43cda1d259dd5 Mon Sep 17 00:00:00 2001
From: Joey Perrott <josephperrott@gmail.com>
Date: Tue, 4 Aug 2020 10:56:39 -0700
Subject: [PATCH] fix(router): prevent calling unsubscribe on undefined
 subscription in RouterPreloader (#38344)

Previously, the `ngOnDestroy` method called `unsubscribe` regardless of if `subscription` had
been initialized.  This can lead to an error attempting to call `unsubscribe` of undefined.
This change prevents this error, and instead only attempts `unsubscribe` when the subscription
has been defined.

PR Close #38344
---
 packages/router/src/router_preloader.ts       | 10 ++++------
 packages/router/test/router_preloader.spec.ts | 17 +++++++++++++++++
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/packages/router/src/router_preloader.ts b/packages/router/src/router_preloader.ts
index d5e7670967..6b6da836bb 100644
--- a/packages/router/src/router_preloader.ts
+++ b/packages/router/src/router_preloader.ts
@@ -74,8 +74,7 @@ export class NoPreloading implements PreloadingStrategy {
 @Injectable()
 export class RouterPreloader implements OnDestroy {
   private loader: RouterConfigLoader;
-  // TODO(issue/24571): remove '!'.
-  private subscription!: Subscription;
+  private subscription?: Subscription;
 
   constructor(
       private router: Router, moduleLoader: NgModuleFactoryLoader, compiler: Compiler,
@@ -98,11 +97,10 @@ export class RouterPreloader implements OnDestroy {
     return this.processRoutes(ngModule, this.router.config);
   }
 
-  // TODO(jasonaden): This class relies on code external to the class to call setUpPreloading. If
-  // this hasn't been done, ngOnDestroy will fail as this.subscription will be undefined. This
-  // should be refactored.
   ngOnDestroy(): void {
-    this.subscription.unsubscribe();
+    if (this.subscription) {
+      this.subscription.unsubscribe();
+    }
   }
 
   private processRoutes(ngModule: NgModuleRef<any>, routes: Routes): Observable<void> {
diff --git a/packages/router/test/router_preloader.spec.ts b/packages/router/test/router_preloader.spec.ts
index a64777e6b9..f186e1796b 100644
--- a/packages/router/test/router_preloader.spec.ts
+++ b/packages/router/test/router_preloader.spec.ts
@@ -19,6 +19,23 @@ describe('RouterPreloader', () => {
   class LazyLoadedCmp {
   }
 
+  describe('should properly handle', () => {
+    beforeEach(() => {
+      TestBed.configureTestingModule({
+        imports: [RouterTestingModule.withRoutes(
+            [{path: 'lazy', loadChildren: 'expected', canLoad: ['someGuard']}])],
+        providers: [{provide: PreloadingStrategy, useExisting: PreloadAllModules}]
+      });
+    });
+
+    it('being destroyed before expected', () => {
+      const preloader: RouterPreloader = TestBed.get(RouterPreloader);
+      // Calling the RouterPreloader's ngOnDestroy method is done to simulate what would happen if
+      // the containing NgModule is destroyed.
+      expect(() => preloader.ngOnDestroy()).not.toThrow();
+    });
+  });
+
   describe('should not load configurations with canLoad guard', () => {
     @NgModule({
       declarations: [LazyLoadedCmp],