Skip to content

Commit

Permalink
fix(router): error if module is destroyed before location is initiali…
Browse files Browse the repository at this point in the history
…zed (#42560)

This is something I ran into while working on a fix for the `TestBed` module teardown behavior for #18831. In the `RouterInitializer.appInitializer` we have a callback to the `LOCATION_INITIALIZED` which has to do some DI lookups. The problem is that if the module is destroyed before the location promise resolves, the `Injector.get` calls will fail. This is unlikely to happen in a real app, but it'll show up in unit tests once the test module teardown behavior is fixed.

PR Close #42560
  • Loading branch information
crisbeto authored and dylhunn committed Jun 17, 2021
1 parent 166e98a commit 07c1ddc
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 4 deletions.
16 changes: 13 additions & 3 deletions packages/router/src/router_module.ts
Expand Up @@ -7,7 +7,7 @@
*/

import {APP_BASE_HREF, HashLocationStrategy, Location, LOCATION_INITIALIZED, LocationStrategy, PathLocationStrategy, PlatformLocation, ViewportScroller} from '@angular/common';
import {ANALYZE_FOR_ENTRY_COMPONENTS, APP_BOOTSTRAP_LISTENER, APP_INITIALIZER, ApplicationRef, Compiler, ComponentRef, Inject, Injectable, InjectionToken, Injector, ModuleWithProviders, NgModule, NgModuleFactoryLoader, NgProbeToken, Optional, Provider, SkipSelf, SystemJsNgModuleLoader} from '@angular/core';
import {ANALYZE_FOR_ENTRY_COMPONENTS, APP_BOOTSTRAP_LISTENER, APP_INITIALIZER, ApplicationRef, Compiler, ComponentRef, Inject, Injectable, InjectionToken, Injector, ModuleWithProviders, NgModule, NgModuleFactoryLoader, NgProbeToken, OnDestroy, Optional, Provider, SkipSelf, SystemJsNgModuleLoader} from '@angular/core';
import {of, Subject} from 'rxjs';

import {EmptyOutletComponent} from './components/empty_outlet';
Expand Down Expand Up @@ -503,15 +503,21 @@ export function rootRoute(router: Router): ActivatedRoute {
* pauses. It waits for the hook to be resolved. We then resolve it only in a bootstrap listener.
*/
@Injectable()
export class RouterInitializer {
private initNavigation: boolean = false;
export class RouterInitializer implements OnDestroy {
private initNavigation = false;
private destroyed = false;
private resultOfPreactivationDone = new Subject<void>();

constructor(private injector: Injector) {}

appInitializer(): Promise<any> {
const p: Promise<any> = this.injector.get(LOCATION_INITIALIZED, Promise.resolve(null));
return p.then(() => {
// If the injector was destroyed, the DI lookups below will fail.
if (this.destroyed) {
return Promise.resolve(true);
}

let resolve: Function = null!;
const res = new Promise(r => resolve = r);
const router = this.injector.get(Router);
Expand Down Expand Up @@ -566,6 +572,10 @@ export class RouterInitializer {
this.resultOfPreactivationDone.next(null!);
this.resultOfPreactivationDone.complete();
}

ngOnDestroy() {
this.destroyed = true;
}
}

export function getAppInitializer(r: RouterInitializer) {
Expand Down
39 changes: 38 additions & 1 deletion packages/router/test/integration.spec.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {CommonModule, Location, LocationStrategy, PlatformLocation} from '@angular/common';
import {APP_BASE_HREF, CommonModule, Location, LOCATION_INITIALIZED, LocationStrategy, PlatformLocation} from '@angular/common';
import {SpyLocation} from '@angular/common/testing';
import {ChangeDetectionStrategy, Component, EventEmitter, Injectable, NgModule, NgModuleFactoryLoader, NgModuleRef, NgZone, OnDestroy, ViewChild, ɵConsole as Console, ɵNoopNgZone as NoopNgZone} from '@angular/core';
import {ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
Expand All @@ -16,6 +16,7 @@ import {ActivatedRoute, ActivatedRouteSnapshot, ActivationEnd, ActivationStart,
import {EMPTY, Observable, Observer, of, Subscription, SubscriptionLike} from 'rxjs';
import {delay, filter, first, map, mapTo, tap} from 'rxjs/operators';

import {RouterInitializer} from '../src/router_module';
import {forEach} from '../src/utils/collection';
import {RouterTestingModule, SpyNgModuleFactoryLoader} from '../testing';

Expand Down Expand Up @@ -6210,6 +6211,42 @@ describe('Integration', () => {
expect(fixture).toContainComponent(Tool2Component, '(e)');
}));
});

describe('RouterInitializer', () => {
it('should not throw from appInitializer if module is destroyed before location is initialized',
done => {
let resolveInitializer: () => void;
let moduleRef: NgModuleRef<SelfDestructModule>;

@NgModule({
imports: [RouterModule.forRoot([])],
providers: [
{
provide: LOCATION_INITIALIZED,
useValue: new Promise<void>(resolve => resolveInitializer = resolve)
},
{
// Required when running the tests in a browser
provide: APP_BASE_HREF,
useValue: ''
}
]
})
class SelfDestructModule {
constructor(ref: NgModuleRef<SelfDestructModule>, routerInitializer: RouterInitializer) {
moduleRef = ref;
routerInitializer.appInitializer().then(done, done.fail);
}
}

TestBed.resetTestingModule()
.configureTestingModule({imports: [SelfDestructModule], declarations: [SimpleCmp]})
.createComponent(SimpleCmp);

moduleRef!.destroy();
resolveInitializer!();
});
});
});

describe('Testing router options', () => {
Expand Down

0 comments on commit 07c1ddc

Please sign in to comment.