From 8bc40d3f4da8244b7cd6bea2f0eeb444ecd5d676 Mon Sep 17 00:00:00 2001 From: Brian Ford Date: Wed, 23 Sep 2015 00:10:26 -0700 Subject: [PATCH] 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 | 13 +++++++++---- .../angular2/src/mock/mock_location_strategy.ts | 4 +++- .../src/router/hash_location_strategy.ts | 15 +++++++++++---- modules/angular2/src/router/instruction.ts | 14 ++++++++++---- modules/angular2/src/router/location.ts | 6 +++--- .../angular2/src/router/location_strategy.ts | 6 +++++- .../src/router/path_location_strategy.ts | 8 +++++--- modules/angular2/src/router/router.ts | 17 ++++++++++++----- modules/angular2/src/router/url_parser.ts | 4 ++-- 9 files changed, 60 insertions(+), 27 deletions(-) diff --git a/modules/angular2/src/mock/location_mock.ts b/modules/angular2/src/mock/location_mock.ts index 640a8e6c5e..00f8bee13d 100644 --- a/modules/angular2/src/mock/location_mock.ts +++ b/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); } diff --git a/modules/angular2/src/mock/mock_location_strategy.ts b/modules/angular2/src/mock/mock_location_strategy.ts index 02a5a2d86b..de2c35fb8c 100644 --- a/modules/angular2/src/mock/mock_location_strategy.ts +++ b/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); } diff --git a/modules/angular2/src/router/hash_location_strategy.ts b/modules/angular2/src/router/hash_location_strategy.ts index cf78094e74..53f060dc93 100644 --- a/modules/angular2/src/router/hash_location_strategy.ts +++ b/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(); } diff --git a/modules/angular2/src/router/instruction.ts b/modules/angular2/src/router/instruction.ts index 26571d1eb6..83bbf66372 100644 --- a/modules/angular2/src/router/instruction.ts +++ b/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 { diff --git a/modules/angular2/src/router/location.ts b/modules/angular2/src/router/location.ts index 2646bb81b1..aec47458b7 100644 --- a/modules/angular2/src/router/location.ts +++ b/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); } /** diff --git a/modules/angular2/src/router/location_strategy.ts b/modules/angular2/src/router/location_strategy.ts index fb944cfcec..4bd56e822e 100644 --- a/modules/angular2/src/router/location_strategy.ts +++ b/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; +} diff --git a/modules/angular2/src/router/path_location_strategy.ts b/modules/angular2/src/router/path_location_strategy.ts index ad316414dc..05a6545fab 100644 --- a/modules/angular2/src/router/path_location_strategy.ts +++ b/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(); } diff --git a/modules/angular2/src/router/router.ts b/modules/angular2/src/router/router.ts index 07856fde98..07dace8939 100644 --- a/modules/angular2/src/router/router.ts +++ b/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 { - 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; } diff --git a/modules/angular2/src/router/url_parser.ts b/modules/angular2/src/router/url_parser.ts index fb52789374..4fc5f445d5 100644 --- a/modules/angular2/src/router/url_parser.ts +++ b/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 {