Skip to content

Commit

Permalink
fix(core): trigger ApplicationRef.destroy when Platform is destroyed (
Browse files Browse the repository at this point in the history
#46497)

This commit updates the `ApplicationRef` logic to trigger the destroy operation when an underlying platform is destroyed. This is needed to make sure all teardown processing is completed correctly to avoid memory leaks.

Closes #46473.

PR Close #46497
  • Loading branch information
AndrewKushnir authored and dylhunn committed Jun 28, 2022
1 parent 53ba3d7 commit 1e7f22f
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 67 deletions.
25 changes: 20 additions & 5 deletions packages/core/src/application_ref.ts
Expand Up @@ -60,7 +60,8 @@ export const ALLOW_MULTIPLE_PLATFORMS = new InjectionToken<boolean>('AllowMultip
* `PlatformRef` class (i.e. register the callback via `PlatformRef.onDestroy`), thus making the
* entire class tree-shakeable.
*/
const PLATFORM_ON_DESTROY = new InjectionToken<() => void>('PlatformOnDestroy');
const PLATFORM_DESTROY_LISTENERS =
new InjectionToken<Set<VoidFunction>>('PlatformDestroyListeners');

const NG_DEV_MODE = typeof ngDevMode === 'undefined' || ngDevMode;

Expand Down Expand Up @@ -230,7 +231,18 @@ export function internalBootstrapApplication(config: {
setLocaleId(localeId || DEFAULT_LOCALE_ID);

const appRef = appInjector.get(ApplicationRef);
appRef.onDestroy(() => onErrorSubscription.unsubscribe());

// If the whole platform is destroyed, invoke the `destroy` method
// for all bootstrapped applications as well.
const destroyListener = () => appRef.destroy();
const onPlatformDestroyListeners = platformInjector.get(PLATFORM_DESTROY_LISTENERS, null);
onPlatformDestroyListeners?.add(destroyListener);

appRef.onDestroy(() => {
onPlatformDestroyListeners?.delete(destroyListener);
onErrorSubscription.unsubscribe();
});

appRef.bootstrap(rootComponent);
return appRef;
});
Expand Down Expand Up @@ -303,7 +315,7 @@ export function createPlatformInjector(providers: StaticProvider[] = [], name?:
name,
providers: [
{provide: INJECTOR_SCOPE, useValue: 'platform'},
{provide: PLATFORM_ON_DESTROY, useValue: () => _platformInjector = null}, //
{provide: PLATFORM_DESTROY_LISTENERS, useValue: new Set([() => _platformInjector = null])},
...providers
],
});
Expand Down Expand Up @@ -523,8 +535,11 @@ export class PlatformRef {
this._modules.slice().forEach(module => module.destroy());
this._destroyListeners.forEach(listener => listener());

const destroyListener = this._injector.get(PLATFORM_ON_DESTROY, null);
destroyListener?.();
const destroyListeners = this._injector.get(PLATFORM_DESTROY_LISTENERS, null);
if (destroyListeners) {
destroyListeners.forEach(listener => listener());
destroyListeners.clear();
}

this._destroyed = true;
}
Expand Down
Expand Up @@ -354,13 +354,13 @@
"name": "PARAM_REGEX"
},
{
"name": "PLATFORM_ID"
"name": "PLATFORM_DESTROY_LISTENERS"
},
{
"name": "PLATFORM_INITIALIZER"
"name": "PLATFORM_ID"
},
{
"name": "PLATFORM_ON_DESTROY"
"name": "PLATFORM_INITIALIZER"
},
{
"name": "PlatformRef"
Expand Down
Expand Up @@ -357,13 +357,13 @@
"name": "Optional"
},
{
"name": "PLATFORM_ID"
"name": "PLATFORM_DESTROY_LISTENERS"
},
{
"name": "PLATFORM_INITIALIZER"
"name": "PLATFORM_ID"
},
{
"name": "PLATFORM_ON_DESTROY"
"name": "PLATFORM_INITIALIZER"
},
{
"name": "PlatformRef"
Expand Down
Expand Up @@ -351,13 +351,13 @@
"name": "Optional"
},
{
"name": "PLATFORM_ID"
"name": "PLATFORM_DESTROY_LISTENERS"
},
{
"name": "PLATFORM_INITIALIZER"
"name": "PLATFORM_ID"
},
{
"name": "PLATFORM_ON_DESTROY"
"name": "PLATFORM_INITIALIZER"
},
{
"name": "PlatformRef"
Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Expand Up @@ -432,13 +432,13 @@
"name": "OutletInjector"
},
{
"name": "PLATFORM_ID"
"name": "PLATFORM_DESTROY_LISTENERS"
},
{
"name": "PLATFORM_INITIALIZER"
"name": "PLATFORM_ID"
},
{
"name": "PLATFORM_ON_DESTROY"
"name": "PLATFORM_INITIALIZER"
},
{
"name": "PathLocationStrategy"
Expand Down
Expand Up @@ -222,13 +222,13 @@
"name": "Observable"
},
{
"name": "PLATFORM_ID"
"name": "PLATFORM_DESTROY_LISTENERS"
},
{
"name": "PLATFORM_INITIALIZER"
"name": "PLATFORM_ID"
},
{
"name": "PLATFORM_ON_DESTROY"
"name": "PLATFORM_INITIALIZER"
},
{
"name": "R3Injector"
Expand Down
135 changes: 88 additions & 47 deletions packages/platform-browser/test/browser/bootstrap_standalone_spec.ts
Expand Up @@ -6,63 +6,60 @@
* found in the LICENSE file at https://angular.io/license
*/

import {DOCUMENT, ɵgetDOM as getDOM} from '@angular/common';
import {Component, destroyPlatform, ErrorHandler, Inject, Injectable, InjectionToken, NgModule} from '@angular/core';
import {inject} from '@angular/core/testing';
import {Component, ErrorHandler, Inject, Injectable, InjectionToken, NgModule, PlatformRef} from '@angular/core';
import {R3Injector} from '@angular/core/src/di/r3_injector';
import {withBody} from '@angular/private/testing';

import {bootstrapApplication, BrowserModule} from '../../src/browser';

describe('bootstrapApplication for standalone components', () => {
let rootEl: HTMLUnknownElement;
beforeEach(inject([DOCUMENT], (doc: any) => {
rootEl = getDOM().createElement('test-app', doc);
getDOM().getDefaultDocument().body.appendChild(rootEl);
}));

afterEach(() => {
destroyPlatform();
rootEl?.remove();
});

class SilentErrorHandler extends ErrorHandler {
override handleError() {
// the error is already re-thrown by the application ref.
// we don't want to print it, but instead catch it in tests.
}
}

it('should create injector where ambient providers shadow explicit providers', async () => {
const testToken = new InjectionToken('test token');
it('should create injector where ambient providers shadow explicit providers',
withBody('<test-app></test-app>', async () => {
const testToken = new InjectionToken('test token');

@NgModule({
providers: [
{provide: testToken, useValue: 'Ambient'},
]
})
class AmbientModule {
}
@NgModule({
providers: [
{provide: testToken, useValue: 'Ambient'},
]
})
class AmbientModule {
}

@Component({
selector: 'test-app',
standalone: true,
template: `({{testToken}})`,
imports: [AmbientModule]
})
class StandaloneCmp {
constructor(@Inject(testToken) readonly testToken: String) {}
}
@Component({
selector: 'test-app',
standalone: true,
template: `({{testToken}})`,
imports: [AmbientModule]
})
class StandaloneCmp {
constructor(@Inject(testToken) readonly testToken: String) {}
}

const appRef = await bootstrapApplication(StandaloneCmp, {
providers: [
{provide: testToken, useValue: 'Bootstrap'},
]
});
class SilentErrorHandler extends ErrorHandler {
override handleError() {
// the error is already re-thrown by the application ref.
// we don't want to print it, but instead catch it in tests.
}
}

appRef.tick();
const appRef = await bootstrapApplication(StandaloneCmp, {
providers: [
{provide: testToken, useValue: 'Bootstrap'},
]
});

// make sure that ambient providers "shadow" ones explicitly provided during bootstrap
expect(rootEl.textContent).toBe('(Ambient)');
});
appRef.tick();

// make sure that ambient providers "shadow" ones explicitly provided during bootstrap
expect(document.body.textContent).toBe('(Ambient)');
}));

/*
This test verifies that ambient providers for the standalone component being bootstrapped
Expand All @@ -74,7 +71,7 @@ describe('bootstrapApplication for standalone components', () => {
- standalone injector (ambient providers go here);
*/
it('should create a standalone injector for standalone components with ambient providers',
async () => {
withBody('<test-app></test-app>', async () => {
const ambientToken = new InjectionToken('ambient token');

@NgModule({
Expand Down Expand Up @@ -119,10 +116,10 @@ describe('bootstrapApplication for standalone components', () => {
expect(e).toBeInstanceOf(Error);
expect((e as Error).message).toContain('No provider for InjectionToken ambient token!');
}
});
}));

it('should throw if `BrowserModule` is imported in the standalone bootstrap scenario',
async () => {
withBody('<test-app></test-app>', async () => {
@Component({
selector: 'test-app',
template: '...',
Expand All @@ -145,10 +142,10 @@ describe('bootstrapApplication for standalone components', () => {
expect((e as Error).message)
.toContain('Providers from the `BrowserModule` have already been loaded.');
}
});
}));

it('should throw if `BrowserModule` is imported indirectly in the standalone bootstrap scenario',
async () => {
withBody('<test-app></test-app>', async () => {
@NgModule({
imports: [BrowserModule],
})
Expand Down Expand Up @@ -177,5 +174,49 @@ describe('bootstrapApplication for standalone components', () => {
expect((e as Error).message)
.toContain('Providers from the `BrowserModule` have already been loaded.');
}
});
}));

it('should trigger an app destroy when a platform is destroyed',
withBody('<test-app></test-app>', async () => {
let compOnDestroyCalled = false;
let serviceOnDestroyCalled = false;
let injectorOnDestroyCalled = false;

@Injectable({providedIn: 'root'})
class ServiceWithOnDestroy {
ngOnDestroy() {
serviceOnDestroyCalled = true;
}
}

@Component({
selector: 'test-app',
standalone: true,
template: 'Hello',
})
class ComponentWithOnDestroy {
constructor(service: ServiceWithOnDestroy) {}

ngOnDestroy() {
compOnDestroyCalled = true;
}
}

const appRef = await bootstrapApplication(ComponentWithOnDestroy);
const injector = (appRef as unknown as {injector: R3Injector}).injector;
injector.onDestroy(() => injectorOnDestroyCalled = true);

expect(document.body.textContent).toBe('Hello');

const platformRef = injector.get(PlatformRef);
platformRef.destroy();

// Verify the callbacks were invoked.
expect(compOnDestroyCalled).toBe(true);
expect(serviceOnDestroyCalled).toBe(true);
expect(injectorOnDestroyCalled).toBe(true);

// Make sure the DOM has been cleaned up as well.
expect(document.body.textContent).toBe('');
}));
});

0 comments on commit 1e7f22f

Please sign in to comment.