From 07cdc2ff442eb79b0aa1c0c42bfc4f40aa781e97 Mon Sep 17 00:00:00 2001 From: Brian Ford Date: Mon, 26 Oct 2015 17:18:08 +0000 Subject: [PATCH] feat(router): add support for route links with no leading slash Closes #4623 --- modules/angular2/src/router/route_registry.ts | 8 ++ modules/angular2/src/router/router.ts | 21 ++++- .../router/integration/router_link_spec.ts | 92 +++++++++++++++++++ modules/angular2/test/router/router_spec.ts | 10 -- 4 files changed, 116 insertions(+), 15 deletions(-) diff --git a/modules/angular2/src/router/route_registry.ts b/modules/angular2/src/router/route_registry.ts index 116790d020..2f77ce9b8f 100644 --- a/modules/angular2/src/router/route_registry.ts +++ b/modules/angular2/src/router/route_registry.ts @@ -254,6 +254,14 @@ export class RouteRegistry { return instruction; } + public hasRoute(name: string, parentComponent: any): boolean { + var componentRecognizer: RouteRecognizer = this._rules.get(parentComponent); + if (isBlank(componentRecognizer)) { + return false; + } + return componentRecognizer.hasRoute(name); + } + // if the child includes a redirect like : "/" -> "/something", // we want to honor that redirection when creating the link private _generateRedirects(componentCursor: Type): Instruction { diff --git a/modules/angular2/src/router/router.ts b/modules/angular2/src/router/router.ts index 78753ede6a..578b08d397 100644 --- a/modules/angular2/src/router/router.ts +++ b/modules/angular2/src/router/router.ts @@ -432,8 +432,21 @@ export class Router { } } } else if (first != '.') { - throw new BaseException( - `Link "${ListWrapper.toJSON(linkParams)}" must start with "/", "./", or "../"`); + // For a link with no leading `./`, `/`, or `../`, we look for a sibling and child. + // If both exist, we throw. Otherwise, we prefer whichever exists. + var childRouteExists = this.registry.hasRoute(first, this.hostComponent); + var parentRouteExists = + isPresent(this.parent) && this.registry.hasRoute(first, this.parent.hostComponent); + + if (parentRouteExists && childRouteExists) { + let msg = + `Link "${ListWrapper.toJSON(linkParams)}" is ambiguous, use "./" or "../" to disambiguate.`; + throw new BaseException(msg); + } + if (parentRouteExists) { + router = this.parent; + } + rest = linkParams; } if (rest[rest.length - 1] == '') { @@ -445,7 +458,7 @@ export class Router { throw new BaseException(msg); } - // TODO: structural cloning and whatnot + var nextInstruction = this.registry.generate(rest, router.hostComponent); var url = []; var parent = router.parent; @@ -454,8 +467,6 @@ export class Router { parent = parent.parent; } - var nextInstruction = this.registry.generate(rest, router.hostComponent); - while (url.length > 0) { nextInstruction = url.pop().replaceChild(nextInstruction); } diff --git a/modules/angular2/test/router/integration/router_link_spec.ts b/modules/angular2/test/router/integration/router_link_spec.ts index 706fedb9b0..a3e222b8a9 100644 --- a/modules/angular2/test/router/integration/router_link_spec.ts +++ b/modules/angular2/test/router/integration/router_link_spec.ts @@ -19,6 +19,7 @@ import { import {NumberWrapper} from 'angular2/src/core/facade/lang'; import {PromiseWrapper} from 'angular2/src/core/facade/async'; +import {ListWrapper} from 'angular2/src/core/facade/collection'; import {provide, Component, DirectiveResolver, View} from 'angular2/core'; @@ -133,6 +134,58 @@ export function main() { }); })); + it('should generate link hrefs from a child to its sibling with no leading slash', + inject([AsyncTestCompleter], (async) => { + compile() + .then((_) => router.config([ + new Route({path: '/page/:number', component: NoPrefixSiblingPageCmp, as: 'Page'}) + ])) + .then((_) => router.navigateByUrl('/page/1')) + .then((_) => { + rootTC.detectChanges(); + expect(DOM.getAttribute(rootTC.debugElement.componentViewChildren[1] + .componentViewChildren[0] + .nativeElement, + 'href')) + .toEqual('/page/2'); + async.done(); + }); + })); + + it('should generate link hrefs to a child with no leading slash', + inject([AsyncTestCompleter], (async) => { + compile() + .then((_) => router.config([ + new Route({path: '/book/:title/...', component: NoPrefixBookCmp, as: 'Book'}) + ])) + .then((_) => router.navigateByUrl('/book/1984/page/1')) + .then((_) => { + rootTC.detectChanges(); + expect(DOM.getAttribute(rootTC.debugElement.componentViewChildren[1] + .componentViewChildren[0] + .nativeElement, + 'href')) + .toEqual('/book/1984/page/100'); + async.done(); + }); + })); + + it('should throw when links without a leading slash are ambiguous', + inject([AsyncTestCompleter], (async) => { + compile() + .then((_) => router.config([ + new Route({path: '/book/:title/...', component: AmbiguousBookCmp, as: 'Book'}) + ])) + .then((_) => router.navigateByUrl('/book/1984/page/1')) + .then((_) => { + var link = ListWrapper.toJSON(['Book', {number: 100}]); + expect(() => rootTC.detectChanges()) + .toThrowErrorWith( + `Link "${link}" is ambiguous, use "./" or "../" to disambiguate.`); + async.done(); + }); + })); + it('should generate link hrefs when asynchronously loaded', inject([AsyncTestCompleter], (async) => { compile() @@ -337,6 +390,21 @@ class SiblingPageCmp { } } +@Component({selector: 'page-cmp'}) +@View({ + template: + `page #{{pageNumber}} | next`, + directives: [RouterLink] +}) +class NoPrefixSiblingPageCmp { + pageNumber: number; + nextPage: number; + constructor(params: RouteParams) { + this.pageNumber = NumberWrapper.parseInt(params.get('number'), 10); + this.nextPage = this.pageNumber + 1; + } +} + @Component({selector: 'hello-cmp'}) @View({template: 'hello'}) class HelloCmp { @@ -377,3 +445,27 @@ class BookCmp { title: string; constructor(params: RouteParams) { this.title = params.get('title'); } } + +@Component({selector: 'book-cmp'}) +@View({ + template: `{{title}} | + `, + directives: ROUTER_DIRECTIVES +}) +@RouteConfig([new Route({path: '/page/:number', component: SiblingPageCmp, as: 'Page'})]) +class NoPrefixBookCmp { + title: string; + constructor(params: RouteParams) { this.title = params.get('title'); } +} + +@Component({selector: 'book-cmp'}) +@View({ + template: `{{title}} | + `, + directives: ROUTER_DIRECTIVES +}) +@RouteConfig([new Route({path: '/page/:number', component: SiblingPageCmp, as: 'Book'})]) +class AmbiguousBookCmp { + title: string; + constructor(params: RouteParams) { this.title = params.get('title'); } +} diff --git a/modules/angular2/test/router/router_spec.ts b/modules/angular2/test/router/router_spec.ts index 3b385ee83c..9c6ccc91fa 100644 --- a/modules/angular2/test/router/router_spec.ts +++ b/modules/angular2/test/router/router_spec.ts @@ -61,7 +61,6 @@ export function main() { }); })); - it('should activate viewports and update URL on navigate', inject([AsyncTestCompleter], (async) => { var outlet = makeDummyOutlet(); @@ -105,7 +104,6 @@ export function main() { }); })); - it('should navigate after being configured', inject([AsyncTestCompleter], (async) => { var outlet = makeDummyOutlet(); @@ -121,14 +119,6 @@ export function main() { }); })); - - 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.`);