fix(router): properly read and serialize query params

This splits out `path` and `query` into separate params for `location.go`
and related methods so that we can handle them properly in both `PathLocationStrategy`
and `HashLocationStrategy`.

This handles the problem of not reading query params to populate `Location` on the
initial page load.

Closes #3957
Closes #4225
Closes #3784

modules/angular2/src/mock/location_mock.ts

@@ -7,6 +7,8 @@ export class SpyLocation implements Location {
   /** @internal */
   _path: string = '';
   /** @internal */
+  _query: string = '';
+  /** @internal */
   _subject: EventEmitter = new EventEmitter();
   /** @internal */
   _baseHref: string = '';
@@ -21,12 +23,15 @@ export class SpyLocation implements Location {
 
   normalizeAbsolutely(url: string): string { return this._baseHref + url; }
 
-  go(url: string) {
-    url = this.normalizeAbsolutely(url);
-    if (this._path == url) {
+  go(path: string, query: string = '') {
+    path = this.normalizeAbsolutely(path);
+    if (this._path == path && this._query == query) {
       return;
     }
-    this._path = url;
+    this._path = path;
+    this._query = query;
+
+    var url = path + (query.length > 0 ? ('?' + query) : '');
     this.urlChanges.push(url);
   }
 

modules/angular2/src/mock/mock_location_strategy.ts

@@ -22,8 +22,10 @@ export class MockLocationStrategy extends LocationStrategy {
     ObservableWrapper.callNext(this._subject, {'url': pathname});
   }
 
-  pushState(ctx: any, title: string, url: string): void {
+  pushState(ctx: any, title: string, path: string, query: string): void {
     this.internalTitle = title;
+
+    var url = path + (query.length > 0 ? ('?' + query) : '');
     this.internalPath = url;
     this.urlChanges.push(url);
   }

modules/angular2/src/router/hash_location_strategy.ts

@@ -1,6 +1,6 @@
 import {DOM} from 'angular2/src/core/dom/dom_adapter';
 import {Injectable} from 'angular2/angular2';
-import {LocationStrategy} from './location_strategy';
+import {LocationStrategy, normalizeQueryParams} from './location_strategy';
 import {EventListener, History, Location} from 'angular2/src/core/facade/browser';
 
 /**
@@ -61,11 +61,18 @@ export class HashLocationStrategy extends LocationStrategy {
     // Dart will complain if a call to substring is
     // executed with a position value that extends the
     // length of string.
-    return path.length > 0 ? path.substring(1) : path;
+    return (path.length > 0 ? path.substring(1) : path) +
+           normalizeQueryParams(this._location.search);
   }
 
-  pushState(state: any, title: string, url: string) {
-    this._history.pushState(state, title, '#' + url);
+  pushState(state: any, title: string, path: string, queryParams: string) {
+    var url = path + normalizeQueryParams(queryParams);
+    if (url.length == 0) {
+      url = this._location.pathname;
+    } else {
+      url = '#' + url;
+    }
+    this._history.pushState(state, title, url);
   }
 
   forward(): void { this._history.forward(); }

modules/angular2/src/router/instruction.ts

@@ -94,12 +94,18 @@ export class PrimaryInstruction {
 }
 
 export function stringifyInstruction(instruction: Instruction): string {
-  var params = instruction.component.urlParams.length > 0 ?
-                   ('?' + instruction.component.urlParams.join('&')) :
-                   '';
+  return stringifyInstructionPath(instruction) + stringifyInstructionQuery(instruction);
+}
 
+export function stringifyInstructionPath(instruction: Instruction): string {
   return instruction.component.urlPath + stringifyAux(instruction) +
-         stringifyPrimary(instruction.child) + params;
+         stringifyPrimary(instruction.child);
+}
+
+export function stringifyInstructionQuery(instruction: Instruction): string {
+  return instruction.component.urlParams.length > 0 ?
+             ('?' + instruction.component.urlParams.join('&')) :
+             '';
 }
 
 function stringifyPrimary(instruction: Instruction): string {

modules/angular2/src/router/location.ts

@@ -125,9 +125,9 @@ export class Location {
    * Changes the browsers URL to the normalized version of the given URL, and pushes a
    * new item onto the platform's history.
    */
-  go(url: string): void {
-    var finalUrl = this.normalizeAbsolutely(url);
-    this.platformStrategy.pushState(null, '', finalUrl);
+  go(path: string, query: string = ''): void {
+    var absolutePath = this.normalizeAbsolutely(path);
+    this.platformStrategy.pushState(null, '', absolutePath, query);
   }
 
   /**

modules/angular2/src/router/location_strategy.ts

@@ -16,9 +16,13 @@
  */
 export abstract class LocationStrategy {
   abstract path(): string;
-  abstract pushState(ctx: any, title: string, url: string): void;
+  abstract pushState(state: any, title: string, url: string, queryParams: string): void;
   abstract forward(): void;
   abstract back(): void;
   abstract onPopState(fn: (_: any) => any): void;
   abstract getBaseHref(): string;
 }
+
+export function normalizeQueryParams(params: string): string {
+  return (params.length > 0 && params.substring(0, 1) != '?') ? ('?' + params) : params;
+}

modules/angular2/src/router/path_location_strategy.ts

@@ -1,7 +1,7 @@
 import {DOM} from 'angular2/src/core/dom/dom_adapter';
 import {Injectable} from 'angular2/angular2';
 import {EventListener, History, Location} from 'angular2/src/core/facade/browser';
-import {LocationStrategy} from './location_strategy';
+import {LocationStrategy, normalizeQueryParams} from './location_strategy';
 
 /**
  * `PathLocationStrategy` is a {@link LocationStrategy} used to configure the
@@ -67,9 +67,11 @@ export class PathLocationStrategy extends LocationStrategy {
 
   getBaseHref(): string { return this._baseHref; }
 
-  path(): string { return this._location.pathname; }
+  path(): string { return this._location.pathname + normalizeQueryParams(this._location.search); }
 
-  pushState(state: any, title: string, url: string) { this._history.pushState(state, title, url); }
+  pushState(state: any, title: string, url: string, queryParams: string) {
+    this._history.pushState(state, title, (url + normalizeQueryParams(queryParams)));
+  }
 
   forward(): void { this._history.forward(); }
 

modules/angular2/src/router/router.ts

@@ -15,7 +15,13 @@ import {
 } from 'angular2/src/core/facade/lang';
 import {BaseException, WrappedException} from 'angular2/src/core/facade/exceptions';
 import {RouteRegistry} from './route_registry';
-import {ComponentInstruction, Instruction, stringifyInstruction} from './instruction';
+import {
+  ComponentInstruction,
+  Instruction,
+  stringifyInstruction,
+  stringifyInstructionPath,
+  stringifyInstructionQuery
+} from './instruction';
 import {RouterOutlet} from './router_outlet';
 import {Location} from './location';
 import {getCanActivateHook} from './route_lifecycle_reflector';
@@ -472,13 +478,14 @@ export class RootRouter extends Router {
   }
 
   commit(instruction: Instruction, _skipLocationChange: boolean = false): Promise<any> {
-    var emitUrl = stringifyInstruction(instruction);
-    if (emitUrl.length > 0) {
-      emitUrl = '/' + emitUrl;
+    var emitPath = stringifyInstructionPath(instruction);
+    var emitQuery = stringifyInstructionQuery(instruction);
+    if (emitPath.length > 0) {
+      emitPath = '/' + emitPath;
     }
     var promise = super.commit(instruction);
     if (!_skipLocationChange) {
-      promise = promise.then((_) => { this._location.go(emitUrl); });
+      promise = promise.then((_) => { this._location.go(emitPath, emitQuery); });
     }
     return promise;
   }

modules/angular2/src/router/url_parser.ts

@@ -70,10 +70,10 @@ export function pathSegmentsToUrl(pathSegments: string[]): Url {
   return url;
 }
 
-var SEGMENT_RE = RegExpWrapper.create('^[^\\/\\(\\)\\?;=&]+');
+var SEGMENT_RE = RegExpWrapper.create('^[^\\/\\(\\)\\?;=&#]+');
 function matchUrlSegment(str: string): string {
   var match = RegExpWrapper.firstMatch(SEGMENT_RE, str);
-  return isPresent(match) ? match[0] : null;
+  return isPresent(match) ? match[0] : '';
 }
 
 export class UrlParser {