From 2d2ae9b8d88f99381852a5e678afb5f292234e7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Wed, 17 Jun 2015 11:57:38 -0700 Subject: [PATCH] feat(router): enforce usage of ... syntax for parent to child component routes --- modules/angular2/src/mock/location_mock.ts | 2 +- .../angular2/src/router/path_recognizer.ts | 32 +++++++++++++++++-- .../angular2/src/router/route_recognizer.ts | 15 +++++++-- modules/angular2/src/router/route_registry.ts | 29 +++++++++++++++-- modules/angular2/test/router/outlet_spec.ts | 4 +-- .../test/router/route_recognizer_spec.ts | 25 ++++++++------- .../test/router/route_registry_spec.ts | 21 ++++++++++-- .../test/router/router_integration_spec.ts | 2 +- 8 files changed, 103 insertions(+), 27 deletions(-) diff --git a/modules/angular2/src/mock/location_mock.ts b/modules/angular2/src/mock/location_mock.ts index d34443a0fe..fddceab5df 100644 --- a/modules/angular2/src/mock/location_mock.ts +++ b/modules/angular2/src/mock/location_mock.ts @@ -16,7 +16,7 @@ export class SpyLocation extends SpyObject { constructor() { super(); - this._path = '/'; + this._path = ''; this.urlChanges = []; this._subject = new EventEmitter(); this._baseHref = ''; diff --git a/modules/angular2/src/router/path_recognizer.ts b/modules/angular2/src/router/path_recognizer.ts index a0f03691ea..54d441ae58 100644 --- a/modules/angular2/src/router/path_recognizer.ts +++ b/modules/angular2/src/router/path_recognizer.ts @@ -27,6 +27,10 @@ export class Segment { regex: string; } +export class ContinuationSegment extends Segment { + generate(params): string { return ''; } +} + class StaticSegment extends Segment { regex: string; name: string; @@ -71,7 +75,7 @@ var wildcardMatcher = RegExpWrapper.create("^\\*([^\/]+)$"); function parsePathString(route: string) { // normalize route as not starting with a "/". Recognition will // also normalize. - if (route[0] === "/") { + if (StringWrapper.startsWith(route, "/")) { route = StringWrapper.substring(route, 1); } @@ -93,7 +97,8 @@ function parsePathString(route: string) { throw new BaseException(`'${route}' has more than the maximum supported number of segments.`); } - for (var i = 0; i < segments.length; i++) { + var limit = segments.length - 1; + for (var i = 0; i <= limit; i++) { var segment = segments[i], match; if (isPresent(match = RegExpWrapper.firstMatch(paramMatcher, segment))) { @@ -101,6 +106,12 @@ function parsePathString(route: string) { specificity += (100 - i); } else if (isPresent(match = RegExpWrapper.firstMatch(wildcardMatcher, segment))) { results.push(new StarSegment(match[1])); + } else if (segment == '...') { + if (i < limit) { + // TODO (matsko): setup a proper error here ` + throw new BaseException(`Unexpected "..." before the end of the path for "${route}".`); + } + results.push(new ContinuationSegment()); } else if (segment.length > 0) { results.push(new StaticSegment(segment)); specificity += 100 * (100 - i); @@ -120,6 +131,7 @@ export class PathRecognizer { segments: List; regex: RegExp; specificity: number; + terminal: boolean = true; constructor(public path: string, public handler: any) { this.segments = []; @@ -131,7 +143,17 @@ export class PathRecognizer { var segments = parsed['segments']; var regexString = '^'; - ListWrapper.forEach(segments, (segment) => { regexString += '/' + segment.regex; }); + ListWrapper.forEach(segments, (segment) => { + if (segment instanceof ContinuationSegment) { + this.terminal = false; + } else { + regexString += '/' + segment.regex; + } + }); + + if (this.terminal) { + regexString += '$'; + } this.regex = RegExpWrapper.create(regexString); this.segments = segments; @@ -143,6 +165,10 @@ export class PathRecognizer { var urlPart = url; for (var i = 0; i < this.segments.length; i++) { var segment = this.segments[i]; + if (segment instanceof ContinuationSegment) { + continue; + } + var match = RegExpWrapper.firstMatch(RegExpWrapper.create('/' + segment.regex), urlPart); urlPart = StringWrapper.substring(urlPart, match[0].length); if (segment.name.length > 0) { diff --git a/modules/angular2/src/router/route_recognizer.ts b/modules/angular2/src/router/route_recognizer.ts index 5df91aa439..7a16f4362e 100644 --- a/modules/angular2/src/router/route_recognizer.ts +++ b/modules/angular2/src/router/route_recognizer.ts @@ -14,7 +14,7 @@ import { StringMapWrapper } from 'angular2/src/facade/collection'; -import {PathRecognizer} from './path_recognizer'; +import {PathRecognizer, ContinuationSegment} from './path_recognizer'; /** * `RouteRecognizer` is responsible for recognizing routes for a single component. @@ -32,9 +32,14 @@ export class RouteRecognizer { this.redirects = new Map(); } - addRedirect(path: string, target: string): void { this.redirects.set(path, target); } + addRedirect(path: string, target: string): void { + if (path == '/') { + path = ''; + } + this.redirects.set(path, target); + } - addConfig(path: string, handler: any, alias: string = null): void { + addConfig(path: string, handler: any, alias: string = null): boolean { var recognizer = new PathRecognizer(path, handler); MapWrapper.forEach(this.matchers, (matcher, _) => { if (recognizer.regex.toString() == matcher.regex.toString()) { @@ -46,6 +51,7 @@ export class RouteRecognizer { if (isPresent(alias)) { this.names.set(alias, recognizer); } + return recognizer.terminal; } @@ -55,6 +61,9 @@ export class RouteRecognizer { */ recognize(url: string): List { var solutions = []; + if (url.length > 0 && url[url.length - 1] == '/') { + url = url.substring(0, url.length - 1); + } MapWrapper.forEach(this.redirects, (target, path) => { // "/" redirect case diff --git a/modules/angular2/src/router/route_registry.ts b/modules/angular2/src/router/route_registry.ts index 3b7a5e4ad4..b4970e8c2a 100644 --- a/modules/angular2/src/router/route_registry.ts +++ b/modules/angular2/src/router/route_registry.ts @@ -53,12 +53,17 @@ export class RouteRegistry { config, {'component': normalizeComponentDeclaration(config['component'])}); var component = config['component']; - this.configFromComponent(component); + var terminal = recognizer.addConfig(config['path'], config, config['as']); - recognizer.addConfig(config['path'], config, config['as']); + if (component['type'] == 'constructor') { + if (terminal) { + assertTerminalComponent(component['constructor'], config['path']); + } else { + this.configFromComponent(component['constructor']); + } + } } - /** * Reads the annotations of a component and configures the registry based on them */ @@ -221,3 +226,21 @@ function mostSpecific(instructions: List): Instruction { } return mostSpecificSolution; } + +function assertTerminalComponent(component, path) { + if (!isType(component)) { + return; + } + + var annotations = reflector.annotations(component); + if (isPresent(annotations)) { + for (var i = 0; i < annotations.length; i++) { + var annotation = annotations[i]; + + if (annotation instanceof RouteConfig) { + throw new BaseException( + `Child routes are not allowed for "${path}". Use "..." on the parent's route path.`); + } + } + } +} diff --git a/modules/angular2/test/router/outlet_spec.ts b/modules/angular2/test/router/outlet_spec.ts index d4e74192e9..0d9f15f7f6 100644 --- a/modules/angular2/test/router/outlet_spec.ts +++ b/modules/angular2/test/router/outlet_spec.ts @@ -100,7 +100,7 @@ export function main() { it('should work with child routers', inject([AsyncTestCompleter], (async) => { compile('outer { }') - .then((_) => rtr.config({'path': '/a', 'component': ParentCmp})) + .then((_) => rtr.config({'path': '/a/...', 'component': ParentCmp})) .then((_) => rtr.navigate('/a/b')) .then((_) => { view.detectChanges(); @@ -153,7 +153,7 @@ export function main() { it('should reuse common parent components', inject([AsyncTestCompleter], (async) => { compile() - .then((_) => rtr.config({'path': '/team/:id', 'component': TeamCmp})) + .then((_) => rtr.config({'path': '/team/:id/...', 'component': TeamCmp})) .then((_) => rtr.navigate('/team/angular/user/rado')) .then((_) => { view.detectChanges(); diff --git a/modules/angular2/test/router/route_recognizer_spec.ts b/modules/angular2/test/router/route_recognizer_spec.ts index c27a287211..47925e42bc 100644 --- a/modules/angular2/test/router/route_recognizer_spec.ts +++ b/modules/angular2/test/router/route_recognizer_spec.ts @@ -87,21 +87,24 @@ export function main() { expect(solution.matchedUrl).toEqual('/bar'); }); - it('should perform a valid redirect when a slash or an empty string is being processed', () => { - recognizer.addRedirect('/', '/matias'); - recognizer.addRedirect('', '/fatias'); + it('should perform a root URL redirect when only a slash or an empty string is being processed', + () => { + recognizer.addRedirect('/', '/matias'); + recognizer.addConfig('/matias', handler); - recognizer.addConfig('/matias', handler); - recognizer.addConfig('/fatias', handler); + recognizer.addConfig('/fatias', handler); - var solutions; + var solutions; - solutions = recognizer.recognize('/'); - expect(solutions[0].matchedUrl).toBe('/matias'); + solutions = recognizer.recognize('/'); + expect(solutions[0].matchedUrl).toBe('/matias'); - solutions = recognizer.recognize(''); - expect(solutions[0].matchedUrl).toBe('/fatias'); - }); + solutions = recognizer.recognize('/fatias'); + expect(solutions[0].matchedUrl).toBe('/fatias'); + + solutions = recognizer.recognize(''); + expect(solutions[0].matchedUrl).toBe('/matias'); + }); it('should generate URLs', () => { recognizer.addConfig('/app/user/:name', handler, 'user'); diff --git a/modules/angular2/test/router/route_registry_spec.ts b/modules/angular2/test/router/route_registry_spec.ts index ea7aebabda..77228282df 100644 --- a/modules/angular2/test/router/route_registry_spec.ts +++ b/modules/angular2/test/router/route_registry_spec.ts @@ -91,7 +91,7 @@ export function main() { })); it('should match the full URL using child components', inject([AsyncTestCompleter], (async) => { - registry.config(rootHostComponent, {'path': '/first', 'component': DummyParentComp}); + registry.config(rootHostComponent, {'path': '/first/...', 'component': DummyParentComp}); registry.recognize('/first/second', rootHostComponent) .then((instruction) => { @@ -103,7 +103,7 @@ export function main() { it('should match the URL using async child components', inject([AsyncTestCompleter], (async) => { - registry.config(rootHostComponent, {'path': '/first', 'component': DummyAsyncComp}); + registry.config(rootHostComponent, {'path': '/first/...', 'component': DummyAsyncComp}); registry.recognize('/first/second', rootHostComponent) .then((instruction) => { @@ -117,7 +117,7 @@ export function main() { inject([AsyncTestCompleter], (async) => { registry.config( rootHostComponent, - {'path': '/first', 'component': {'loader': AsyncParentLoader, 'type': 'loader'}}); + {'path': '/first/...', 'component': {'loader': AsyncParentLoader, 'type': 'loader'}}); registry.recognize('/first/second', rootHostComponent) .then((instruction) => { @@ -139,6 +139,21 @@ export function main() { {'path': '/some/path', 'component': {'type': 'intentionallyWrongComponentType'}})) .toThrowError('Invalid component type \'intentionallyWrongComponentType\''); }); + + it('should throw when a parent config is missing the `...` suffix any of its children add routes', + () => { + expect(() => + registry.config(rootHostComponent, {'path': '/', 'component': DummyParentComp})) + .toThrowError( + 'Child routes are not allowed for "/". Use "..." on the parent\'s route path.'); + }); + + it('should throw when a parent config is missing the `...` suffix any of its children add routes', + () => { + expect(() => registry.config(rootHostComponent, + {'path': '/home/.../fun/', 'component': DummyParentComp})) + .toThrowError('Unexpected "..." before the end of the path for "home/.../fun/".'); + }); }); } diff --git a/modules/angular2/test/router/router_integration_spec.ts b/modules/angular2/test/router/router_integration_spec.ts index addb2addd3..f6a2840586 100644 --- a/modules/angular2/test/router/router_integration_spec.ts +++ b/modules/angular2/test/router/router_integration_spec.ts @@ -122,7 +122,7 @@ class ParentCmp { @Component({selector: 'app-cmp'}) @View({template: `root { }`, directives: routerDirectives}) -@RouteConfig([{path: '/parent', component: ParentCmp}]) +@RouteConfig([{path: '/parent/...', component: ParentCmp}]) class HierarchyAppCmp { constructor(public router: Router, public location: BrowserLocation) {} }