diff --git a/goldens/public-api/common/common.md b/goldens/public-api/common/common.md index 3b7c57a281e..19d4c52758c 100644 --- a/goldens/public-api/common/common.md +++ b/goldens/public-api/common/common.md @@ -308,7 +308,7 @@ export class KeyValuePipe implements PipeTransform { } // @public -class Location_2 { +class Location_2 implements OnDestroy { constructor(platformStrategy: LocationStrategy, platformLocation: PlatformLocation); back(): void; forward(): void; @@ -317,9 +317,11 @@ class Location_2 { historyGo(relativePosition?: number): void; isCurrentPathEqualTo(path: string, query?: string): boolean; static joinWithSlash: (start: string, end: string) => string; + // (undocumented) + ngOnDestroy(): void; normalize(url: string): string; static normalizeQueryParams: (params: string) => string; - onUrlChange(fn: (url: string, state: unknown) => void): void; + onUrlChange(fn: (url: string, state: unknown) => void): VoidFunction; path(includeHash?: boolean): string; prepareExternalUrl(url: string): string; replaceState(path: string, query?: string, state?: any): void; diff --git a/goldens/public-api/common/testing/testing.md b/goldens/public-api/common/testing/testing.md index b98a5e1701c..d28b832f9b0 100644 --- a/goldens/public-api/common/testing/testing.md +++ b/goldens/public-api/common/testing/testing.md @@ -120,9 +120,11 @@ export class SpyLocation implements Location_2 { // (undocumented) isCurrentPathEqualTo(path: string, query?: string): boolean; // (undocumented) + ngOnDestroy(): void; + // (undocumented) normalize(url: string): string; // (undocumented) - onUrlChange(fn: (url: string, state: unknown) => void): void; + onUrlChange(fn: (url: string, state: unknown) => void): VoidFunction; // (undocumented) path(): string; // (undocumented) diff --git a/packages/common/src/location/location.ts b/packages/common/src/location/location.ts index 4df0e7f420e..1c765627314 100644 --- a/packages/common/src/location/location.ts +++ b/packages/common/src/location/location.ts @@ -6,8 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ -import {EventEmitter, Injectable, ɵɵinject} from '@angular/core'; +import {EventEmitter, Injectable, OnDestroy, ɵɵinject} from '@angular/core'; import {SubscriptionLike} from 'rxjs'; + import {LocationStrategy} from './location_strategy'; import {PlatformLocation} from './platform_location'; import {joinWithSlash, normalizeQueryParams, stripTrailingSlash} from './util'; @@ -53,7 +54,7 @@ export interface PopStateEvent { // See #23917 useFactory: createLocation, }) -export class Location { +export class Location implements OnDestroy { /** @internal */ _subject: EventEmitter = new EventEmitter(); /** @internal */ @@ -65,7 +66,7 @@ export class Location { /** @internal */ _urlChangeListeners: ((url: string, state: unknown) => void)[] = []; /** @internal */ - _urlChangeSubscription?: SubscriptionLike; + _urlChangeSubscription: SubscriptionLike|null = null; constructor(platformStrategy: LocationStrategy, platformLocation: PlatformLocation) { this._platformStrategy = platformStrategy; @@ -82,6 +83,12 @@ export class Location { }); } + /** @nodoc */ + ngOnDestroy(): void { + this._urlChangeSubscription?.unsubscribe(); + this._urlChangeListeners = []; + } + /** * Normalizes the URL path for this location. * @@ -209,8 +216,9 @@ export class Location { * framework that are not detectible through "popstate" or "hashchange" events. * * @param fn The change handler function, which take a URL and a location history state. + * @returns A function that, when executed, unregisters a URL change listener. */ - onUrlChange(fn: (url: string, state: unknown) => void) { + onUrlChange(fn: (url: string, state: unknown) => void): VoidFunction { this._urlChangeListeners.push(fn); if (!this._urlChangeSubscription) { @@ -218,6 +226,16 @@ export class Location { this._notifyUrlChangeListeners(v.url, v.state); }); } + + return () => { + const fnIndex = this._urlChangeListeners.indexOf(fn); + this._urlChangeListeners.splice(fnIndex, 1); + + if (this._urlChangeListeners.length === 0) { + this._urlChangeSubscription?.unsubscribe(); + this._urlChangeSubscription = null; + } + }; } /** @internal */ diff --git a/packages/common/testing/src/location_mock.ts b/packages/common/testing/src/location_mock.ts index 0c6a8e6f8f8..e53025b9a54 100644 --- a/packages/common/testing/src/location_mock.ts +++ b/packages/common/testing/src/location_mock.ts @@ -33,7 +33,12 @@ export class SpyLocation implements Location { /** @internal */ _urlChangeListeners: ((url: string, state: unknown) => void)[] = []; /** @internal */ - _urlChangeSubscription?: SubscriptionLike; + _urlChangeSubscription: SubscriptionLike|null = null; + + ngOnDestroy(): void { + this._urlChangeSubscription?.unsubscribe(); + this._urlChangeListeners = []; + } setInitialPath(url: string) { this._history[this._historyIndex].path = url; @@ -138,7 +143,7 @@ export class SpyLocation implements Location { } } - onUrlChange(fn: (url: string, state: unknown) => void) { + onUrlChange(fn: (url: string, state: unknown) => void): VoidFunction { this._urlChangeListeners.push(fn); if (!this._urlChangeSubscription) { @@ -146,6 +151,16 @@ export class SpyLocation implements Location { this._notifyUrlChangeListeners(v.url, v.state); }); } + + return () => { + const fnIndex = this._urlChangeListeners.indexOf(fn); + this._urlChangeListeners.splice(fnIndex, 1); + + if (this._urlChangeListeners.length === 0) { + this._urlChangeSubscription?.unsubscribe(); + this._urlChangeSubscription = null; + } + }; } /** @internal */ diff --git a/packages/router/test/bootstrap.spec.ts b/packages/router/test/bootstrap.spec.ts index ff425ca1410..f9699a5c02e 100644 --- a/packages/router/test/bootstrap.spec.ts +++ b/packages/router/test/bootstrap.spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {APP_BASE_HREF, DOCUMENT, ɵgetDOM as getDOM} from '@angular/common'; +import {APP_BASE_HREF, DOCUMENT, Location, ɵgetDOM as getDOM} from '@angular/common'; import {ApplicationRef, Component, CUSTOM_ELEMENTS_SCHEMA, destroyPlatform, NgModule} from '@angular/core'; import {inject} from '@angular/core/testing'; import {BrowserModule} from '@angular/platform-browser'; @@ -407,6 +407,51 @@ describe('bootstrap', () => { expect(window.removeEventListener).toHaveBeenCalledWith('hashchange', jasmine.any(Function)); }); + it('should unregister a URL change listener and unsubscribe from URL changes when the root view is removed', + async () => { + const changeListener = jasmine.createSpy('changeListener'); + + @Component({template: 'second simple'}) + class SecondSimpleCmp { + } + + @NgModule({ + imports: [ + BrowserModule, + RouterModule.forRoot( + [{path: 'a', component: SimpleCmp}, {path: 'b', component: SecondSimpleCmp}], + {initialNavigation: 'enabled'}) + ], + declarations: [RootCmp, SimpleCmp, SecondSimpleCmp], + bootstrap: [RootCmp], + providers: testProviders + }) + class TestModule { + } + + const ngModuleRef = await platformBrowserDynamic().bootstrapModule(TestModule); + const router = ngModuleRef.injector.get(Router); + const location = ngModuleRef.injector.get(Location); + + const removeUrlChangeFn = location.onUrlChange(changeListener); + + await router.navigateByUrl('/a'); + expect(changeListener).toHaveBeenCalledTimes(1); + + removeUrlChangeFn(); + await router.navigateByUrl('/b'); + expect(changeListener).toHaveBeenCalledTimes(1); + + location.onUrlChange((url: string, state: unknown) => {}); + + ngModuleRef.destroy(); + + // Let's ensure that URL change listeners are unregistered when the root view is removed, + // tho the last returned `onUrlChange` function hasn't been invoked. + expect((location as any)._urlChangeListeners.length).toEqual(0); + expect((location as any)._urlChangeSubscription.closed).toEqual(true); + }); + it('can schedule a navigation from the NavigationEnd event #37460', async (done) => { @NgModule({ imports: [