X Tutup
Skip to content

Commit aa85856

Browse files
btfordbtford
committed
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
1 parent fb4f1e8 commit aa85856

File tree

9 files changed

+139
-12
lines changed

9 files changed

+139
-12
lines changed

‎modules/angular2/src/mock/location_mock.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,16 @@ export class SpyLocation implements Location {
2121

2222
path(): string { return this._path; }
2323

24-
simulateUrlPop(pathname: string) { ObservableWrapper.callEmit(this._subject, {'url': pathname}); }
24+
simulateUrlPop(pathname: string) {
25+
ObservableWrapper.callEmit(this._subject, {'url': pathname, 'pop': true});
26+
}
27+
28+
simulateHashChange(pathname: string) {
29+
// Because we don't prevent the native event, the browser will independently update the path
30+
this.setInitialPath(pathname);
31+
this.urlChanges.push('hash: ' + pathname);
32+
ObservableWrapper.callEmit(this._subject, {'url': pathname, 'pop': true, 'type': 'hashchange'});
33+
}
2534

2635
prepareExternalUrl(url: string): string {
2736
if (url.length > 0 && !url.startsWith('/')) {
@@ -42,6 +51,15 @@ export class SpyLocation implements Location {
4251
this.urlChanges.push(url);
4352
}
4453

54+
replaceState(path: string, query: string = '') {
55+
path = this.prepareExternalUrl(path);
56+
this._path = path;
57+
this._query = query;
58+
59+
var url = path + (query.length > 0 ? ('?' + query) : '');
60+
this.urlChanges.push('replace: ' + url);
61+
}
62+
4563
forward() {
4664
// TODO
4765
}

‎modules/angular2/src/mock/mock_location_strategy.ts‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export class MockLocationStrategy extends LocationStrategy {
1515

1616
simulatePopState(url: string): void {
1717
this.internalPath = url;
18-
ObservableWrapper.callEmit(this._subject, null);
18+
ObservableWrapper.callEmit(this._subject, new MockPopStateEvent(this.path()));
1919
}
2020

2121
path(): string { return this.internalPath; }
@@ -27,10 +27,6 @@ export class MockLocationStrategy extends LocationStrategy {
2727
return this.internalBaseHref + internal;
2828
}
2929

30-
simulateUrlPop(pathname: string): void {
31-
ObservableWrapper.callEmit(this._subject, {'url': pathname});
32-
}
33-
3430
pushState(ctx: any, title: string, path: string, query: string): void {
3531
this.internalTitle = title;
3632

@@ -41,6 +37,16 @@ export class MockLocationStrategy extends LocationStrategy {
4137
this.urlChanges.push(externalUrl);
4238
}
4339

40+
replaceState(ctx: any, title: string, path: string, query: string): void {
41+
this.internalTitle = title;
42+
43+
var url = path + (query.length > 0 ? ('?' + query) : '');
44+
this.internalPath = url;
45+
46+
var externalUrl = this.prepareExternalUrl(url);
47+
this.urlChanges.push('replace: ' + externalUrl);
48+
}
49+
4450
onPopState(fn: (value: any) => void): void { ObservableWrapper.subscribe(this._subject, fn); }
4551

4652
getBaseHref(): string { return this.internalBaseHref; }
@@ -55,3 +61,9 @@ export class MockLocationStrategy extends LocationStrategy {
5561

5662
forward(): void { throw 'not implemented'; }
5763
}
64+
65+
class MockPopStateEvent {
66+
pop: boolean = true;
67+
type: string = 'popstate';
68+
constructor(public newUrl: string) {}
69+
}

‎modules/angular2/src/router/hash_location_strategy.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,10 @@ export class HashLocationStrategy extends LocationStrategy {
5858
}
5959
}
6060

61-
onPopState(fn: EventListener): void { this._platformLocation.onPopState(fn); }
61+
onPopState(fn: EventListener): void {
62+
this._platformLocation.onPopState(fn);
63+
this._platformLocation.onHashChange(fn);
64+
}
6265

6366
getBaseHref(): string { return this._baseHref; }
6467

@@ -87,6 +90,14 @@ export class HashLocationStrategy extends LocationStrategy {
8790
this._platformLocation.pushState(state, title, url);
8891
}
8992

93+
replaceState(state: any, title: string, path: string, queryParams: string) {
94+
var url = this.prepareExternalUrl(path + normalizeQueryParams(queryParams));
95+
if (url.length == 0) {
96+
url = this._platformLocation.pathname;
97+
}
98+
this._platformLocation.replaceState(state, title, url);
99+
}
100+
90101
forward(): void { this._platformLocation.forward(); }
91102

92103
back(): void { this._platformLocation.back(); }

‎modules/angular2/src/router/location.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,9 @@ export class Location {
5252
constructor(public platformStrategy: LocationStrategy) {
5353
var browserBaseHref = this.platformStrategy.getBaseHref();
5454
this._baseHref = stripTrailingSlash(stripIndexHtml(browserBaseHref));
55-
this.platformStrategy.onPopState(
56-
(_) => { ObservableWrapper.callEmit(this._subject, {'url': this.path(), 'pop': true}); });
55+
this.platformStrategy.onPopState((ev) => {
56+
ObservableWrapper.callEmit(this._subject, {'url': this.path(), 'pop': true, 'type': ev.type});
57+
});
5758
}
5859

5960
/**
@@ -82,6 +83,7 @@ export class Location {
8283
return this.platformStrategy.prepareExternalUrl(url);
8384
}
8485

86+
// TODO: rename this method to pushState
8587
/**
8688
* Changes the browsers URL to the normalized version of the given URL, and pushes a
8789
* new item onto the platform's history.
@@ -90,6 +92,14 @@ export class Location {
9092
this.platformStrategy.pushState(null, '', path, query);
9193
}
9294

95+
/**
96+
* Changes the browsers URL to the normalized version of the given URL, and replaces
97+
* the top item on the platform's history stack.
98+
*/
99+
replaceState(path: string, query: string = ''): void {
100+
this.platformStrategy.replaceState(null, '', path, query);
101+
}
102+
93103
/**
94104
* Navigates forward in the platform's history.
95105
*/

‎modules/angular2/src/router/location_strategy.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export abstract class LocationStrategy {
2121
abstract path(): string;
2222
abstract prepareExternalUrl(internal: string): string;
2323
abstract pushState(state: any, title: string, url: string, queryParams: string): void;
24+
abstract replaceState(state: any, title: string, url: string, queryParams: string): void;
2425
abstract forward(): void;
2526
abstract back(): void;
2627
abstract onPopState(fn: (_: any) => any): void;

‎modules/angular2/src/router/path_location_strategy.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,11 @@ export class PathLocationStrategy extends LocationStrategy {
9393
this._platformLocation.pushState(state, title, externalUrl);
9494
}
9595

96+
replaceState(state: any, title: string, url: string, queryParams: string) {
97+
var externalUrl = this.prepareExternalUrl(url + normalizeQueryParams(queryParams));
98+
this._platformLocation.replaceState(state, title, externalUrl);
99+
}
100+
96101
forward(): void { this._platformLocation.forward(); }
97102

98103
back(): void { this._platformLocation.back(); }

‎modules/angular2/src/router/platform_location.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ export class PlatformLocation {
4040
this._history.pushState(state, title, url);
4141
}
4242

43+
replaceState(state: any, title: string, url: string): void {
44+
this._history.replaceState(state, title, url);
45+
}
46+
4347
forward(): void { this._history.forward(); }
4448

4549
back(): void { this._history.back(); }

‎modules/angular2/src/router/router.ts‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -425,8 +425,38 @@ export class RootRouter extends Router {
425425
@Inject(ROUTER_PRIMARY_COMPONENT) primaryComponent: Type) {
426426
super(registry, null, primaryComponent);
427427
this._location = location;
428-
this._locationSub = this._location.subscribe(
429-
(change) => this.navigateByUrl(change['url'], isPresent(change['pop'])));
428+
this._locationSub = this._location.subscribe((change) => {
429+
// we call recognize ourselves
430+
this.recognize(change['url'])
431+
.then((instruction) => {
432+
this.navigateByInstruction(instruction, isPresent(change['pop']))
433+
.then((_) => {
434+
// this is a popstate event; no need to change the URL
435+
if (isPresent(change['pop']) && change['type'] != 'hashchange') {
436+
return;
437+
}
438+
var emitPath = instruction.toUrlPath();
439+
var emitQuery = instruction.toUrlQuery();
440+
if (emitPath.length > 0) {
441+
emitPath = '/' + emitPath;
442+
}
443+
444+
// Because we've opted to use All hashchange events occur outside Angular.
445+
// However, apps that are migrating might have hash links that operate outside
446+
// angular to which routing must respond.
447+
// To support these cases where we respond to hashchanges and redirect as a
448+
// result, we need to replace the top item on the stack.
449+
if (change['type'] == 'hashchange') {
450+
if (instruction.toRootUrl() != this._location.path()) {
451+
this._location.replaceState(emitPath, emitQuery);
452+
}
453+
} else {
454+
this._location.go(emitPath, emitQuery);
455+
}
456+
});
457+
});
458+
});
459+
430460
this.registry.configFromComponent(primaryComponent);
431461
this.navigateByUrl(location.path());
432462
}

‎modules/angular2/test/router/router_spec.ts‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {SpyLocation} from 'angular2/src/mock/location_mock';
2020
import {Location} from 'angular2/src/router/location';
2121

2222
import {RouteRegistry, ROUTER_PRIMARY_COMPONENT} from 'angular2/src/router/route_registry';
23-
import {RouteConfig, AsyncRoute, Route} from 'angular2/src/router/route_config_decorator';
23+
import {RouteConfig, AsyncRoute, Route, Redirect} from 'angular2/src/router/route_config_decorator';
2424
import {DirectiveResolver} from 'angular2/src/core/linker/directive_resolver';
2525

2626
import {provide} from 'angular2/core';
@@ -99,6 +99,42 @@ export function main() {
9999
});
100100
}));
101101

102+
// See https://github.com/angular/angular/issues/5590
103+
it('should replace history when triggered by a hashchange with a redirect',
104+
inject([AsyncTestCompleter], (async) => {
105+
var outlet = makeDummyOutlet();
106+
107+
router.registerPrimaryOutlet(outlet)
108+
.then((_) => router.config([
109+
new Redirect({path: '/a', redirectTo: ['B']}),
110+
new Route({path: '/b', component: DummyComponent, name: 'B'})
111+
]))
112+
.then((_) => {
113+
router.subscribe((_) => {
114+
expect(location.urlChanges).toEqual(['hash: a', 'replace: /b']);
115+
async.done();
116+
});
117+
118+
location.simulateHashChange('a');
119+
});
120+
}));
121+
122+
it('should push history when triggered by a hashchange without a redirect',
123+
inject([AsyncTestCompleter], (async) => {
124+
var outlet = makeDummyOutlet();
125+
126+
router.registerPrimaryOutlet(outlet)
127+
.then((_) => router.config([new Route({path: '/a', component: DummyComponent})]))
128+
.then((_) => {
129+
router.subscribe((_) => {
130+
expect(location.urlChanges).toEqual(['hash: a']);
131+
async.done();
132+
});
133+
134+
location.simulateHashChange('a');
135+
});
136+
}));
137+
102138
it('should navigate after being configured', inject([AsyncTestCompleter], (async) => {
103139
var outlet = makeDummyOutlet();
104140

0 commit comments

Comments
 (0)
X Tutup