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
angular · Oct 26, 2015 · 2e059dc · 2e059dc
1 parent 2674eac
commit 2e059dc
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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