fix(router): set correct redirect/default URL from hashchange

Currently, hashchange events outside of Angular that cause navigation
do not take into account cases where the initial route URL changes
due to a redirect or a default route.

Closes #5590

Closes #5683
This commit is contained in:
Brian Ford 2015-12-07 16:05:57 -08:00
parent fb4f1e8dc9
commit aa85856e1c
9 changed files with 139 additions and 12 deletions

View File

@ -21,7 +21,16 @@ export class SpyLocation implements Location {
path(): string { return this._path; }
simulateUrlPop(pathname: string) { ObservableWrapper.callEmit(this._subject, {'url': pathname}); }
simulateUrlPop(pathname: string) {
ObservableWrapper.callEmit(this._subject, {'url': pathname, 'pop': true});
}
simulateHashChange(pathname: string) {
// Because we don't prevent the native event, the browser will independently update the path
this.setInitialPath(pathname);
this.urlChanges.push('hash: ' + pathname);
ObservableWrapper.callEmit(this._subject, {'url': pathname, 'pop': true, 'type': 'hashchange'});
}
prepareExternalUrl(url: string): string {
if (url.length > 0 && !url.startsWith('/')) {
@ -42,6 +51,15 @@ export class SpyLocation implements Location {
this.urlChanges.push(url);
}
replaceState(path: string, query: string = '') {
path = this.prepareExternalUrl(path);
this._path = path;
this._query = query;
var url = path + (query.length > 0 ? ('?' + query) : '');
this.urlChanges.push('replace: ' + url);
}
forward() {
// TODO
}

View File

@ -15,7 +15,7 @@ export class MockLocationStrategy extends LocationStrategy {
simulatePopState(url: string): void {
this.internalPath = url;
ObservableWrapper.callEmit(this._subject, null);
ObservableWrapper.callEmit(this._subject, new MockPopStateEvent(this.path()));
}
path(): string { return this.internalPath; }
@ -27,10 +27,6 @@ export class MockLocationStrategy extends LocationStrategy {
return this.internalBaseHref + internal;
}
simulateUrlPop(pathname: string): void {
ObservableWrapper.callEmit(this._subject, {'url': pathname});
}
pushState(ctx: any, title: string, path: string, query: string): void {
this.internalTitle = title;
@ -41,6 +37,16 @@ export class MockLocationStrategy extends LocationStrategy {
this.urlChanges.push(externalUrl);
}
replaceState(ctx: any, title: string, path: string, query: string): void {
this.internalTitle = title;
var url = path + (query.length > 0 ? ('?' + query) : '');
this.internalPath = url;
var externalUrl = this.prepareExternalUrl(url);
this.urlChanges.push('replace: ' + externalUrl);
}
onPopState(fn: (value: any) => void): void { ObservableWrapper.subscribe(this._subject, fn); }
getBaseHref(): string { return this.internalBaseHref; }
@ -55,3 +61,9 @@ export class MockLocationStrategy extends LocationStrategy {
forward(): void { throw 'not implemented'; }
}
class MockPopStateEvent {
pop: boolean = true;
type: string = 'popstate';
constructor(public newUrl: string) {}
}

View File

@ -58,7 +58,10 @@ export class HashLocationStrategy extends LocationStrategy {
}
}
onPopState(fn: EventListener): void { this._platformLocation.onPopState(fn); }
onPopState(fn: EventListener): void {
this._platformLocation.onPopState(fn);
this._platformLocation.onHashChange(fn);
}
getBaseHref(): string { return this._baseHref; }
@ -87,6 +90,14 @@ export class HashLocationStrategy extends LocationStrategy {
this._platformLocation.pushState(state, title, url);
}
replaceState(state: any, title: string, path: string, queryParams: string) {
var url = this.prepareExternalUrl(path + normalizeQueryParams(queryParams));
if (url.length == 0) {
url = this._platformLocation.pathname;
}
this._platformLocation.replaceState(state, title, url);
}
forward(): void { this._platformLocation.forward(); }
back(): void { this._platformLocation.back(); }

View File

@ -52,8 +52,9 @@ export class Location {
constructor(public platformStrategy: LocationStrategy) {
var browserBaseHref = this.platformStrategy.getBaseHref();
this._baseHref = stripTrailingSlash(stripIndexHtml(browserBaseHref));
this.platformStrategy.onPopState(
(_) => { ObservableWrapper.callEmit(this._subject, {'url': this.path(), 'pop': true}); });
this.platformStrategy.onPopState((ev) => {
ObservableWrapper.callEmit(this._subject, {'url': this.path(), 'pop': true, 'type': ev.type});
});
}
/**
@ -82,6 +83,7 @@ export class Location {
return this.platformStrategy.prepareExternalUrl(url);
}
// TODO: rename this method to pushState
/**
* Changes the browsers URL to the normalized version of the given URL, and pushes a
* new item onto the platform's history.
@ -90,6 +92,14 @@ export class Location {
this.platformStrategy.pushState(null, '', path, query);
}
/**
* Changes the browsers URL to the normalized version of the given URL, and replaces
* the top item on the platform's history stack.
*/
replaceState(path: string, query: string = ''): void {
this.platformStrategy.replaceState(null, '', path, query);
}
/**
* Navigates forward in the platform's history.
*/

View File

@ -21,6 +21,7 @@ export abstract class LocationStrategy {
abstract path(): string;
abstract prepareExternalUrl(internal: string): string;
abstract pushState(state: any, title: string, url: string, queryParams: string): void;
abstract replaceState(state: any, title: string, url: string, queryParams: string): void;
abstract forward(): void;
abstract back(): void;
abstract onPopState(fn: (_: any) => any): void;

View File

@ -93,6 +93,11 @@ export class PathLocationStrategy extends LocationStrategy {
this._platformLocation.pushState(state, title, externalUrl);
}
replaceState(state: any, title: string, url: string, queryParams: string) {
var externalUrl = this.prepareExternalUrl(url + normalizeQueryParams(queryParams));
this._platformLocation.replaceState(state, title, externalUrl);
}
forward(): void { this._platformLocation.forward(); }
back(): void { this._platformLocation.back(); }

View File

@ -40,6 +40,10 @@ export class PlatformLocation {
this._history.pushState(state, title, url);
}
replaceState(state: any, title: string, url: string): void {
this._history.replaceState(state, title, url);
}
forward(): void { this._history.forward(); }
back(): void { this._history.back(); }

View File

@ -425,8 +425,38 @@ export class RootRouter extends Router {
@Inject(ROUTER_PRIMARY_COMPONENT) primaryComponent: Type) {
super(registry, null, primaryComponent);
this._location = location;
this._locationSub = this._location.subscribe(
(change) => this.navigateByUrl(change['url'], isPresent(change['pop'])));
this._locationSub = this._location.subscribe((change) => {
// we call recognize ourselves
this.recognize(change['url'])
.then((instruction) => {
this.navigateByInstruction(instruction, isPresent(change['pop']))
.then((_) => {
// this is a popstate event; no need to change the URL
if (isPresent(change['pop']) && change['type'] != 'hashchange') {
return;
}
var emitPath = instruction.toUrlPath();
var emitQuery = instruction.toUrlQuery();
if (emitPath.length > 0) {
emitPath = '/' + emitPath;
}
// Because we've opted to use All hashchange events occur outside Angular.
// However, apps that are migrating might have hash links that operate outside
// angular to which routing must respond.
// To support these cases where we respond to hashchanges and redirect as a
// result, we need to replace the top item on the stack.
if (change['type'] == 'hashchange') {
if (instruction.toRootUrl() != this._location.path()) {
this._location.replaceState(emitPath, emitQuery);
}
} else {
this._location.go(emitPath, emitQuery);
}
});
});
});
this.registry.configFromComponent(primaryComponent);
this.navigateByUrl(location.path());
}

View File

@ -20,7 +20,7 @@ import {SpyLocation} from 'angular2/src/mock/location_mock';
import {Location} from 'angular2/src/router/location';
import {RouteRegistry, ROUTER_PRIMARY_COMPONENT} from 'angular2/src/router/route_registry';
import {RouteConfig, AsyncRoute, Route} from 'angular2/src/router/route_config_decorator';
import {RouteConfig, AsyncRoute, Route, Redirect} from 'angular2/src/router/route_config_decorator';
import {DirectiveResolver} from 'angular2/src/core/linker/directive_resolver';
import {provide} from 'angular2/core';
@ -99,6 +99,42 @@ export function main() {
});
}));
// See https://github.com/angular/angular/issues/5590
it('should replace history when triggered by a hashchange with a redirect',
inject([AsyncTestCompleter], (async) => {
var outlet = makeDummyOutlet();
router.registerPrimaryOutlet(outlet)
.then((_) => router.config([
new Redirect({path: '/a', redirectTo: ['B']}),
new Route({path: '/b', component: DummyComponent, name: 'B'})
]))
.then((_) => {
router.subscribe((_) => {
expect(location.urlChanges).toEqual(['hash: a', 'replace: /b']);
async.done();
});
location.simulateHashChange('a');
});
}));
it('should push history when triggered by a hashchange without a redirect',
inject([AsyncTestCompleter], (async) => {
var outlet = makeDummyOutlet();
router.registerPrimaryOutlet(outlet)
.then((_) => router.config([new Route({path: '/a', component: DummyComponent})]))
.then((_) => {
router.subscribe((_) => {
expect(location.urlChanges).toEqual(['hash: a']);
async.done();
});
location.simulateHashChange('a');
});
}));
it('should navigate after being configured', inject([AsyncTestCompleter], (async) => {
var outlet = makeDummyOutlet();