feat(router): enforce usage of ... syntax for parent to child component routes

Author: matsko

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 = '';

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,14 +97,21 @@ 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))) {
       results.push(new DynamicSegment(match[1]));
       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<Segment>;
   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) {

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<RouteMatch> {
     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

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>): 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.`);
+      }
+    }
+  }
+}

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 { <router-outlet></router-outlet> }')
-             .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();

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');

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/".');
+       });
   });
 }
 

modules/angular2/test/router/router_integration_spec.ts

@@ -122,7 +122,7 @@ class ParentCmp {
 
 @Component({selector: 'app-cmp'})
 @View({template: `root { <router-outlet></router-outlet> }`, directives: routerDirectives})
-@RouteConfig([{path: '/parent', component: ParentCmp}])
+@RouteConfig([{path: '/parent/...', component: ParentCmp}])
 class HierarchyAppCmp {
   constructor(public router: Router, public location: BrowserLocation) {}
 }