From ac80df09595644b5cc3fede854937cefdaaecf83 Mon Sep 17 00:00:00 2001 From: Brian Ford Date: Thu, 7 May 2015 21:10:12 -0700 Subject: [PATCH] fix(router): reuse common parent components --- modules/angular2/src/router/instruction.js | 33 ++++++++++++++- modules/angular2/src/router/router.js | 40 ++++++++++++------ modules/angular2/test/router/outlet_spec.js | 45 +++++++++++++++++++++ 3 files changed, 104 insertions(+), 14 deletions(-) diff --git a/modules/angular2/src/router/instruction.js b/modules/angular2/src/router/instruction.js index 71ef0a31ca..bc116987f9 100644 --- a/modules/angular2/src/router/instruction.js +++ b/modules/angular2/src/router/instruction.js @@ -19,8 +19,10 @@ export class Instruction { router:any; matchedUrl:string; params:Map; + reuse:boolean; constructor({params, component, children, matchedUrl}:{params:StringMap, component:any, children:Map, matchedUrl:string} = {}) { + this.reuse = false; this.matchedUrl = matchedUrl; if (isPresent(children)) { this._children = children; @@ -51,13 +53,42 @@ export class Instruction { } /** - * Takes a function: + * Does a synchronous, breadth-first traversal of the graph of instructions. + * Takes a function with signature: * (parent:Instruction, child:Instruction) => {} */ traverseSync(fn:Function) { this.forEachChild((childInstruction, _) => fn(this, childInstruction)); 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) { + 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) { + this.forEachChild((childInstruction, outletName) => { + var oldInstructionChild = oldInstruction.getChildInstruction(outletName); + if (shouldReuseComponent(childInstruction, oldInstructionChild)) { + childInstruction.reuse = true; + } + }); + } +} + +function shouldReuseComponent(instr1:Instruction, instr2:Instruction) { + return instr1.component == instr2.component && + StringMapWrapper.equals(instr1.params, instr2.params); } function mapObjAsync(obj:StringMap, fn) { diff --git a/modules/angular2/src/router/router.js b/modules/angular2/src/router/router.js index ebfcd9f5c4..0ec0da47e7 100644 --- a/modules/angular2/src/router/router.js +++ b/modules/angular2/src/router/router.js @@ -1,6 +1,6 @@ import {Promise, PromiseWrapper, EventEmitter, ObservableWrapper} from 'angular2/src/facade/async'; import {Map, MapWrapper, List, ListWrapper} from 'angular2/src/facade/collection'; -import {isBlank, Type} from 'angular2/src/facade/lang'; +import {isBlank, isPresent, Type} from 'angular2/src/facade/lang'; import {RouteRegistry} from './route_registry'; import {Pipeline} from './pipeline'; @@ -24,6 +24,8 @@ export class Router { lastNavigationAttempt: string; previousUrl:string; + _currentInstruction:Instruction; + _pipeline:Pipeline; _registry:RouteRegistry; _outlets:Map; @@ -42,6 +44,7 @@ export class Router { this._registry = registry; this._pipeline = pipeline; this._subject = new EventEmitter(); + this._currentInstruction = null; } @@ -61,7 +64,11 @@ export class Router { */ registerOutlet(outlet:RouterOutlet, name = 'default'):Promise { MapWrapper.set(this._outlets, name, outlet); - return this.renavigate(); + if (isPresent(this._currentInstruction)) { + var childInstruction = this._currentInstruction.getChildInstruction(name); + return outlet.activate(childInstruction); + } + return PromiseWrapper.resolve(true); } @@ -107,23 +114,26 @@ export class Router { this.lastNavigationAttempt = url; - var instruction = this.recognize(url); + var matchedInstruction = this.recognize(url); - if (isBlank(instruction)) { + if (isBlank(matchedInstruction)) { return PromiseWrapper.resolve(false); } - instruction.router = this; + if(isPresent(this._currentInstruction)) { + matchedInstruction.reuseComponentsFrom(this._currentInstruction); + } + + matchedInstruction.router = this; this._startNavigating(); - var result = this._pipeline.process(instruction) + var result = this._pipeline.process(matchedInstruction) .then((_) => { - this._location.go(instruction.matchedUrl); - }) - .then((_) => { - ObservableWrapper.callNext(this._subject, instruction.matchedUrl); - }) - .then((_) => this._finishNavigating()); + this._location.go(matchedInstruction.matchedUrl); + ObservableWrapper.callNext(this._subject, matchedInstruction.matchedUrl); + this._finishNavigating(); + this._currentInstruction = matchedInstruction; + }); PromiseWrapper.catchError(result, (_) => this._finishNavigating()); @@ -148,7 +158,11 @@ export class Router { activateOutlets(instruction:Instruction):Promise { return this._queryOutlets((outlet, name) => { - return outlet.activate(instruction.getChildInstruction(name)); + var childInstruction = instruction.getChildInstruction(name); + if (childInstruction.reuse) { + return PromiseWrapper.resolve(true); + } + return outlet.activate(childInstruction); }) .then((_) => instruction.mapChildrenAsync((instruction, _) => { return instruction.router.activateOutlets(instruction); diff --git a/modules/angular2/test/router/outlet_spec.js b/modules/angular2/test/router/outlet_spec.js index 6e95f18bbb..8d21b65b11 100644 --- a/modules/angular2/test/router/outlet_spec.js +++ b/modules/angular2/test/router/outlet_spec.js @@ -31,6 +31,8 @@ import {Location} from 'angular2/src/router/location'; import {RouteRegistry} from 'angular2/src/router/route_registry'; import {DirectiveMetadataReader} from 'angular2/src/core/compiler/directive_metadata_reader'; +var teamCmpCount; + export function main() { describe('Outlet Directive', () => { @@ -51,6 +53,7 @@ export function main() { ctx = new MyComp(); rtr = router; location = loc; + teamCmpCount = 0; })); function compile(template:string = "") { @@ -132,6 +135,7 @@ export function main() { }); })); + it('should generate link hrefs without params', inject([AsyncTestCompleter], (async) => { compile('') .then((_) => rtr.config({'path': '/user', 'component': UserCmp, 'as': 'user'})) @@ -143,6 +147,26 @@ export function main() { }); })); + + it('should reuse common parent components', inject([AsyncTestCompleter, Location], (async, location) => { + compile() + .then((_) => rtr.config({'path': '/team/:id', 'component': TeamCmp })) + .then((_) => rtr.navigate('/team/angular/user/rado')) + .then((_) => { + view.detectChanges(); + expect(teamCmpCount).toBe(1); + expect(view.rootNodes).toHaveText('team angular { hello rado }'); + }) + .then((_) => rtr.navigate('/team/angular/user/victor')) + .then((_) => { + view.detectChanges(); + expect(teamCmpCount).toBe(1); + expect(view.rootNodes).toHaveText('team angular { hello victor }'); + async.done(); + }); + })); + + it('should generate link hrefs with params', inject([AsyncTestCompleter], (async) => { ctx.name = 'brian'; compile('{{name}}') @@ -242,6 +266,27 @@ class ParentCmp { constructor() {} } + +@Component({ + selector: 'team-cmp' +}) +@View({ + template: "team {{id}} { }", + directives: [RouterOutlet] +}) +@RouteConfig([{ + path: '/user/:name', + component: UserCmp +}]) +class TeamCmp { + id:string; + constructor(params:RouteParams) { + this.id = params.get('id'); + teamCmpCount += 1; + } +} + + @Component({ selector: 'my-comp' })