fix(common): cleanup URL change listeners when the root view is removed (#44901)

The `Location` creates the `_urlChangeSubscription` when the `onUrlChange` is called for the first time.
The subscription `next` function captures `this` and prevents the `Location` from being garbage collected
when the root view is removed.

PR Close #44901
This commit is contained in:
arturovt 2022-01-31 00:40:10 +02:00 committed by Alex Rickabaugh
parent f17e26f7c1
commit bedb257afc
5 changed files with 92 additions and 10 deletions

View file

@ -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;

View file

@ -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)

View file

@ -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<any> = 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 */

View file

@ -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 */

View file

@ -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: [