From 286a249a9a9bf5f9b9dbacd578d6cd92ec8dc440 Mon Sep 17 00:00:00 2001 From: Brian Ford Date: Mon, 6 Jul 2015 17:41:15 -0700 Subject: [PATCH] feat(router): support deep-linking to siblings Closes #2807 --- modules/angular2/router.ts | 3 +- modules/angular2/src/router/route_registry.ts | 52 ++---------- modules/angular2/src/router/router.ts | 80 +++++++++++++++++-- modules/angular2/src/router/router_link.ts | 5 +- modules/angular2/test/router/outlet_spec.ts | 75 ++++++++++++++++- .../test/router/route_registry_spec.ts | 42 +++------- modules/angular2/test/router/router_spec.ts | 33 +++++++- 7 files changed, 198 insertions(+), 92 deletions(-) diff --git a/modules/angular2/router.ts b/modules/angular2/router.ts index 9c7e605291..fecc2a7f34 100644 --- a/modules/angular2/router.ts +++ b/modules/angular2/router.ts @@ -34,8 +34,7 @@ import {List} from './src/facade/collection'; export const routerDirectives: List = CONST_EXPR([RouterOutlet, RouterLink]); export var routerInjectables: List = [ - bind(RouteRegistry) - .toFactory((appRoot) => new RouteRegistry(appRoot), [appComponentTypeToken]), + RouteRegistry, Pipeline, bind(LocationStrategy).toClass(HTML5LocationStrategy), Location, diff --git a/modules/angular2/src/router/route_registry.ts b/modules/angular2/src/router/route_registry.ts index 757e751631..bf10fcb1ac 100644 --- a/modules/angular2/src/router/route_registry.ts +++ b/modules/angular2/src/router/route_registry.ts @@ -32,8 +32,6 @@ import {Injectable} from 'angular2/di'; export class RouteRegistry { private _rules: Map = new Map(); - constructor(private _rootHostComponent: any) {} - /** * Given a component and a configuration object, add the route to this registry */ @@ -144,41 +142,22 @@ export class RouteRegistry { } /** - * Given a list with component names and params like: `['./user', {id: 3 }]` + * Given a normalized list with component names and params like: `['user', {id: 3 }]` * generates a url with a leading slash relative to the provided `parentComponent`. */ generate(linkParams: List, parentComponent): string { - let normalizedLinkParams = splitAndFlattenLinkParams(linkParams); - let url = '/'; - + let url = ''; let componentCursor = parentComponent; - - // The first segment should be either '.' (generate from parent) or '' (generate from root). - // When we normalize above, we strip all the slashes, './' becomes '.' and '/' becomes ''. - if (normalizedLinkParams[0] == '') { - componentCursor = this._rootHostComponent; - } else if (normalizedLinkParams[0] != '.') { - throw new BaseException( - `Link "${ListWrapper.toJSON(linkParams)}" must start with "/" or "./"`); - } - - if (normalizedLinkParams[normalizedLinkParams.length - 1] == '') { - ListWrapper.removeLast(normalizedLinkParams); - } - - if (normalizedLinkParams.length < 2) { - let msg = `Link "${ListWrapper.toJSON(linkParams)}" must include a route name.`; - throw new BaseException(msg); - } - - for (let i = 1; i < normalizedLinkParams.length; i += 1) { - let segment = normalizedLinkParams[i]; + for (let i = 0; i < linkParams.length; i += 1) { + let segment = linkParams[i]; if (!isString(segment)) { throw new BaseException(`Unexpected segment "${segment}" in link DSL. Expected a string.`); + } else if (segment == '' || segment == '.' || segment == '..') { + throw new BaseException(`"${segment}/" is only allowed at the beginning of a link DSL.`); } let params = null; - if (i + 1 < normalizedLinkParams.length) { - let nextSegment = normalizedLinkParams[i + 1]; + if (i + 1 < linkParams.length) { + let nextSegment = linkParams[i + 1]; if (isStringMap(nextSegment)) { params = nextSegment; i += 1; @@ -274,18 +253,3 @@ function assertTerminalComponent(component, path) { } } } - -/* - * Given: ['/a/b', {c: 2}] - * Returns: ['', 'a', 'b', {c: 2}] - */ -var SLASH = new RegExp('/'); -function splitAndFlattenLinkParams(linkParams: List): List { - return ListWrapper.reduce(linkParams, (accumulation, item) => { - if (isString(item)) { - return ListWrapper.concat(accumulation, StringWrapper.split(item, SLASH)); - } - accumulation.push(item); - return accumulation; - }, []); -} diff --git a/modules/angular2/src/router/router.ts b/modules/angular2/src/router/router.ts index 6d8b313231..d65bef4c23 100644 --- a/modules/angular2/src/router/router.ts +++ b/modules/angular2/src/router/router.ts @@ -1,6 +1,14 @@ import {Promise, PromiseWrapper, EventEmitter, ObservableWrapper} from 'angular2/src/facade/async'; import {Map, MapWrapper, List, ListWrapper} from 'angular2/src/facade/collection'; -import {isBlank, isPresent, Type, isArray} from 'angular2/src/facade/lang'; +import { + isBlank, + isString, + StringWrapper, + isPresent, + Type, + isArray, + BaseException +} from 'angular2/src/facade/lang'; import {RouteRegistry} from './route_registry'; import {Pipeline} from './pipeline'; @@ -42,7 +50,7 @@ export class Router { // todo(jeffbcross): rename _registry to registry since it is accessed from subclasses // todo(jeffbcross): rename _pipeline to pipeline since it is accessed from subclasses - constructor(public _registry: RouteRegistry, public _pipeline: Pipeline, public parent: Router, + constructor(public registry: RouteRegistry, public _pipeline: Pipeline, public parent: Router, public hostComponent: any) {} @@ -88,9 +96,9 @@ export class Router { config(config: StringMap| List>): Promise { if (isArray(config)) { (>config) - .forEach((configObject) => { this._registry.config(this.hostComponent, configObject); }); + .forEach((configObject) => { this.registry.config(this.hostComponent, configObject); }); } else { - this._registry.config(this.hostComponent, config); + this.registry.config(this.hostComponent, config); } return this.renavigate(); } @@ -170,7 +178,7 @@ export class Router { * Given a URL, returns an instruction representing the component graph */ recognize(url: string): Promise { - return this._registry.recognize(url, this.hostComponent); + return this.registry.recognize(url, this.hostComponent); } @@ -192,7 +200,48 @@ export class Router { * app's base href. */ generate(linkParams: List): string { - return this._registry.generate(linkParams, this.hostComponent); + let normalizedLinkParams = splitAndFlattenLinkParams(linkParams); + + var first = ListWrapper.first(normalizedLinkParams); + var rest = ListWrapper.slice(normalizedLinkParams, 1); + + var router = this; + + // The first segment should be either '.' (generate from parent) or '' (generate from root). + // When we normalize above, we strip all the slashes, './' becomes '.' and '/' becomes ''. + if (first == '') { + while (isPresent(router.parent)) { + router = router.parent; + } + } else if (first == '..') { + router = router.parent; + while (ListWrapper.first(rest) == '..') { + rest = ListWrapper.slice(rest, 1); + router = router.parent; + if (isBlank(router)) { + throw new BaseException( + `Link "${ListWrapper.toJSON(linkParams)}" has too many "../" segments.`); + } + } + } else if (first != '.') { + throw new BaseException( + `Link "${ListWrapper.toJSON(linkParams)}" must start with "/", "./", or "../"`); + } + + if (rest[rest.length - 1] == '') { + ListWrapper.removeLast(rest); + } + + if (rest.length < 1) { + let msg = `Link "${ListWrapper.toJSON(linkParams)}" must include a route name.`; + throw new BaseException(msg); + } + + let url = ''; + if (isPresent(router.parent) && isPresent(router.parent._currentInstruction)) { + url = router.parent._currentInstruction.capturedUrl; + } + return url + '/' + this.registry.generate(rest, router.hostComponent); } } @@ -204,7 +253,7 @@ export class RootRouter extends Router { super(registry, pipeline, null, hostComponent); this._location = location; this._location.subscribe((change) => this.navigate(change['url'])); - this._registry.configFromComponent(hostComponent); + this.registry.configFromComponent(hostComponent); this.navigate(location.path()); } @@ -216,7 +265,7 @@ export class RootRouter extends Router { class ChildRouter extends Router { constructor(parent: Router, hostComponent) { - super(parent._registry, parent._pipeline, parent, hostComponent); + super(parent.registry, parent._pipeline, parent, hostComponent); this.parent = parent; } @@ -226,3 +275,18 @@ class ChildRouter extends Router { return this.parent.navigate(url); } } + +/* + * Given: ['/a/b', {c: 2}] + * Returns: ['', 'a', 'b', {c: 2}] + */ +var SLASH = new RegExp('/'); +function splitAndFlattenLinkParams(linkParams: List): List { + return ListWrapper.reduce(linkParams, (accumulation, item) => { + if (isString(item)) { + return ListWrapper.concat(accumulation, StringWrapper.split(item, SLASH)); + } + accumulation.push(item); + return accumulation; + }, []); +} diff --git a/modules/angular2/src/router/router_link.ts b/modules/angular2/src/router/router_link.ts index 82c20ac750..fd541bda91 100644 --- a/modules/angular2/src/router/router_link.ts +++ b/modules/angular2/src/router/router_link.ts @@ -27,10 +27,11 @@ import {Location} from './location'; * means that we want to generate a link for the `team` route with params `{teamId: 1}`, * and with a child route `user` with params `{userId: 2}`. * - * The first route name should be prepended with either `./` or `/`. + * The first route name should be prepended with `/`, `./`, or `../`. * If the route begins with `/`, the router will look up the route from the root of the app. * If the route begins with `./`, the router will instead look in the current component's - * children for the route. + * children for the route. And if the route begins with `../`, the router will look at the + * current component's parent. * * @exportedAs angular2/router */ diff --git a/modules/angular2/test/router/outlet_spec.ts b/modules/angular2/test/router/outlet_spec.ts index 207febb72a..54926490b9 100644 --- a/modules/angular2/test/router/outlet_spec.ts +++ b/modules/angular2/test/router/outlet_spec.ts @@ -18,7 +18,7 @@ import { import {Injector, bind} from 'angular2/di'; import {Component, View} from 'angular2/src/core/annotations/decorators'; import * as annotations from 'angular2/src/core/annotations_impl/view'; -import {CONST} from 'angular2/src/facade/lang'; +import {CONST, NumberWrapper} from 'angular2/src/facade/lang'; import {RootRouter} from 'angular2/src/router/router'; import {Pipeline} from 'angular2/src/router/pipeline'; @@ -42,7 +42,7 @@ export function main() { beforeEachBindings(() => [ Pipeline, - bind(RouteRegistry).toFactory(() => new RouteRegistry(MyComp)), + RouteRegistry, DirectiveResolver, bind(Location).toClass(SpyLocation), bind(Router) @@ -185,6 +185,49 @@ export function main() { }); })); + + it('should generate link hrefs from a child to its sibling', + inject([AsyncTestCompleter], (async) => { + compile() + .then((_) => rtr.config( + {'path': '/page/:number', 'component': SiblingPageCmp, 'as': 'page'})) + .then((_) => rtr.navigate('/page/1')) + .then((_) => { + rootTC.detectChanges(); + expect(DOM.getAttribute(rootTC.componentViewChildren[1] + .componentViewChildren[0] + .children[0] + .nativeElement, + 'href')) + .toEqual('/page/2'); + async.done(); + }); + })); + + it('should generate relative links preserving the existing parent route', + inject([AsyncTestCompleter], (async) => { + compile() + .then((_) => + rtr.config({'path': '/book/:title/...', 'component': BookCmp, 'as': 'book'})) + .then((_) => rtr.navigate('/book/1984/page/1')) + .then((_) => { + rootTC.detectChanges(); + expect(DOM.getAttribute( + rootTC.componentViewChildren[1].componentViewChildren[0].nativeElement, + 'href')) + .toEqual('/book/1984/page/100'); + + expect(DOM.getAttribute(rootTC.componentViewChildren[1] + .componentViewChildren[2] + .componentViewChildren[0] + .children[0] + .nativeElement, + 'href')) + .toEqual('/book/1984/page/2'); + async.done(); + }); + })); + describe('when clicked', () => { var clickOnElement = function(view) { @@ -266,6 +309,34 @@ class UserCmp { } +@Component({selector: 'page-cmp'}) +@View({ + template: + `page #{{pageNumber}} | next`, + directives: [RouterLink] +}) +class SiblingPageCmp { + pageNumber: number; + nextPage: number; + constructor(params: RouteParams) { + this.pageNumber = NumberWrapper.parseInt(params.get('number'), 10); + this.nextPage = this.pageNumber + 1; + } +} + +@Component({selector: 'book-cmp'}) +@View({ + template: `{{title}} | + `, + directives: [RouterLink, RouterOutlet] +}) +@RouteConfig([{path: '/page/:number', component: SiblingPageCmp, 'as': 'page'}]) +class BookCmp { + title: string; + constructor(params: RouteParams) { this.title = params.get('title'); } +} + + @Component({selector: 'parent-cmp'}) @View({template: "inner { }", directives: [RouterOutlet]}) @RouteConfig([{path: '/b', component: HelloCmp}]) diff --git a/modules/angular2/test/router/route_registry_spec.ts b/modules/angular2/test/router/route_registry_spec.ts index 05cb22d667..f6bdc41fd1 100644 --- a/modules/angular2/test/router/route_registry_spec.ts +++ b/modules/angular2/test/router/route_registry_spec.ts @@ -20,7 +20,7 @@ export function main() { describe('RouteRegistry', () => { var registry, rootHostComponent = new Object(); - beforeEach(() => { registry = new RouteRegistry(rootHostComponent); }); + beforeEach(() => { registry = new RouteRegistry(); }); it('should match the full URL', inject([AsyncTestCompleter], (async) => { registry.config(rootHostComponent, {'path': '/', 'component': DummyCompA}); @@ -37,9 +37,9 @@ export function main() { registry.config(rootHostComponent, {'path': '/first/...', 'component': DummyParentComp, 'as': 'firstCmp'}); - expect(registry.generate(['./firstCmp/secondCmp'], rootHostComponent)) - .toEqual('/first/second'); - expect(registry.generate(['./secondCmp'], DummyParentComp)).toEqual('/second'); + expect(registry.generate(['firstCmp', 'secondCmp'], rootHostComponent)) + .toEqual('first/second'); + expect(registry.generate(['secondCmp'], DummyParentComp)).toEqual('second'); }); it('should generate URLs with params', () => { @@ -47,20 +47,9 @@ export function main() { rootHostComponent, {'path': '/first/:param/...', 'component': DummyParentParamComp, 'as': 'firstCmp'}); - var url = registry.generate(['./firstCmp', {param: 'one'}, 'secondCmp', {param: 'two'}], + var url = registry.generate(['firstCmp', {param: 'one'}, 'secondCmp', {param: 'two'}], rootHostComponent); - expect(url).toEqual('/first/one/second/two'); - }); - - it('should generate URLs from the root component when the path starts with /', () => { - registry.config(rootHostComponent, - {'path': '/first/...', 'component': DummyParentComp, 'as': 'firstCmp'}); - - expect(registry.generate(['/firstCmp', 'secondCmp'], rootHostComponent)) - .toEqual('/first/second'); - expect(registry.generate(['/firstCmp', 'secondCmp'], DummyParentComp)) - .toEqual('/first/second'); - expect(registry.generate(['/firstCmp/secondCmp'], DummyParentComp)).toEqual('/first/second'); + expect(url).toEqual('first/one/second/two'); }); it('should generate URLs of loaded components after they are loaded', @@ -71,30 +60,17 @@ export function main() { 'as': 'firstCmp' }); - expect(() => registry.generate(['/firstCmp/secondCmp'], rootHostComponent)) + expect(() => registry.generate(['firstCmp', 'secondCmp'], rootHostComponent)) .toThrowError('Could not find route config for "secondCmp".'); registry.recognize('/first/second', rootHostComponent) .then((_) => { - expect(registry.generate(['/firstCmp/secondCmp'], rootHostComponent)) - .toEqual('/first/second'); + expect(registry.generate(['firstCmp', 'secondCmp'], rootHostComponent)) + .toEqual('first/second'); async.done(); }); })); - it('should throw when linkParams does not start with a "/" or "./"', () => { - expect(() => registry.generate(['firstCmp', 'secondCmp'], rootHostComponent)) - .toThrowError( - `Link "${ListWrapper.toJSON(['firstCmp', 'secondCmp'])}" must start with "/" or "./"`); - }); - - it('should throw when linkParams does not include a route name', () => { - expect(() => registry.generate(['./'], rootHostComponent)) - .toThrowError(`Link "${ListWrapper.toJSON(['./'])}" must include a route name.`); - expect(() => registry.generate(['/'], rootHostComponent)) - .toThrowError(`Link "${ListWrapper.toJSON(['/'])}" must include a route name.`); - }); - it('should prefer static segments to dynamic', inject([AsyncTestCompleter], (async) => { registry.config(rootHostComponent, {'path': '/:site', 'component': DummyCompB}); registry.config(rootHostComponent, {'path': '/home', 'component': DummyCompA}); diff --git a/modules/angular2/test/router/router_spec.ts b/modules/angular2/test/router/router_spec.ts index 21fb351e52..45bc098538 100644 --- a/modules/angular2/test/router/router_spec.ts +++ b/modules/angular2/test/router/router_spec.ts @@ -14,6 +14,7 @@ import { import {IMPLEMENTS} from 'angular2/src/facade/lang'; import {Promise, PromiseWrapper} from 'angular2/src/facade/async'; +import {ListWrapper} from 'angular2/src/facade/collection'; import {Router, RootRouter} from 'angular2/src/router/router'; import {Pipeline} from 'angular2/src/router/pipeline'; import {RouterOutlet} from 'angular2/src/router/router_outlet'; @@ -21,6 +22,7 @@ import {SpyLocation} from 'angular2/src/mock/location_mock'; import {Location} from 'angular2/src/router/location'; import {RouteRegistry} from 'angular2/src/router/route_registry'; +import {RouteConfig} from 'angular2/src/router/route_config_decorator'; import {DirectiveResolver} from 'angular2/src/core/compiler/directive_resolver'; import {bind} from 'angular2/di'; @@ -31,7 +33,7 @@ export function main() { beforeEachBindings(() => [ Pipeline, - bind(RouteRegistry).toFactory(() => new RouteRegistry(AppCmp)), + RouteRegistry, DirectiveResolver, bind(Location).toClass(SpyLocation), bind(Router) @@ -74,6 +76,7 @@ export function main() { }); })); + it('should navigate after being configured', inject([AsyncTestCompleter], (async) => { var outlet = makeDummyOutlet(); @@ -88,6 +91,30 @@ export function main() { async.done(); }); })); + + + it('should throw when linkParams does not start with a "/" or "./"', () => { + expect(() => router.generate(['firstCmp', 'secondCmp'])) + .toThrowError( + `Link "${ListWrapper.toJSON(['firstCmp', 'secondCmp'])}" must start with "/", "./", or "../"`); + }); + + + it('should throw when linkParams does not include a route name', () => { + expect(() => router.generate(['./'])) + .toThrowError(`Link "${ListWrapper.toJSON(['./'])}" must include a route name.`); + expect(() => router.generate(['/'])) + .toThrowError(`Link "${ListWrapper.toJSON(['/'])}" must include a route name.`); + }); + + + it('should generate URLs from the root component when the path starts with /', () => { + router.config({'path': '/first/...', 'component': DummyParentComp, 'as': 'firstCmp'}); + + expect(router.generate(['/firstCmp', 'secondCmp'])).toEqual('/first/second'); + expect(router.generate(['/firstCmp', 'secondCmp'])).toEqual('/first/second'); + expect(router.generate(['/firstCmp/secondCmp'])).toEqual('/first/second'); + }); }); } @@ -99,6 +126,10 @@ class DummyOutlet extends SpyObject { class DummyComponent {} +@RouteConfig([{'path': '/second', 'component': DummyComponent, 'as': 'secondCmp'}]) +class DummyParentComp { +} + function makeDummyOutlet() { var ref = new DummyOutlet(); ref.spy('activate').andCallFake((_) => PromiseWrapper.resolve(true));