From c29ab86d857556f5549e46780694963574f8a322 Mon Sep 17 00:00:00 2001 From: Brian Ford Date: Thu, 14 May 2015 13:01:48 -0700 Subject: [PATCH] refactor(router): improve control flow of descendant route activation --- modules/angular2/src/router/instruction.js | 48 ++++---- modules/angular2/src/router/pipeline.js | 9 -- modules/angular2/src/router/router.js | 111 +++++++++++------- modules/angular2/src/router/router_outlet.js | 45 +++++-- .../test/router/route_registry_spec.js | 10 +- 5 files changed, 134 insertions(+), 89 deletions(-) diff --git a/modules/angular2/src/router/instruction.js b/modules/angular2/src/router/instruction.js index a7d8a10c53..3c7d98559b 100644 --- a/modules/angular2/src/router/instruction.js +++ b/modules/angular2/src/router/instruction.js @@ -16,26 +16,32 @@ export class RouteParams { export class Instruction { component:any; - _children:StringMap; - router:any; - matchedUrl:string; - params:StringMap; + _children:Map; + + // the part of the URL captured by this instruction + capturedUrl:string; + + // the part of the URL captured by this instruction and all children + accumulatedUrl:string; + + params:Map; reuse:boolean; cost:number; constructor({params, component, children, matchedUrl, parentCost}:{params:StringMap, component:any, children:StringMap, matchedUrl:string, cost:number} = {}) { this.reuse = false; - this.matchedUrl = matchedUrl; + this.capturedUrl = matchedUrl; + this.accumulatedUrl = matchedUrl; this.cost = parentCost; if (isPresent(children)) { this._children = children; var childUrl; StringMapWrapper.forEach(this._children, (child, _) => { - childUrl = child.matchedUrl; + childUrl = child.accumulatedUrl; this.cost += child.cost; }); if (isPresent(childUrl)) { - this.matchedUrl += childUrl; + this.accumulatedUrl += childUrl; } } else { this._children = StringMapWrapper.create(); @@ -44,7 +50,11 @@ export class Instruction { this.params = params; } - getChildInstruction(outletName:string): Instruction { + hasChild(outletName:string):Instruction { + return StringMapWrapper.contains(this._children, outletName); + } + + getChild(outletName:string):Instruction { return StringMapWrapper.get(this._children, outletName); } @@ -52,37 +62,23 @@ export class Instruction { StringMapWrapper.forEach(this._children, fn); } - mapChildrenAsync(fn):Promise { - return mapObjAsync(this._children, fn); - } - /** * Does a synchronous, breadth-first traversal of the graph of instructions. * Takes a function with signature: * (parent:Instruction, child:Instruction) => {} */ traverseSync(fn:Function): void { - this.forEachChild((childInstruction, _) => fn(this, childInstruction)); + this.forEachChild(fn); this.forEachChild((childInstruction, _) => childInstruction.traverseSync(fn)); } - /** - * Does an asynchronous, breadth-first traversal of the graph of instructions. - * Takes a function with signature: - * (child:Instruction, parentOutletName:string) => {} - */ - traverseAsync(fn:Function):Promise { - return this.mapChildrenAsync(fn) - .then((_) => this.mapChildrenAsync((childInstruction, _) => childInstruction.traverseAsync(fn))); - } - /** * Takes a currently active instruction and sets a reuse flag on this instruction */ reuseComponentsFrom(oldInstruction:Instruction): void { - this.forEachChild((childInstruction, outletName) => { - var oldInstructionChild = oldInstruction.getChildInstruction(outletName); + this.traverseSync((childInstruction, outletName) => { + var oldInstructionChild = oldInstruction.getChild(outletName); if (shouldReuseComponent(childInstruction, oldInstructionChild)) { childInstruction.reuse = true; } @@ -104,5 +100,3 @@ function mapObj(obj:StringMap, fn: Function):List { StringMapWrapper.forEach(obj, (value, key) => ListWrapper.push(result, fn(value, key))); return result; } - -export var noopInstruction = new Instruction(); diff --git a/modules/angular2/src/router/pipeline.js b/modules/angular2/src/router/pipeline.js index ab2363ecac..f199044b4d 100644 --- a/modules/angular2/src/router/pipeline.js +++ b/modules/angular2/src/router/pipeline.js @@ -11,15 +11,6 @@ export class Pipeline { constructor() { this.steps = [ - instruction => instruction.traverseSync((parentInstruction, childInstruction) => { - childInstruction.router = parentInstruction.router.childRouter(childInstruction.component); - }), - instruction => instruction.router.traverseOutlets((outlet, name) => { - return outlet.canDeactivate(instruction.getChildInstruction(name)); - }), - instruction => instruction.router.traverseOutlets((outlet, name) => { - return outlet.canActivate(instruction.getChildInstruction(name)); - }), instruction => instruction.router.activateOutlets(instruction) ]; } diff --git a/modules/angular2/src/router/router.js b/modules/angular2/src/router/router.js index de614f98cb..12c515cd6c 100644 --- a/modules/angular2/src/router/router.js +++ b/modules/angular2/src/router/router.js @@ -15,6 +15,15 @@ import {Location} from './location'; * You can see the state of the router by inspecting the read-only field `router.navigating`. * This may be useful for showing a spinner, for instance. * + * ## Concepts + * Routers and component instances have a 1:1 correspondence. + * + * The router holds reference to a number of "outlets." An outlet is a placeholder that the + * router dynamically fills in depending on the current URL. + * + * When the router navigates from a URL, it must first recognizes it and serialize it into an `Instruction`. + * The router uses the `RouteRegistry` to get an `Instruction`. + * * @exportedAs angular2/router */ export class Router { @@ -28,19 +37,16 @@ export class Router { _pipeline:Pipeline; _registry:RouteRegistry; - _outlets:Map; - _children:Map; + _outlets:Map; _subject:EventEmitter; - _location:Location; - constructor(registry:RouteRegistry, pipeline:Pipeline, location:Location, parent:Router, hostComponent) { + + constructor(registry:RouteRegistry, pipeline:Pipeline, parent:Router, hostComponent:any) { this.hostComponent = hostComponent; this.navigating = false; this.parent = parent; this.previousUrl = null; this._outlets = MapWrapper.create(); - this._children = MapWrapper.create(); - this._location = location; this._registry = registry; this._pipeline = pipeline; this._subject = new EventEmitter(); @@ -51,25 +57,18 @@ export class Router { /** * Constructs a child router. You probably don't need to use this unless you're writing a reusable component. */ - childRouter(outletName = 'default'): Router { - var router = MapWrapper.get(this._children, outletName); - - if (isBlank(router)) { - router = new ChildRouter(this, outletName); - MapWrapper.set(this._children, outletName, router); - } - - return router; + childRouter(hostComponent:any): Router { + return new ChildRouter(this, hostComponent); } /** * Register an object to notify of route changes. You probably don't need to use this unless you're writing a reusable component. */ - registerOutlet(outlet:RouterOutlet, name: string = 'default'):Promise { + registerOutlet(outlet:RouterOutlet, name: string = 'default'): Promise { MapWrapper.set(this._outlets, name, outlet); if (isPresent(this._currentInstruction)) { - var childInstruction = this._currentInstruction.getChildInstruction(name); + var childInstruction = this._currentInstruction.getChild(name); return outlet.activate(childInstruction); } return PromiseWrapper.resolve(true); @@ -77,7 +76,7 @@ export class Router { /** - * Update the routing configuration and trigger a navigation. + * Dynamically update the routing configuration and trigger a navigation. * * # Usage * @@ -98,7 +97,6 @@ export class Router { config(config:any): Promise { if (config instanceof List) { config.forEach((configObject) => { - // TODO: this is a hack this._registry.config(this.hostComponent, configObject); }); } else { @@ -110,6 +108,9 @@ export class Router { /** * Navigate to a URL. Returns a promise that resolves to the canonical URL for the route. + * + * If the given URL begins with a `/`, router will navigate absolutely. + * If the given URL does not begin with `/`, the router will navigate relative to this component. */ navigate(url:string):Promise { if (this.navigating) { @@ -124,19 +125,16 @@ export class Router { return PromiseWrapper.resolve(false); } - if(isPresent(this._currentInstruction)) { + if (isPresent(this._currentInstruction)) { matchedInstruction.reuseComponentsFrom(this._currentInstruction); } - matchedInstruction.router = this; this._startNavigating(); - var result = this._pipeline.process(matchedInstruction) + var result = this.commit(matchedInstruction) .then((_) => { - this._location.go(matchedInstruction.matchedUrl); - ObservableWrapper.callNext(this._subject, matchedInstruction.matchedUrl); + ObservableWrapper.callNext(this._subject, matchedInstruction.accumulatedUrl); this._finishNavigating(); - this._currentInstruction = matchedInstruction; }); PromiseWrapper.catchError(result, (_) => this._finishNavigating()); @@ -152,6 +150,7 @@ export class Router { this.navigating = false; } + /** * Subscribe to URL updates from the router */ @@ -160,25 +159,46 @@ export class Router { } - activateOutlets(instruction:Instruction):Promise { - return this._queryOutlets((outlet, name) => { - var childInstruction = instruction.getChildInstruction(name); - if (childInstruction.reuse) { - return PromiseWrapper.resolve(true); + /** + * + */ + commit(instruction:Instruction):Promise { + this._currentInstruction = instruction; + + // collect all outlets that do not have a corresponding child instruction + // and remove them from the internal map of child outlets + var toDeactivate = ListWrapper.create(); + MapWrapper.forEach(this._outlets, (outlet, outletName) => { + if (!instruction.hasChild(outletName)) { + MapWrapper.delete(this._outlets, outletName); + ListWrapper.push(toDeactivate, outlet); } - return outlet.activate(childInstruction); - }) - .then((_) => instruction.mapChildrenAsync((instruction, _) => { - return instruction.router.activateOutlets(instruction); - })); + }); + + return PromiseWrapper.all(ListWrapper.map(toDeactivate, (outlet) => outlet.deactivate())) + .then((_) => this.activate(instruction)); } - traverseOutlets(fn):Promise { - return this._queryOutlets(fn) - .then((_) => mapObjAsync(this._children, (child, _) => child.traverseOutlets(fn))); + + /** + * Recursively remove all components contained by this router's outlets. + * Calls deactivate hooks on all descendant components + */ + deactivate():Promise { + return this._eachOutletAsync((outlet) => outlet.deactivate); } - _queryOutlets(fn):Promise { + + /** + * Recursively activate. + * Calls the "activate" hook on descendant components. + */ + activate(instruction:Instruction):Promise { + return this._eachOutletAsync((outlet, name) => outlet.activate(instruction.getChild(name))); + } + + + _eachOutletAsync(fn):Promise { return mapObjAsync(this._outlets, fn); } @@ -212,17 +232,26 @@ export class Router { } export class RootRouter extends Router { + _location:Location; + constructor(registry:RouteRegistry, pipeline:Pipeline, location:Location, hostComponent:Type) { - super(registry, pipeline, location, null, hostComponent); + super(registry, pipeline, null, hostComponent); + this._location = location; this._location.subscribe((change) => this.navigate(change['url'])); this._registry.configFromComponent(hostComponent); this.navigate(location.path()); } + + commit(instruction):Promise { + return super.commit(instruction).then((_) => { + this._location.go(instruction.accumulatedUrl); + }); + } } class ChildRouter extends Router { constructor(parent:Router, hostComponent) { - super(parent._registry, parent._pipeline, parent._location, parent, hostComponent); + super(parent._registry, parent._pipeline, parent, hostComponent); this.parent = parent; } } diff --git a/modules/angular2/src/router/router_outlet.js b/modules/angular2/src/router/router_outlet.js index a0f95c0b89..3309153c12 100644 --- a/modules/angular2/src/router/router_outlet.js +++ b/modules/angular2/src/router/router_outlet.js @@ -1,5 +1,5 @@ import {Promise, PromiseWrapper} from 'angular2/src/facade/async'; -import {isBlank} from 'angular2/src/facade/lang'; +import {isBlank, isPresent} from 'angular2/src/facade/lang'; import {Directive} from 'angular2/src/core/annotations_impl/annotations'; import {Attribute} from 'angular2/src/core/annotations_impl/di'; @@ -9,43 +9,74 @@ import {Injector, bind} from 'angular2/di'; import * as routerMod from './router'; import {Instruction, RouteParams} from './instruction' + +/** + * A router outlet is a placeholder that Angular dynamically fills based on the application's route. + * + * ## Use + * + * ``` + * + * ``` + * + * Route outlets can also optionally have a name: + * + * ``` + * + * + * ``` + * + */ @Directive({ selector: 'router-outlet' }) export class RouterOutlet { _compiler:Compiler; _injector:Injector; - _router:routerMod.Router; + _parentRouter:routerMod.Router; + _childRouter:routerMod.Router; _viewContainer:ViewContainerRef; constructor(viewContainer:ViewContainerRef, compiler:Compiler, router:routerMod.Router, injector:Injector, @Attribute('name') nameAttr:String) { if (isBlank(nameAttr)) { nameAttr = 'default'; } - this._router = router; + this._parentRouter = router; + this._childRouter = null; this._viewContainer = viewContainer; this._compiler = compiler; this._injector = injector; - this._router.registerOutlet(this, nameAttr); + this._parentRouter.registerOutlet(this, nameAttr); } + /** + * Given an instruction, update the contents of this viewport. + */ activate(instruction:Instruction): Promise { + // if we're able to reuse the component, we just have to pass along the + if (instruction.reuse && isPresent(this._childRouter)) { + return this._childRouter.commit(instruction); + } return this._compiler.compileInHost(instruction.component).then((pv) => { + this._childRouter = this._parentRouter.childRouter(instruction.component); var outletInjector = this._injector.resolveAndCreateChild([ bind(RouteParams).toValue(new RouteParams(instruction.params)), - bind(routerMod.Router).toValue(instruction.router) + bind(routerMod.Router).toValue(this._childRouter) ]); this._viewContainer.clear(); this._viewContainer.create(pv, 0, null, outletInjector); + return this._childRouter.commit(instruction); }); } - canActivate(instruction:Instruction): Promise { - return PromiseWrapper.resolve(true); + deactivate():Promise { + return (isPresent(this._childRouter) ? this._childRouter.deactivate() : PromiseWrapper.resolve(true)) + .then((_) => this._viewContainer.clear()); } canDeactivate(instruction:Instruction): Promise { + // TODO: how to get ahold of the component instance here? return PromiseWrapper.resolve(true); } } diff --git a/modules/angular2/test/router/route_registry_spec.js b/modules/angular2/test/router/route_registry_spec.js index 813e908915..1b21bca264 100644 --- a/modules/angular2/test/router/route_registry_spec.js +++ b/modules/angular2/test/router/route_registry_spec.js @@ -24,7 +24,7 @@ export function main() { var instruction = registry.recognize('/test', rootHostComponent); - expect(instruction.getChildInstruction('default').component).toBe(DummyCompB); + expect(instruction.getChild('default').component).toBe(DummyCompB); }); it('should prefer static segments to dynamic', () => { @@ -33,7 +33,7 @@ export function main() { var instruction = registry.recognize('/home', rootHostComponent); - expect(instruction.getChildInstruction('default').component).toBe(DummyCompA); + expect(instruction.getChild('default').component).toBe(DummyCompA); }); it('should prefer dynamic segments to star', () => { @@ -42,7 +42,7 @@ export function main() { var instruction = registry.recognize('/home', rootHostComponent); - expect(instruction.getChildInstruction('default').component).toBe(DummyCompA); + expect(instruction.getChild('default').component).toBe(DummyCompA); }); it('should match the full URL recursively', () => { @@ -50,8 +50,8 @@ export function main() { var instruction = registry.recognize('/first/second', rootHostComponent); - var parentInstruction = instruction.getChildInstruction('default'); - var childInstruction = parentInstruction.getChildInstruction('default'); + var parentInstruction = instruction.getChild('default'); + var childInstruction = parentInstruction.getChild('default'); expect(parentInstruction.component).toBe(DummyParentComp); expect(childInstruction.component).toBe(DummyCompB);