diff --git a/goldens/public-api/common/common.d.ts b/goldens/public-api/common/common.d.ts index 63da20ca44..3226b869f0 100644 --- a/goldens/public-api/common/common.d.ts +++ b/goldens/public-api/common/common.d.ts @@ -96,11 +96,12 @@ export declare function getLocaleWeekEndRange(locale: string): [WeekDay, WeekDay export declare function getNumberOfCurrencyDigits(code: string): number; -export declare class HashLocationStrategy extends LocationStrategy { +export declare class HashLocationStrategy extends LocationStrategy implements OnDestroy { constructor(_platformLocation: PlatformLocation, _baseHref?: string); back(): void; forward(): void; getBaseHref(): string; + ngOnDestroy(): void; onPopState(fn: LocationChangeListener): void; path(includeHash?: boolean): string; prepareExternalUrl(internal: string): string; @@ -324,11 +325,12 @@ export declare enum NumberSymbol { CurrencyGroup = 13 } -export declare class PathLocationStrategy extends LocationStrategy { +export declare class PathLocationStrategy extends LocationStrategy implements OnDestroy { constructor(_platformLocation: PlatformLocation, href?: string); back(): void; forward(): void; getBaseHref(): string; + ngOnDestroy(): void; onPopState(fn: LocationChangeListener): void; path(includeHash?: boolean): string; prepareExternalUrl(internal: string): string; @@ -355,8 +357,8 @@ export declare abstract class PlatformLocation { abstract forward(): void; abstract getBaseHrefFromDOM(): string; abstract getState(): unknown; - abstract onHashChange(fn: LocationChangeListener): void; - abstract onPopState(fn: LocationChangeListener): void; + abstract onHashChange(fn: LocationChangeListener): VoidFunction; + abstract onPopState(fn: LocationChangeListener): VoidFunction; abstract pushState(state: any, title: string, url: string): void; abstract replaceState(state: any, title: string, url: string): void; } diff --git a/goldens/public-api/common/testing/testing.d.ts b/goldens/public-api/common/testing/testing.d.ts index cda45d1a1e..b4b4b8fca8 100644 --- a/goldens/public-api/common/testing/testing.d.ts +++ b/goldens/public-api/common/testing/testing.d.ts @@ -33,8 +33,8 @@ export declare class MockPlatformLocation implements PlatformLocation { forward(): void; getBaseHrefFromDOM(): string; getState(): unknown; - onHashChange(fn: LocationChangeListener): void; - onPopState(fn: LocationChangeListener): void; + onHashChange(fn: LocationChangeListener): VoidFunction; + onPopState(fn: LocationChangeListener): VoidFunction; pushState(state: any, title: string, newUrl: string): void; replaceState(state: any, title: string, newUrl: string): void; } diff --git a/goldens/size-tracking/aio-payloads.json b/goldens/size-tracking/aio-payloads.json index f785126125..fef0112d62 100755 --- a/goldens/size-tracking/aio-payloads.json +++ b/goldens/size-tracking/aio-payloads.json @@ -26,4 +26,4 @@ } } } -} +} \ No newline at end of file diff --git a/packages/common/src/location/hash_location_strategy.ts b/packages/common/src/location/hash_location_strategy.ts index 5f06a98bd5..b46ff1ccfa 100644 --- a/packages/common/src/location/hash_location_strategy.ts +++ b/packages/common/src/location/hash_location_strategy.ts @@ -6,7 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -import {Inject, Injectable, Optional} from '@angular/core'; +import {Inject, Injectable, OnDestroy, Optional} from '@angular/core'; + import {APP_BASE_HREF, LocationStrategy} from './location_strategy'; import {LocationChangeListener, PlatformLocation} from './platform_location'; import {joinWithSlash, normalizeQueryParams} from './util'; @@ -32,8 +33,10 @@ import {joinWithSlash, normalizeQueryParams} from './util'; * @publicApi */ @Injectable() -export class HashLocationStrategy extends LocationStrategy { +export class HashLocationStrategy extends LocationStrategy implements OnDestroy { private _baseHref: string = ''; + private _removeListenerFns: (() => void)[] = []; + constructor( private _platformLocation: PlatformLocation, @Optional() @Inject(APP_BASE_HREF) _baseHref?: string) { @@ -43,9 +46,15 @@ export class HashLocationStrategy extends LocationStrategy { } } + ngOnDestroy(): void { + while (this._removeListenerFns.length) { + this._removeListenerFns.pop()!(); + } + } + onPopState(fn: LocationChangeListener): void { - this._platformLocation.onPopState(fn); - this._platformLocation.onHashChange(fn); + this._removeListenerFns.push( + this._platformLocation.onPopState(fn), this._platformLocation.onHashChange(fn)); } getBaseHref(): string { diff --git a/packages/common/src/location/location_strategy.ts b/packages/common/src/location/location_strategy.ts index 3d9bc0e05f..731f5c6091 100644 --- a/packages/common/src/location/location_strategy.ts +++ b/packages/common/src/location/location_strategy.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Inject, Injectable, InjectionToken, Optional, ɵɵinject} from '@angular/core'; +import {Inject, Injectable, InjectionToken, OnDestroy, Optional, ɵɵinject} from '@angular/core'; import {DOCUMENT} from '../dom_tokens'; import {LocationChangeListener, PlatformLocation} from './platform_location'; import {joinWithSlash, normalizeQueryParams} from './util'; @@ -105,8 +105,9 @@ export const APP_BASE_HREF = new InjectionToken('appBaseHref'); * @publicApi */ @Injectable() -export class PathLocationStrategy extends LocationStrategy { +export class PathLocationStrategy extends LocationStrategy implements OnDestroy { private _baseHref: string; + private _removeListenerFns: (() => void)[] = []; constructor( private _platformLocation: PlatformLocation, @@ -125,9 +126,15 @@ export class PathLocationStrategy extends LocationStrategy { this._baseHref = href; } + ngOnDestroy(): void { + while (this._removeListenerFns.length) { + this._removeListenerFns.pop()!(); + } + } + onPopState(fn: LocationChangeListener): void { - this._platformLocation.onPopState(fn); - this._platformLocation.onHashChange(fn); + this._removeListenerFns.push( + this._platformLocation.onPopState(fn), this._platformLocation.onHashChange(fn)); } getBaseHref(): string { diff --git a/packages/common/src/location/platform_location.ts b/packages/common/src/location/platform_location.ts index 539bbb5a7a..06c1b81cf9 100644 --- a/packages/common/src/location/platform_location.ts +++ b/packages/common/src/location/platform_location.ts @@ -40,8 +40,14 @@ import {DOCUMENT} from '../dom_tokens'; export abstract class PlatformLocation { abstract getBaseHrefFromDOM(): string; abstract getState(): unknown; - abstract onPopState(fn: LocationChangeListener): void; - abstract onHashChange(fn: LocationChangeListener): void; + /** + * Returns a function that, when executed, removes the `popstate` event handler. + */ + abstract onPopState(fn: LocationChangeListener): VoidFunction; + /** + * Returns a function that, when executed, removes the `hashchange` event handler. + */ + abstract onHashChange(fn: LocationChangeListener): VoidFunction; abstract get href(): string; abstract get protocol(): string; @@ -122,12 +128,16 @@ export class BrowserPlatformLocation extends PlatformLocation { return getDOM().getBaseHref(this._doc)!; } - onPopState(fn: LocationChangeListener): void { - getDOM().getGlobalEventTarget(this._doc, 'window').addEventListener('popstate', fn, false); + onPopState(fn: LocationChangeListener): VoidFunction { + const window = getDOM().getGlobalEventTarget(this._doc, 'window'); + window.addEventListener('popstate', fn, false); + return () => window.removeEventListener('popstate', fn); } - onHashChange(fn: LocationChangeListener): void { - getDOM().getGlobalEventTarget(this._doc, 'window').addEventListener('hashchange', fn, false); + onHashChange(fn: LocationChangeListener): VoidFunction { + const window = getDOM().getGlobalEventTarget(this._doc, 'window'); + window.addEventListener('hashchange', fn, false); + return () => window.removeEventListener('hashchange', fn); } get href(): string { diff --git a/packages/common/testing/src/mock_platform_location.ts b/packages/common/testing/src/mock_platform_location.ts index b3c97ddf69..e4f3f33607 100644 --- a/packages/common/testing/src/mock_platform_location.ts +++ b/packages/common/testing/src/mock_platform_location.ts @@ -153,13 +153,15 @@ export class MockPlatformLocation implements PlatformLocation { return this.baseHref; } - onPopState(fn: LocationChangeListener): void { + onPopState(fn: LocationChangeListener): VoidFunction { // No-op: a state stack is not implemented, so // no events will ever come. + return () => {}; } - onHashChange(fn: LocationChangeListener): void { - this.hashUpdate.subscribe(fn); + onHashChange(fn: LocationChangeListener): VoidFunction { + const subscription = this.hashUpdate.subscribe(fn); + return () => subscription.unsubscribe(); } get href(): string { diff --git a/packages/platform-server/src/location.ts b/packages/platform-server/src/location.ts index 054e4f85ba..fe404c2412 100644 --- a/packages/platform-server/src/location.ts +++ b/packages/platform-server/src/location.ts @@ -70,13 +70,15 @@ export class ServerPlatformLocation implements PlatformLocation { return getDOM().getBaseHref(this._doc)!; } - onPopState(fn: LocationChangeListener): void { + onPopState(fn: LocationChangeListener): VoidFunction { // No-op: a state stack is not implemented, so // no events will ever come. + return () => {}; } - onHashChange(fn: LocationChangeListener): void { - this._hashUpdate.subscribe(fn); + onHashChange(fn: LocationChangeListener): VoidFunction { + const subscription = this._hashUpdate.subscribe(fn); + return () => subscription.unsubscribe(); } get url(): string { diff --git a/packages/router/test/bootstrap.spec.ts b/packages/router/test/bootstrap.spec.ts index 5c76c6b8e2..e3b9a63915 100644 --- a/packages/router/test/bootstrap.spec.ts +++ b/packages/router/test/bootstrap.spec.ts @@ -6,13 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ -import {APP_BASE_HREF, DOCUMENT, Location, ɵgetDOM as getDOM} from '@angular/common'; +import {APP_BASE_HREF, DOCUMENT, ɵ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'; import {platformBrowserDynamic} from '@angular/platform-browser-dynamic'; import {NavigationEnd, Resolve, Router, RouterModule} from '@angular/router'; -import {filter, first} from 'rxjs/operators'; describe('bootstrap', () => { if (isNode) return; @@ -369,7 +368,30 @@ describe('bootstrap', () => { done(); }); - function waitForNavigationToComplete(router: Router): Promise { - return router.events.pipe(filter((e: any) => e instanceof NavigationEnd), first()).toPromise(); - } + it('should cleanup "popstate" and "hashchange" listeners', async () => { + @NgModule({ + imports: [BrowserModule, RouterModule.forRoot([])], + declarations: [RootCmp], + bootstrap: [RootCmp], + providers: testProviders, + }) + class TestModule { + } + + spyOn(window, 'addEventListener').and.callThrough(); + spyOn(window, 'removeEventListener').and.callThrough(); + + const ngModuleRef = await platformBrowserDynamic().bootstrapModule(TestModule); + ngModuleRef.destroy(); + + expect(window.addEventListener).toHaveBeenCalledTimes(2); + + expect(window.addEventListener) + .toHaveBeenCalledWith('popstate', jasmine.any(Function), jasmine.any(Boolean)); + expect(window.addEventListener) + .toHaveBeenCalledWith('hashchange', jasmine.any(Function), jasmine.any(Boolean)); + + expect(window.removeEventListener).toHaveBeenCalledWith('popstate', jasmine.any(Function)); + expect(window.removeEventListener).toHaveBeenCalledWith('hashchange', jasmine.any(Function)); + }); });