From 2e059dc916a3320a5bf941272ef3f91dc2cf41b5 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Mon, 26 Oct 2015 11:01:02 -0700 Subject: [PATCH] feat(router): Make RootRouter disposable to allow cleanup of Location subscription. ROUTER_PROVIDERS now automatically disposes of the RootRouter when the application is disposed. Closes #4915 --- modules/angular2/router.ts | 18 ++++++++++-------- modules/angular2/src/router/router.ts | 12 ++++++++++-- .../integration/router_integration_spec.ts | 8 +++++++- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/modules/angular2/router.ts b/modules/angular2/router.ts index 2522c3ac85..0209117056 100644 --- a/modules/angular2/router.ts +++ b/modules/angular2/router.ts @@ -113,12 +113,12 @@ export const ROUTER_PROVIDERS: any[] = CONST_EXPR([ RouteRegistry, CONST_EXPR(new Provider(LocationStrategy, {useClass: PathLocationStrategy})), Location, - CONST_EXPR( - new Provider(Router, - { - useFactory: routerFactory, - deps: CONST_EXPR([RouteRegistry, Location, ROUTER_PRIMARY_COMPONENT]) - })), + CONST_EXPR(new Provider( + Router, + { + useFactory: routerFactory, + deps: CONST_EXPR([RouteRegistry, Location, ROUTER_PRIMARY_COMPONENT, ApplicationRef]) + })), CONST_EXPR(new Provider( ROUTER_PRIMARY_COMPONENT, {useFactory: routerPrimaryComponentFactory, deps: CONST_EXPR([ApplicationRef])})) @@ -129,8 +129,10 @@ export const ROUTER_PROVIDERS: any[] = CONST_EXPR([ */ export const ROUTER_BINDINGS = ROUTER_PROVIDERS; -function routerFactory(registry, location, primaryComponent) { - return new RootRouter(registry, location, primaryComponent); +function routerFactory(registry, location, primaryComponent, appRef) { + var rootRouter = new RootRouter(registry, location, primaryComponent); + appRef.registerDisposeListener(() => rootRouter.dispose()); + return rootRouter; } function routerPrimaryComponentFactory(app) { diff --git a/modules/angular2/src/router/router.ts b/modules/angular2/src/router/router.ts index 6dd66e5186..78753ede6a 100644 --- a/modules/angular2/src/router/router.ts +++ b/modules/angular2/src/router/router.ts @@ -467,12 +467,13 @@ export class Router { export class RootRouter extends Router { /** @internal */ _location: Location; + _locationSub: Object; constructor(registry: RouteRegistry, location: Location, primaryComponent: Type) { super(registry, null, primaryComponent); this._location = location; - this._location.subscribe((change) => - this.navigateByUrl(change['url'], isPresent(change['pop']))); + this._locationSub = this._location.subscribe( + (change) => this.navigateByUrl(change['url'], isPresent(change['pop']))); this.registry.configFromComponent(primaryComponent); this.navigateByUrl(location.path()); } @@ -489,6 +490,13 @@ export class RootRouter extends Router { } return promise; } + + dispose(): void { + if (isPresent(this._locationSub)) { + ObservableWrapper.dispose(this._locationSub); + this._locationSub = null; + } + } } class ChildRouter extends Router { diff --git a/modules/angular2/test/router/integration/router_integration_spec.ts b/modules/angular2/test/router/integration/router_integration_spec.ts index 523816a471..2c9d5b3de8 100644 --- a/modules/angular2/test/router/integration/router_integration_spec.ts +++ b/modules/angular2/test/router/integration/router_integration_spec.ts @@ -34,11 +34,17 @@ import { import {LocationStrategy} from 'angular2/src/router/location_strategy'; import {MockLocationStrategy} from 'angular2/src/mock/mock_location_strategy'; +import {ApplicationRef} from 'angular2/src/core/application_ref'; +import {MockApplicationRef} from 'angular2/src/mock/mock_application_ref'; export function main() { describe('router injectables', () => { beforeEachBindings(() => { - return [ROUTER_PROVIDERS, provide(LocationStrategy, {useClass: MockLocationStrategy})]; + return [ + ROUTER_PROVIDERS, + provide(LocationStrategy, {useClass: MockLocationStrategy}), + provide(ApplicationRef, {useClass: MockApplicationRef}) + ]; }); // do not refactor out the `bootstrap` functionality. We still want to