Skip to content

Commit fff424a

Browse files
LayZeeDKatscott
authored andcommitted
fix(common): prevent duplicate URL change notifications (#37404)
Prevent duplicate notifications from being emitted when multiple URL change listeners are registered using Location#onUrlChange. PR Close #37404
1 parent b5d1c8b commit fff424a

File tree

2 files changed

+34
-5
lines changed

2 files changed

+34
-5
lines changed

packages/common/src/location/location.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export class Location {
6464
_platformLocation: PlatformLocation;
6565
/** @internal */
6666
_urlChangeListeners: ((url: string, state: unknown) => void)[] = [];
67+
private _urlChangeSubscription?: SubscriptionLike;
6768

6869
constructor(platformStrategy: LocationStrategy, platformLocation: PlatformLocation) {
6970
this._platformStrategy = platformStrategy;
@@ -194,9 +195,12 @@ export class Location {
194195
*/
195196
onUrlChange(fn: (url: string, state: unknown) => void) {
196197
this._urlChangeListeners.push(fn);
197-
this.subscribe(v => {
198-
this._notifyUrlChangeListeners(v.url, v.state);
199-
});
198+
199+
if (!this._urlChangeSubscription) {
200+
this._urlChangeSubscription = this.subscribe(v => {
201+
this._notifyUrlChangeListeners(v.url, v.state);
202+
});
203+
}
200204
}
201205

202206
/** @internal */

packages/common/test/location/location_spec.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {CommonModule, Location, LocationStrategy, PathLocationStrategy, PlatformLocation} from '@angular/common';
10-
import {MockPlatformLocation} from '@angular/common/testing';
10+
import {MockLocationStrategy, MockPlatformLocation} from '@angular/common/testing';
1111
import {inject, TestBed} from '@angular/core/testing';
1212

1313
const baseUrl = '/base';
@@ -84,7 +84,7 @@ describe('Location Class', () => {
8484
TestBed.configureTestingModule({
8585
imports: [CommonModule],
8686
providers: [
87-
{provide: LocationStrategy, useClass: PathLocationStrategy},
87+
{provide: LocationStrategy, useClass: MockLocationStrategy},
8888
{
8989
provide: PlatformLocation,
9090
useFactory: () => {
@@ -113,5 +113,30 @@ describe('Location Class', () => {
113113
expect((location as any)._urlChangeListeners.length).toBe(1);
114114
expect((location as any)._urlChangeListeners[0]).toEqual(changeListener);
115115
}));
116+
117+
it('should only notify listeners once when multiple listeners are registered', () => {
118+
const location = TestBed.inject(Location);
119+
const locationStrategy = TestBed.inject(LocationStrategy) as MockLocationStrategy;
120+
let notificationCount = 0;
121+
122+
function incrementChangeListener(url: string, state: unknown) {
123+
notificationCount += 1;
124+
125+
return undefined;
126+
}
127+
128+
function noopChangeListener(url: string, state: unknown) {
129+
return undefined;
130+
}
131+
132+
location.onUrlChange(incrementChangeListener);
133+
location.onUrlChange(noopChangeListener);
134+
135+
expect(notificationCount).toBe(0);
136+
137+
locationStrategy.simulatePopState('/test');
138+
139+
expect(notificationCount).toBe(1);
140+
});
116141
});
117142
});

0 commit comments

Comments
 (0)