From 3569fdf45195c36713e43a91a4feba4fa3c9facf Mon Sep 17 00:00:00 2001
From: Lars Gyrup Brink Nielsen <larsbrinknielsen@gmail.com>
Date: Tue, 2 Jun 2020 23:05:39 +0200
Subject: [PATCH] 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
---
 packages/common/src/location/location.ts      | 10 +++++--
 .../common/test/location/location_spec.ts     | 29 +++++++++++++++++--
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/packages/common/src/location/location.ts b/packages/common/src/location/location.ts
index e5151afd66..fbf0675cc8 100644
--- a/packages/common/src/location/location.ts
+++ b/packages/common/src/location/location.ts
@@ -64,6 +64,7 @@ export class Location {
   _platformLocation: PlatformLocation;
   /** @internal */
   _urlChangeListeners: ((url: string, state: unknown) => void)[] = [];
+  private _urlChangeSubscription?: SubscriptionLike;
 
   constructor(platformStrategy: LocationStrategy, platformLocation: PlatformLocation) {
     this._platformStrategy = platformStrategy;
@@ -194,9 +195,12 @@ export class Location {
    */
   onUrlChange(fn: (url: string, state: unknown) => void) {
     this._urlChangeListeners.push(fn);
-    this.subscribe(v => {
-      this._notifyUrlChangeListeners(v.url, v.state);
-    });
+
+    if (!this._urlChangeSubscription) {
+      this._urlChangeSubscription = this.subscribe(v => {
+        this._notifyUrlChangeListeners(v.url, v.state);
+      });
+    }
   }
 
   /** @internal */
diff --git a/packages/common/test/location/location_spec.ts b/packages/common/test/location/location_spec.ts
index bcc9e6f673..b4da98a2f0 100644
--- a/packages/common/test/location/location_spec.ts
+++ b/packages/common/test/location/location_spec.ts
@@ -7,7 +7,7 @@
  */
 
 import {CommonModule, Location, LocationStrategy, PathLocationStrategy, PlatformLocation} from '@angular/common';
-import {MockPlatformLocation} from '@angular/common/testing';
+import {MockLocationStrategy, MockPlatformLocation} from '@angular/common/testing';
 import {inject, TestBed} from '@angular/core/testing';
 
 const baseUrl = '/base';
@@ -84,7 +84,7 @@ describe('Location Class', () => {
       TestBed.configureTestingModule({
         imports: [CommonModule],
         providers: [
-          {provide: LocationStrategy, useClass: PathLocationStrategy},
+          {provide: LocationStrategy, useClass: MockLocationStrategy},
           {
             provide: PlatformLocation,
             useFactory: () => {
@@ -113,5 +113,30 @@ describe('Location Class', () => {
          expect((location as any)._urlChangeListeners.length).toBe(1);
          expect((location as any)._urlChangeListeners[0]).toEqual(changeListener);
        }));
+
+    it('should only notify listeners once when multiple listeners are registered', () => {
+      const location = TestBed.inject(Location);
+      const locationStrategy = TestBed.inject(LocationStrategy) as MockLocationStrategy;
+      let notificationCount = 0;
+
+      function incrementChangeListener(url: string, state: unknown) {
+        notificationCount += 1;
+
+        return undefined;
+      }
+
+      function noopChangeListener(url: string, state: unknown) {
+        return undefined;
+      }
+
+      location.onUrlChange(incrementChangeListener);
+      location.onUrlChange(noopChangeListener);
+
+      expect(notificationCount).toBe(0);
+
+      locationStrategy.simulatePopState('/test');
+
+      expect(notificationCount).toBe(1);
+    });
   });
 });