refactor(router): cleanup & simplifications

packages/router/src/apply_redirects.ts

@@ -54,6 +54,11 @@ function canLoadFails(route: Route): Observable<LoadedRouterConfig> {
           `Cannot load children because the guard of the route "path: '${route.path}'" returned false`)));
 }
 
+/**
+ * Returns the `UrlTree` with the redirection applied.
+ *
+ * Lazy modules are loaded along the way.
+ */
 export function applyRedirects(
     moduleInjector: Injector, configLoader: RouterConfigLoader, urlSerializer: UrlSerializer,
     urlTree: UrlTree, config: Routes): Observable<UrlTree> {
@@ -131,6 +136,7 @@ class ApplyRedirects {
     return this.expandSegment(ngModule, segmentGroup, routes, segmentGroup.segments, outlet, true);
   }
 
+  // Recursively expand segment groups for all the child outlets
   private expandChildren(
       ngModule: NgModuleRef<any>, routes: Route[],
       segmentGroup: UrlSegmentGroup): Observable<{[name: string]: UrlSegmentGroup}> {
@@ -182,16 +188,16 @@ class ApplyRedirects {
       return noMatch(segmentGroup);
     }
 
-    if (route.redirectTo !== undefined && !(allowRedirects && this.allowRedirects)) {
-      return noMatch(segmentGroup);
-    }
-
     if (route.redirectTo === undefined) {
       return this.matchSegmentAgainstRoute(ngModule, segmentGroup, route, paths);
     }
 
-    return this.expandSegmentAgainstRouteUsingRedirect(
+    if (allowRedirects && this.allowRedirects) {
-        ngModule, segmentGroup, routes, route, paths, outlet);
+      return this.expandSegmentAgainstRouteUsingRedirect(
+          ngModule, segmentGroup, routes, route, paths, outlet);
+    }
+
+    return noMatch(segmentGroup);
   }
 
   private expandSegmentAgainstRouteUsingRedirect(
@@ -294,8 +300,9 @@ class ApplyRedirects {
     }
 
     if (route.loadChildren) {
-      if ((<any>route)._loadedConfig !== void 0) {
+      // lazy children belong to the loaded module
-        return of ((<any>route)._loadedConfig);
+      if (route._loadedConfig !== undefined) {
+        return of (route._loadedConfig);
       }
 
       return mergeMap.call(runCanLoadGuard(ngModule.injector, route), (shouldLoad: boolean) => {
@@ -417,8 +424,6 @@ function match(segmentGroup: UrlSegmentGroup, route: Route, segments: UrlSegment
   lastChild: number,
   positionalParamSegments: {[k: string]: UrlSegment}
 } {
-  const noMatch =
-      {matched: false, consumedSegments: <any[]>[], lastChild: 0, positionalParamSegments: {}};
   if (route.path === '') {
     if ((route.pathMatch === 'full') && (segmentGroup.hasChildren() || segments.length > 0)) {
       return {matched: false, consumedSegments: [], lastChild: 0, positionalParamSegments: {}};
@@ -429,13 +434,18 @@ function match(segmentGroup: UrlSegmentGroup, route: Route, segments: UrlSegment
 
   const matcher = route.matcher || defaultUrlMatcher;
   const res = matcher(segments, segmentGroup, route);
-  if (!res) return noMatch;
+
+  if (!res) {
+    return {
+      matched: false, consumedSegments: <any[]>[], lastChild: 0, positionalParamSegments: {},
+    }
+  }
 
   return {
     matched: true,
     consumedSegments: res.consumed,
     lastChild: res.consumed.length,
-    positionalParamSegments: res.posParams
+    positionalParamSegments: res.posParams,
   };
 }
 
@@ -475,7 +485,7 @@ function addEmptySegmentsToChildrenIfNeeded(
     children: {[name: string]: UrlSegmentGroup}): {[name: string]: UrlSegmentGroup} {
   const res: {[name: string]: UrlSegmentGroup} = {};
   for (const r of routes) {
-    if (emptyPathRedirect(segmentGroup, slicedSegments, r) && !children[getOutlet(r)]) {
+    if (isEmptyPathRedirect(segmentGroup, slicedSegments, r) && !children[getOutlet(r)]) {
       res[getOutlet(r)] = new UrlSegmentGroup([], {});
     }
   }
@@ -495,22 +505,19 @@ function createChildrenForEmptySegments(
 }
 
 function containsEmptyPathRedirectsWithNamedOutlets(
-    segmentGroup: UrlSegmentGroup, slicedSegments: UrlSegment[], routes: Route[]): boolean {
+    segmentGroup: UrlSegmentGroup, segments: UrlSegment[], routes: Route[]): boolean {
-  return routes
+  return routes.some(
-             .filter(
+      r => isEmptyPathRedirect(segmentGroup, segments, r) && getOutlet(r) !== PRIMARY_OUTLET);
-                 r => emptyPathRedirect(segmentGroup, slicedSegments, r) &&
-                     getOutlet(r) !== PRIMARY_OUTLET)
-             .length > 0;
 }
 
 function containsEmptyPathRedirects(
-    segmentGroup: UrlSegmentGroup, slicedSegments: UrlSegment[], routes: Route[]): boolean {
+    segmentGroup: UrlSegmentGroup, segments: UrlSegment[], routes: Route[]): boolean {
-  return routes.filter(r => emptyPathRedirect(segmentGroup, slicedSegments, r)).length > 0;
+  return routes.some(r => isEmptyPathRedirect(segmentGroup, segments, r));
 }
 
-function emptyPathRedirect(
+function isEmptyPathRedirect(
-    segmentGroup: UrlSegmentGroup, slicedSegments: UrlSegment[], r: Route): boolean {
+    segmentGroup: UrlSegmentGroup, segments: UrlSegment[], r: Route): boolean {
-  if ((segmentGroup.hasChildren() || slicedSegments.length > 0) && r.pathMatch === 'full') {
+  if ((segmentGroup.hasChildren() || segments.length > 0) && r.pathMatch === 'full') {
     return false;
   }
 
@@ -518,5 +525,5 @@ function emptyPathRedirect(
 }
 
 function getOutlet(route: Route): string {
-  return route.outlet ? route.outlet : PRIMARY_OUTLET;
+  return route.outlet || PRIMARY_OUTLET;
 }

packages/router/src/router_state.ts

@@ -38,8 +38,7 @@ import {Tree, TreeNode} from './utils/tree';
  *
  * @description
  * RouterState is a tree of activated routes. Every node in this tree knows about the "consumed" URL
- * segments,
+ * segments, the extracted parameters, and the resolved data.
- * the extracted parameters, and the resolved data.
  *
  * See {@link ActivatedRoute} for more information.
  *

packages/router/src/shared.ts

@@ -108,34 +108,36 @@ export function isNavigationCancelingError(error: Error) {
   return (error as any)[NAVIGATION_CANCELING_ERROR];
 }
 
+// Matches the route configuration (`route`) against the actual URL (`segments`).
 export function defaultUrlMatcher(
-    segments: UrlSegment[], segmentGroup: UrlSegmentGroup, route: Route): UrlMatchResult {
+    segments: UrlSegment[], segmentGroup: UrlSegmentGroup, route: Route): UrlMatchResult|null {
-  const path = route.path;
+  const parts = route.path.split('/');
-  const parts = path.split('/');
-  const posParams: {[key: string]: UrlSegment} = {};
-  const consumed: UrlSegment[] = [];
 
-  let currentIndex = 0;
+  if (parts.length > segments.length) {
-
+    // The actual URL is shorter than the config, no match
-  for (let i = 0; i < parts.length; ++i) {
+    return null;
-    if (currentIndex >= segments.length) return null;
-    const current = segments[currentIndex];
-
-    const p = parts[i];
-    const isPosParam = p.startsWith(':');
-
-    if (!isPosParam && p !== current.path) return null;
-    if (isPosParam) {
-      posParams[p.substring(1)] = current;
-    }
-    consumed.push(current);
-    currentIndex++;
   }
 
   if (route.pathMatch === 'full' &&
-      (segmentGroup.hasChildren() || currentIndex < segments.length)) {
+      (segmentGroup.hasChildren() || parts.length < segments.length)) {
+    // The config is longer than the actual URL but we are looking for a full match, return null
     return null;
-  } else {
-    return {consumed, posParams};
   }
+
+  const posParams: {[key: string]: UrlSegment} = {};
+
+  // Check each config part against the actual URL
+  for (let index = 0; index < parts.length; index++) {
+    const part = parts[index];
+    const segment = segments[index];
+    const isParameter = part.startsWith(':');
+    if (isParameter) {
+      posParams[part.substring(1)] = segment;
+    } else if (part !== segment.path) {
+      // The actual URL part does not match the config, no match
+      return null;
+    }
+  }
+
+  return {consumed: segments.slice(0, parts.length), posParams};
 }

packages/router/src/url_tree.ts

@@ -207,21 +207,13 @@ export class UrlSegment {
   toString(): string { return serializePath(this); }
 }
 
-export function equalSegments(a: UrlSegment[], b: UrlSegment[]): boolean {
+export function equalSegments(as: UrlSegment[], bs: UrlSegment[]): boolean {
-  if (a.length !== b.length) return false;
+  return equalPath(as, bs) && as.every((a, i) => shallowEqual(a.parameters, bs[i].parameters));
-  for (let i = 0; i < a.length; ++i) {
-    if (a[i].path !== b[i].path) return false;
-    if (!shallowEqual(a[i].parameters, b[i].parameters)) return false;
-  }
-  return true;
 }
 
-export function equalPath(a: UrlSegment[], b: UrlSegment[]): boolean {
+export function equalPath(as: UrlSegment[], bs: UrlSegment[]): boolean {
-  if (a.length !== b.length) return false;
+  if (as.length !== bs.length) return false;
-  for (let i = 0; i < a.length; ++i) {
+  return as.every((a, i) => a.path === bs[i].path);
-    if (a[i].path !== b[i].path) return false;
-  }
-  return true;
 }
 
 export function mapChildrenIntoArray<T>(
@@ -288,8 +280,8 @@ export class DefaultUrlSerializer implements UrlSerializer {
   serialize(tree: UrlTree): string {
     const segment = `/${serializeSegment(tree.root, true)}`;
     const query = serializeQueryParams(tree.queryParams);
-    const fragment =
+    const fragment = typeof tree.fragment === `string` ? `#${encodeURI(tree.fragment)}` : '';
-        tree.fragment !== null && tree.fragment !== undefined ? `#${encodeURI(tree.fragment)}` : '';
+
     return `${segment}${query}${fragment}`;
   }
 }
@@ -301,34 +293,35 @@ export function serializePaths(segment: UrlSegmentGroup): string {
 }
 
 function serializeSegment(segment: UrlSegmentGroup, root: boolean): string {
-  if (segment.hasChildren() && root) {
+  if (!segment.hasChildren()) {
+    return serializePaths(segment);
+  }
+
+  if (root) {
     const primary = segment.children[PRIMARY_OUTLET] ?
         serializeSegment(segment.children[PRIMARY_OUTLET], false) :
         '';
     const children: string[] = [];
+
     forEach(segment.children, (v: UrlSegmentGroup, k: string) => {
       if (k !== PRIMARY_OUTLET) {
         children.push(`${k}:${serializeSegment(v, false)}`);
       }
     });
-    if (children.length > 0) {
-      return `${primary}(${children.join('//')})`;
-    } else {
-      return `${primary}`;
-    }
 
-  } else if (segment.hasChildren() && !root) {
+    return children.length > 0 ? `${primary}(${children.join('//')})` : primary;
+
+  } else {
     const children = mapChildrenIntoArray(segment, (v: UrlSegmentGroup, k: string) => {
       if (k === PRIMARY_OUTLET) {
         return [serializeSegment(segment.children[PRIMARY_OUTLET], false)];
-      } else {
-        return [`${k}:${serializeSegment(v, false)}`];
       }
-    });
-    return `${serializePaths(segment)}/(${children.join('//')})`;
 
-  } else {
+      return [`${k}:${serializeSegment(v, false)}`];
-    return serializePaths(segment);
+
+    });
+
+    return `${serializePaths(segment)}/(${children.join('//')})`;
   }
 }
 
@@ -360,7 +353,6 @@ function serializeQueryParams(params: {[key: string]: any}): string {
 
 const SEGMENT_RE = /^[^\/()?;=&#]+/;
 function matchSegments(str: string): string {
-  SEGMENT_RE.lastIndex = 0;
   const match = str.match(SEGMENT_RE);
   return match ? match[0] : '';
 }
@@ -368,7 +360,6 @@ function matchSegments(str: string): string {
 const QUERY_PARAM_RE = /^[^=?&#]+/;
 // Return the name of the query param at the start of the string or an empty string
 function matchQueryParams(str: string): string {
-  QUERY_PARAM_RE.lastIndex = 0;
   const match = str.match(SEGMENT_RE);
   return match ? match[0] : '';
 }
@@ -376,126 +367,101 @@ function matchQueryParams(str: string): string {
 const QUERY_PARAM_VALUE_RE = /^[^?&#]+/;
 // Return the value of the query param at the start of the string or an empty string
 function matchUrlQueryParamValue(str: string): string {
-  QUERY_PARAM_VALUE_RE.lastIndex = 0;
   const match = str.match(QUERY_PARAM_VALUE_RE);
   return match ? match[0] : '';
 }
 
 class UrlParser {
   private remaining: string;
+
   constructor(private url: string) { this.remaining = url; }
 
-  peekStartsWith(str: string): boolean { return this.remaining.startsWith(str); }
-
-  capture(str: string): void {
-    if (!this.remaining.startsWith(str)) {
-      throw new Error(`Expected "${str}".`);
-    }
-    this.remaining = this.remaining.substring(str.length);
-  }
-
   parseRootSegment(): UrlSegmentGroup {
-    if (this.remaining.startsWith('/')) {
+    this.consumeOptional('/');
-      this.capture('/');
-    }
 
-    if (this.remaining === '' || this.remaining.startsWith('?') || this.remaining.startsWith('#')) {
+    if (this.remaining === '' || this.peekStartsWith('?') || this.peekStartsWith('#')) {
       return new UrlSegmentGroup([], {});
     }
 
+    // The root segment group never has segments
     return new UrlSegmentGroup([], this.parseChildren());
   }
 
-  parseChildren(): {[key: string]: UrlSegmentGroup} {
+  parseQueryParams(): {[key: string]: any} {
-    if (this.remaining.length == 0) {
+    const params: {[key: string]: any} = {};
+    if (this.consumeOptional('?')) {
+      do {
+        this.parseQueryParam(params);
+      } while (this.consumeOptional('&'));
+    }
+    return params;
+  }
+
+  parseFragment(): string { return this.consumeOptional('#') ? decodeURI(this.remaining) : null; }
+
+  private parseChildren(): {[outlet: string]: UrlSegmentGroup} {
+    if (this.remaining === '') {
       return {};
     }
 
-    if (this.peekStartsWith('/')) {
+    this.consumeOptional('/');
-      this.capture('/');
-    }
 
-    const paths: any[] = [];
+    const segments: UrlSegment[] = [];
     if (!this.peekStartsWith('(')) {
-      paths.push(this.parseSegments());
+      segments.push(this.parseSegment());
     }
 
     while (this.peekStartsWith('/') && !this.peekStartsWith('//') && !this.peekStartsWith('/(')) {
       this.capture('/');
-      paths.push(this.parseSegments());
+      segments.push(this.parseSegment());
     }
 
-    let children: {[key: string]: UrlSegmentGroup} = {};
+    let children: {[outlet: string]: UrlSegmentGroup} = {};
     if (this.peekStartsWith('/(')) {
       this.capture('/');
       children = this.parseParens(true);
     }
 
-    let res: {[key: string]: UrlSegmentGroup} = {};
+    let res: {[outlet: string]: UrlSegmentGroup} = {};
     if (this.peekStartsWith('(')) {
       res = this.parseParens(false);
     }
 
-    if (paths.length > 0 || Object.keys(children).length > 0) {
+    if (segments.length > 0 || Object.keys(children).length > 0) {
-      res[PRIMARY_OUTLET] = new UrlSegmentGroup(paths, children);
+      res[PRIMARY_OUTLET] = new UrlSegmentGroup(segments, children);
     }
 
     return res;
   }
 
-  parseSegments(): UrlSegment {
+  // parse a segment with its matrix parameters
+  // ie `name;k1=v1;k2`
+  private parseSegment(): UrlSegment {
     const path = matchSegments(this.remaining);
     if (path === '' && this.peekStartsWith(';')) {
       throw new Error(`Empty path url segment cannot have parameters: '${this.remaining}'.`);
     }
 
     this.capture(path);
-    let matrixParams: {[key: string]: any} = {};
+    return new UrlSegment(decode(path), this.parseMatrixParams());
-    if (this.peekStartsWith(';')) {
-      matrixParams = this.parseMatrixParams();
-    }
-    return new UrlSegment(decode(path), matrixParams);
   }
 
-  parseQueryParams(): {[key: string]: any} {
+  private parseMatrixParams(): {[key: string]: any} {
     const params: {[key: string]: any} = {};
-    if (this.peekStartsWith('?')) {
+    while (this.consumeOptional(';')) {
-      this.capture('?');
-      this.parseQueryParam(params);
-      while (this.remaining.length > 0 && this.peekStartsWith('&')) {
-        this.capture('&');
-        this.parseQueryParam(params);
-      }
-    }
-    return params;
-  }
-
-  parseFragment(): string {
-    if (this.peekStartsWith('#')) {
-      return decodeURI(this.remaining.substring(1));
-    }
-
-    return null;
-  }
-
-  parseMatrixParams(): {[key: string]: any} {
-    const params: {[key: string]: any} = {};
-    while (this.remaining.length > 0 && this.peekStartsWith(';')) {
-      this.capture(';');
       this.parseParam(params);
     }
     return params;
   }
 
-  parseParam(params: {[key: string]: any}): void {
+  private parseParam(params: {[key: string]: any}): void {
     const key = matchSegments(this.remaining);
     if (!key) {
       return;
     }
     this.capture(key);
     let value: any = '';
-    if (this.peekStartsWith('=')) {
+    if (this.consumeOptional('=')) {
-      this.capture('=');
       const valueMatch = matchSegments(this.remaining);
       if (valueMatch) {
         value = valueMatch;
@@ -507,15 +473,14 @@ class UrlParser {
   }
 
   // Parse a single query parameter `name[=value]`
-  parseQueryParam(params: {[key: string]: any}): void {
+  private parseQueryParam(params: {[key: string]: any}): void {
     const key = matchQueryParams(this.remaining);
     if (!key) {
       return;
     }
     this.capture(key);
     let value: any = '';
-    if (this.peekStartsWith('=')) {
+    if (this.consumeOptional('=')) {
-      this.capture('=');
       const valueMatch = matchUrlQueryParamValue(this.remaining);
       if (valueMatch) {
         value = valueMatch;
@@ -540,10 +505,12 @@ class UrlParser {
     }
   }
 
-  parseParens(allowPrimary: boolean): {[key: string]: UrlSegmentGroup} {
+  // parse `(a/b//outlet_name:c/d)`
+  private parseParens(allowPrimary: boolean): {[outlet: string]: UrlSegmentGroup} {
     const segments: {[key: string]: UrlSegmentGroup} = {};
     this.capture('(');
-    while (!this.peekStartsWith(')') && this.remaining.length > 0) {
+
+    while (!this.consumeOptional(')') && this.remaining.length > 0) {
       const path = matchSegments(this.remaining);
 
       const next = this.remaining[path.length];
@@ -566,11 +533,26 @@ class UrlParser {
       const children = this.parseChildren();
       segments[outletName] = Object.keys(children).length === 1 ? children[PRIMARY_OUTLET] :
                                                                   new UrlSegmentGroup([], children);
-      if (this.peekStartsWith('//')) {
+      this.consumeOptional('//');
-        this.capture('//');
-      }
     }
-    this.capture(')');
+
     return segments;
   }
+
+  private peekStartsWith(str: string): boolean { return this.remaining.startsWith(str); }
+
+  // Consumes the prefix when it is present and returns whether it has been consumed
+  private consumeOptional(str: string): boolean {
+    if (this.peekStartsWith(str)) {
+      this.remaining = this.remaining.substring(str.length);
+      return true;
+    }
+    return false;
+  }
+
+  private capture(str: string): void {
+    if (!this.consumeOptional(str)) {
+      throw new Error(`Expected "${str}".`);
+    }
+  }
 }

packages/router/src/utils/collection.ts

@@ -45,10 +45,6 @@ export function flatten<T>(arr: T[][]): T[] {
   return Array.prototype.concat.apply([], arr);
 }
 
-export function first<T>(a: T[]): T {
-  return a.length > 0 ? a[0] : null;
-}
-
 export function last<T>(a: T[]): T {
   return a.length > 0 ? a[a.length - 1] : null;
 }
@@ -67,34 +63,26 @@ export function forEach<K, V>(map: {[key: string]: V}, callback: (v: V, k: strin
 
 export function waitForMap<A, B>(
     obj: {[k: string]: A}, fn: (k: string, a: A) => Observable<B>): Observable<{[k: string]: B}> {
-  const waitFor: Observable<B>[] = [];
+  if (Object.keys(obj).length === 0) {
+    return of ({})
+  }
+
+  const waitHead: Observable<B>[] = [];
+  const waitTail: Observable<B>[] = [];
   const res: {[k: string]: B} = {};
 
   forEach(obj, (a: A, k: string) => {
+    const mapped = map.call(fn(k, a), (r: B) => res[k] = r);
     if (k === PRIMARY_OUTLET) {
-      waitFor.push(map.call(fn(k, a), (_: B) => {
+      waitHead.push(mapped);
-        res[k] = _;
+    } else {
-        return _;
+      waitTail.push(mapped);
-      }));
     }
   });
 
-  forEach(obj, (a: A, k: string) => {
+  const concat$ = concatAll.call(of (...waitHead, ...waitTail));
-    if (k !== PRIMARY_OUTLET) {
+  const last$ = l.last.call(concat$);
-      waitFor.push(map.call(fn(k, a), (_: B) => {
+  return map.call(last$, () => res);
-        res[k] = _;
-        return _;
-      }));
-    }
-  });
-
-  if (waitFor.length > 0) {
-    const concatted$ = concatAll.call(of (...waitFor));
-    const last$ = l.last.call(concatted$);
-    return map.call(last$, () => res);
-  }
-
-  return of (res);
 }
 
 export function andObservables(observables: Observable<Observable<any>>): Observable<boolean> {