Skip to content

Commit 7630234

Browse files
josephperrottAndrewKushnir
authored andcommitted
fix(router): prevent calling unsubscribe on undefined subscription in RouterPreloader (angular#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 angular#38344
1 parent ba175be commit 7630234

File tree

2 files changed

+21
-6
lines changed

2 files changed

+21
-6
lines changed

packages/router/src/router_preloader.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@ export class NoPreloading implements PreloadingStrategy {
7474
@Injectable()
7575
export class RouterPreloader implements OnDestroy {
7676
private loader: RouterConfigLoader;
77-
// TODO(issue/24571): remove '!'.
78-
private subscription!: Subscription;
77+
private subscription?: Subscription;
7978

8079
constructor(
8180
private router: Router, moduleLoader: NgModuleFactoryLoader, compiler: Compiler,
@@ -98,11 +97,10 @@ export class RouterPreloader implements OnDestroy {
9897
return this.processRoutes(ngModule, this.router.config);
9998
}
10099

101-
// TODO(jasonaden): This class relies on code external to the class to call setUpPreloading. If
102-
// this hasn't been done, ngOnDestroy will fail as this.subscription will be undefined. This
103-
// should be refactored.
104100
ngOnDestroy(): void {
105-
this.subscription.unsubscribe();
101+
if (this.subscription) {
102+
this.subscription.unsubscribe();
103+
}
106104
}
107105

108106
private processRoutes(ngModule: NgModuleRef<any>, routes: Routes): Observable<void> {

packages/router/test/router_preloader.spec.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,23 @@ describe('RouterPreloader', () => {
1919
class LazyLoadedCmp {
2020
}
2121

22+
describe('should properly handle', () => {
23+
beforeEach(() => {
24+
TestBed.configureTestingModule({
25+
imports: [RouterTestingModule.withRoutes(
26+
[{path: 'lazy', loadChildren: 'expected', canLoad: ['someGuard']}])],
27+
providers: [{provide: PreloadingStrategy, useExisting: PreloadAllModules}]
28+
});
29+
});
30+
31+
it('being destroyed before expected', () => {
32+
const preloader: RouterPreloader = TestBed.get(RouterPreloader);
33+
// Calling the RouterPreloader's ngOnDestroy method is done to simulate what would happen if
34+
// the containing NgModule is destroyed.
35+
expect(() => preloader.ngOnDestroy()).not.toThrow();
36+
});
37+
});
38+
2239
describe('should not load configurations with canLoad guard', () => {
2340
@NgModule({
2441
declarations: [LazyLoadedCmp],

0 commit comments

Comments
 (0)