Skip to content

Commit c6a9300

Browse files
aahmedayedatscott
aahmedayed
authored and
atscott
committed
fix(common): synchronise location mock behavior with the navigators (angular#41730)
* Do not emit url pop on Location.go * Emit a `popstate` event before each `hashchange` to have the same behavior of the browser. * Track the url change in the internal history when calling `simulateHashChange` The changes to the router tests reflect the goals of the test. Generally when `Location.go` is used to trigger navigations, it is only relevant for `HashLocationStrategy` and verifying that the Router picks up changes from manual URL changes. To do this, we convert those calls to `simulateHashChange` instead. Manual URL bar changes to the path when not using the `HashLocationStrategy` would otherwise trigger a full page refresh so they aren't relevant to these test scenarios which assert correct behavior during the lifetime of the router. [Reference for no `popstate` on `pushState`/`replaceState`](https://developer.mozilla.org/en-US/docs/Web/API/Window/popstate_event) > Note that just calling history.pushState() or history.replaceState() won't trigger a popstate event. The popstate event will be triggered by doing a browser action such as a click on the back or forward button (or calling history.back() or history.forward() in JavaScript). [Reference for `popstate` before `hashChange`](https://developer.mozilla.org/en-US/docs/Web/API/Window/popstate_event#when_popstate_is_sent) > When the transition occurs, either due to the user triggering the browser's > "Back" button or otherwise, the popstate event is near the end of the process to transition to the new location ... > 12. If the value of state changed, the popstate event is sent to the document. > 13. Any persisted user state is restored, if the browser chooses to do so. > 14. If the original and new entry's shared the same document, but had different fragments in their URLs, send the hashchange event to the window. BREAKING CHANGE: The behavior of the `SpyLocation` used by the `RouterTestingModule` has changed to match the behavior of browsers. It no longer emits a 'popstate' event when `Location.go` is called. In addition, `simulateHashChange` now triggers _both_ a `hashchange` and a `popstate` event. Tests which use `location.go` and expect the changes to be picked up by the `Router` should likely change to `simulateHashChange` instead. Each test is different in what it attempts to assert so there is no single change that works for all tests. Each test using the `SpyLocation` to simulate browser URL changes should be evaluated on a case-by-case basis. fixes angular#27059 PR Close angular#41730
1 parent fc3b50e commit c6a9300

File tree

3 files changed

+61
-189
lines changed

3 files changed

+61
-189
lines changed

‎packages/common/testing/src/location_mock.ts‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import {Location, LocationStrategy, PlatformLocation} from '@angular/common';
1010
import {EventEmitter, Injectable} from '@angular/core';
1111
import {SubscriptionLike} from 'rxjs';
1212

13+
import {normalizeQueryParams} from '../../src/location/util';
14+
1315
/**
1416
* A spy for {@link Location} that allows tests to fire simulated location events.
1517
*
@@ -62,9 +64,13 @@ export class SpyLocation implements Location {
6264
}
6365

6466
simulateHashChange(pathname: string) {
65-
// Because we don't prevent the native event, the browser will independently update the path
66-
this.setInitialPath(pathname);
67+
const path = this.prepareExternalUrl(pathname);
68+
this.pushHistory(path, '', null);
69+
6770
this.urlChanges.push('hash: ' + pathname);
71+
// the browser will automatically fire popstate event before each `hashchange` event, so we need
72+
// to simulate it.
73+
this._subject.emit({'url': pathname, 'pop': true, 'type': 'popstate'});
6874
this._subject.emit({'url': pathname, 'pop': true, 'type': 'hashchange'});
6975
}
7076

@@ -78,11 +84,7 @@ export class SpyLocation implements Location {
7884
go(path: string, query: string = '', state: any = null) {
7985
path = this.prepareExternalUrl(path);
8086

81-
if (this._historyIndex > 0) {
82-
this._history.splice(this._historyIndex + 1);
83-
}
84-
this._history.push(new LocationState(path, query, state));
85-
this._historyIndex = this._history.length - 1;
87+
this.pushHistory(path, query, state);
8688

8789
const locationState = this._history[this._historyIndex - 1];
8890
if (locationState.path == path && locationState.query == query) {
@@ -91,7 +93,7 @@ export class SpyLocation implements Location {
9193

9294
const url = path + (query.length > 0 ? ('?' + query) : '');
9395
this.urlChanges.push(url);
94-
this._subject.emit({'url': url, 'pop': false});
96+
this._notifyUrlChangeListeners(path + normalizeQueryParams(query), state);
9597
}
9698

9799
replaceState(path: string, query: string = '', state: any = null) {
@@ -108,19 +110,22 @@ export class SpyLocation implements Location {
108110

109111
const url = path + (query.length > 0 ? ('?' + query) : '');
110112
this.urlChanges.push('replace: ' + url);
113+
this._notifyUrlChangeListeners(path + normalizeQueryParams(query), state);
111114
}
112115

113116
forward() {
114117
if (this._historyIndex < (this._history.length - 1)) {
115118
this._historyIndex++;
116-
this._subject.emit({'url': this.path(), 'state': this.getState(), 'pop': true});
119+
this._subject.emit(
120+
{'url': this.path(), 'state': this.getState(), 'pop': true, 'type': 'popstate'});
117121
}
118122
}
119123

120124
back() {
121125
if (this._historyIndex > 0) {
122126
this._historyIndex--;
123-
this._subject.emit({'url': this.path(), 'state': this.getState(), 'pop': true});
127+
this._subject.emit(
128+
{'url': this.path(), 'state': this.getState(), 'pop': true, 'type': 'popstate'});
124129
}
125130
}
126131

@@ -157,6 +162,14 @@ export class SpyLocation implements Location {
157162
normalize(url: string): string {
158163
return null!;
159164
}
165+
166+
private pushHistory(path: string, query: string, state: any) {
167+
if (this._historyIndex > 0) {
168+
this._history.splice(this._historyIndex + 1);
169+
}
170+
this._history.push(new LocationState(path, query, state));
171+
this._historyIndex = this._history.length - 1;
172+
}
160173
}
161174

162175
class LocationState {

‎packages/router/test/computed_state_restoration.spec.ts‎

Lines changed: 4 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -6,168 +6,18 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {CommonModule, Location, LocationStrategy, PlatformLocation} from '@angular/common';
9+
import {CommonModule, Location} from '@angular/common';
1010
import {SpyLocation} from '@angular/common/testing';
11-
import {Component, EventEmitter, Injectable, NgModule} from '@angular/core';
11+
import {Component, Injectable, NgModule} from '@angular/core';
1212
import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
1313
import {expect} from '@angular/platform-browser/testing/src/matchers';
1414
import {CanActivate, CanDeactivate, Resolve, Router, RouterModule, UrlTree} from '@angular/router';
15-
import {EMPTY, Observable, of, SubscriptionLike} from 'rxjs';
15+
import {EMPTY, Observable, of} from 'rxjs';
1616

1717
import {isUrlTree} from '../src/utils/type_guards';
1818
import {RouterTestingModule} from '../testing';
1919

2020
describe('`restoredState#ɵrouterPageId`', () => {
21-
// TODO: Remove RouterSpyLocation after #38884 is submitted.
22-
class RouterSpyLocation implements Location {
23-
urlChanges: string[] = [];
24-
private _history: LocationState[] = [new LocationState('', '', null)];
25-
private _historyIndex: number = 0;
26-
/** @internal */
27-
_subject: EventEmitter<any> = new EventEmitter();
28-
/** @internal */
29-
_baseHref: string = '';
30-
/** @internal */
31-
_platformStrategy: LocationStrategy = null!;
32-
/** @internal */
33-
_platformLocation: PlatformLocation = null!;
34-
/** @internal */
35-
_urlChangeListeners: ((url: string, state: unknown) => void)[] = [];
36-
/** @internal */
37-
_urlChangeSubscription?: SubscriptionLike;
38-
39-
setInitialPath(url: string) {
40-
this._history[this._historyIndex].path = url;
41-
}
42-
43-
setBaseHref(url: string) {
44-
this._baseHref = url;
45-
}
46-
47-
path(): string {
48-
return this._history[this._historyIndex].path;
49-
}
50-
51-
getState(): unknown {
52-
return this._history[this._historyIndex].state;
53-
}
54-
55-
isCurrentPathEqualTo(path: string, query: string = ''): boolean {
56-
const givenPath = path.endsWith('/') ? path.substring(0, path.length - 1) : path;
57-
const currPath = this.path().endsWith('/') ?
58-
this.path().substring(0, this.path().length - 1) :
59-
this.path();
60-
61-
return currPath == givenPath + (query.length > 0 ? ('?' + query) : '');
62-
}
63-
64-
simulateUrlPop(pathname: string) {
65-
this._subject.emit({'url': pathname, 'pop': true, 'type': 'popstate'});
66-
}
67-
68-
simulateHashChange(pathname: string) {
69-
// Because we don't prevent the native event, the browser will independently update the path
70-
this.setInitialPath(pathname);
71-
this.urlChanges.push('hash: ' + pathname);
72-
this._subject.emit({'url': pathname, 'pop': true, 'type': 'hashchange'});
73-
}
74-
75-
prepareExternalUrl(url: string): string {
76-
if (url.length > 0 && !url.startsWith('/')) {
77-
url = '/' + url;
78-
}
79-
return this._baseHref + url;
80-
}
81-
82-
go(path: string, query: string = '', state: any = null) {
83-
path = this.prepareExternalUrl(path);
84-
85-
if (this._historyIndex > 0) {
86-
this._history.splice(this._historyIndex + 1);
87-
}
88-
this._history.push(new LocationState(path, query, state));
89-
this._historyIndex = this._history.length - 1;
90-
91-
const locationState = this._history[this._historyIndex - 1];
92-
if (locationState.path == path && locationState.query == query) {
93-
return;
94-
}
95-
96-
const url = path + (query.length > 0 ? ('?' + query) : '');
97-
this.urlChanges.push(url);
98-
}
99-
100-
replaceState(path: string, query: string = '', state: any = null) {
101-
path = this.prepareExternalUrl(path);
102-
103-
const history = this._history[this._historyIndex];
104-
if (history.path == path && history.query == query) {
105-
return;
106-
}
107-
108-
history.path = path;
109-
history.query = query;
110-
history.state = state;
111-
112-
const url = path + (query.length > 0 ? ('?' + query) : '');
113-
this.urlChanges.push('replace: ' + url);
114-
}
115-
116-
forward() {
117-
if (this._historyIndex < (this._history.length - 1)) {
118-
this._historyIndex++;
119-
this._subject.emit(
120-
{'url': this.path(), 'state': this.getState(), 'pop': true, 'type': 'popstate'});
121-
}
122-
}
123-
124-
back() {
125-
if (this._historyIndex > 0) {
126-
this._historyIndex--;
127-
this._subject.emit(
128-
{'url': this.path(), 'state': this.getState(), 'pop': true, 'type': 'popstate'});
129-
}
130-
}
131-
132-
historyGo(relativePosition: number = 0): void {
133-
const nextPageIndex = this._historyIndex + relativePosition;
134-
if (nextPageIndex >= 0 && nextPageIndex < this._history.length) {
135-
this._historyIndex = nextPageIndex;
136-
this._subject.emit(
137-
{'url': this.path(), 'state': this.getState(), 'pop': true, 'type': 'popstate'});
138-
}
139-
}
140-
141-
onUrlChange(fn: (url: string, state: unknown) => void) {
142-
this._urlChangeListeners.push(fn);
143-
144-
if (!this._urlChangeSubscription) {
145-
this._urlChangeSubscription = this.subscribe(v => {
146-
this._notifyUrlChangeListeners(v.url, v.state);
147-
});
148-
}
149-
}
150-
151-
/** @internal */
152-
_notifyUrlChangeListeners(url: string = '', state: unknown) {
153-
this._urlChangeListeners.forEach(fn => fn(url, state));
154-
}
155-
156-
subscribe(
157-
onNext: (value: any) => void, onThrow?: ((error: any) => void)|null,
158-
onReturn?: (() => void)|null): SubscriptionLike {
159-
return this._subject.subscribe({next: onNext, error: onThrow, complete: onReturn});
160-
}
161-
162-
normalize(url: string): string {
163-
return null!;
164-
}
165-
}
166-
167-
class LocationState {
168-
constructor(public path: string, public query: string, public state: any) {}
169-
}
170-
17121
@Injectable({providedIn: 'root'})
17222
class MyCanDeactivateGuard implements CanDeactivate<any> {
17323
allow: boolean = true;
@@ -227,7 +77,7 @@ describe('`restoredState#ɵrouterPageId`', () => {
22777
imports: [TestModule],
22878
providers: [
22979
{provide: 'alwaysFalse', useValue: (a: any) => false},
230-
{provide: Location, useClass: RouterSpyLocation}
80+
{provide: Location, useClass: SpyLocation}
23181
]
23282
});
23383
const router = TestBed.inject(Router);

0 commit comments

Comments
 (0)