Skip to content

Commit 8b6f421

Browse files
AndrewKushnirdylhunn
authored andcommitted
fix(core): trigger ApplicationRef.destroy when Platform is destroyed (angular#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 angular#46473. PR Close angular#46497
1 parent 07d1205 commit 8b6f421

File tree

7 files changed

+123
-67
lines changed

7 files changed

+123
-67
lines changed

packages/core/src/application_ref.ts

+20-5
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ export const ALLOW_MULTIPLE_PLATFORMS = new InjectionToken<boolean>('AllowMultip
6060
* `PlatformRef` class (i.e. register the callback via `PlatformRef.onDestroy`), thus making the
6161
* entire class tree-shakeable.
6262
*/
63-
const PLATFORM_ON_DESTROY = new InjectionToken<() => void>('PlatformOnDestroy');
63+
const PLATFORM_DESTROY_LISTENERS =
64+
new InjectionToken<Set<VoidFunction>>('PlatformDestroyListeners');
6465

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

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

232233
const appRef = appInjector.get(ApplicationRef);
233-
appRef.onDestroy(() => onErrorSubscription.unsubscribe());
234+
235+
// If the whole platform is destroyed, invoke the `destroy` method
236+
// for all bootstrapped applications as well.
237+
const destroyListener = () => appRef.destroy();
238+
const onPlatformDestroyListeners = platformInjector.get(PLATFORM_DESTROY_LISTENERS, null);
239+
onPlatformDestroyListeners?.add(destroyListener);
240+
241+
appRef.onDestroy(() => {
242+
onPlatformDestroyListeners?.delete(destroyListener);
243+
onErrorSubscription.unsubscribe();
244+
});
245+
234246
appRef.bootstrap(rootComponent);
235247
return appRef;
236248
});
@@ -303,7 +315,7 @@ export function createPlatformInjector(providers: StaticProvider[] = [], name?:
303315
name,
304316
providers: [
305317
{provide: INJECTOR_SCOPE, useValue: 'platform'},
306-
{provide: PLATFORM_ON_DESTROY, useValue: () => _platformInjector = null}, //
318+
{provide: PLATFORM_DESTROY_LISTENERS, useValue: new Set([() => _platformInjector = null])},
307319
...providers
308320
],
309321
});
@@ -523,8 +535,11 @@ export class PlatformRef {
523535
this._modules.slice().forEach(module => module.destroy());
524536
this._destroyListeners.forEach(listener => listener());
525537

526-
const destroyListener = this._injector.get(PLATFORM_ON_DESTROY, null);
527-
destroyListener?.();
538+
const destroyListeners = this._injector.get(PLATFORM_DESTROY_LISTENERS, null);
539+
if (destroyListeners) {
540+
destroyListeners.forEach(listener => listener());
541+
destroyListeners.clear();
542+
}
528543

529544
this._destroyed = true;
530545
}

packages/core/test/bundling/animations/bundle.golden_symbols.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -354,13 +354,13 @@
354354
"name": "PARAM_REGEX"
355355
},
356356
{
357-
"name": "PLATFORM_ID"
357+
"name": "PLATFORM_DESTROY_LISTENERS"
358358
},
359359
{
360-
"name": "PLATFORM_INITIALIZER"
360+
"name": "PLATFORM_ID"
361361
},
362362
{
363-
"name": "PLATFORM_ON_DESTROY"
363+
"name": "PLATFORM_INITIALIZER"
364364
},
365365
{
366366
"name": "PlatformRef"

packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -357,13 +357,13 @@
357357
"name": "Optional"
358358
},
359359
{
360-
"name": "PLATFORM_ID"
360+
"name": "PLATFORM_DESTROY_LISTENERS"
361361
},
362362
{
363-
"name": "PLATFORM_INITIALIZER"
363+
"name": "PLATFORM_ID"
364364
},
365365
{
366-
"name": "PLATFORM_ON_DESTROY"
366+
"name": "PLATFORM_INITIALIZER"
367367
},
368368
{
369369
"name": "PlatformRef"

packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -351,13 +351,13 @@
351351
"name": "Optional"
352352
},
353353
{
354-
"name": "PLATFORM_ID"
354+
"name": "PLATFORM_DESTROY_LISTENERS"
355355
},
356356
{
357-
"name": "PLATFORM_INITIALIZER"
357+
"name": "PLATFORM_ID"
358358
},
359359
{
360-
"name": "PLATFORM_ON_DESTROY"
360+
"name": "PLATFORM_INITIALIZER"
361361
},
362362
{
363363
"name": "PlatformRef"

packages/core/test/bundling/router/bundle.golden_symbols.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -432,13 +432,13 @@
432432
"name": "OutletInjector"
433433
},
434434
{
435-
"name": "PLATFORM_ID"
435+
"name": "PLATFORM_DESTROY_LISTENERS"
436436
},
437437
{
438-
"name": "PLATFORM_INITIALIZER"
438+
"name": "PLATFORM_ID"
439439
},
440440
{
441-
"name": "PLATFORM_ON_DESTROY"
441+
"name": "PLATFORM_INITIALIZER"
442442
},
443443
{
444444
"name": "PathLocationStrategy"

packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -222,13 +222,13 @@
222222
"name": "Observable"
223223
},
224224
{
225-
"name": "PLATFORM_ID"
225+
"name": "PLATFORM_DESTROY_LISTENERS"
226226
},
227227
{
228-
"name": "PLATFORM_INITIALIZER"
228+
"name": "PLATFORM_ID"
229229
},
230230
{
231-
"name": "PLATFORM_ON_DESTROY"
231+
"name": "PLATFORM_INITIALIZER"
232232
},
233233
{
234234
"name": "R3Injector"

packages/platform-browser/test/browser/bootstrap_standalone_spec.ts

+88-47
Original file line numberDiff line numberDiff line change
@@ -6,63 +6,60 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

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

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

1515
describe('bootstrapApplication for standalone components', () => {
16-
let rootEl: HTMLUnknownElement;
17-
beforeEach(inject([DOCUMENT], (doc: any) => {
18-
rootEl = getDOM().createElement('test-app', doc);
19-
getDOM().getDefaultDocument().body.appendChild(rootEl);
20-
}));
21-
22-
afterEach(() => {
23-
destroyPlatform();
24-
rootEl?.remove();
25-
});
26-
2716
class SilentErrorHandler extends ErrorHandler {
2817
override handleError() {
2918
// the error is already re-thrown by the application ref.
3019
// we don't want to print it, but instead catch it in tests.
3120
}
3221
}
3322

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

37-
@NgModule({
38-
providers: [
39-
{provide: testToken, useValue: 'Ambient'},
40-
]
41-
})
42-
class AmbientModule {
43-
}
27+
@NgModule({
28+
providers: [
29+
{provide: testToken, useValue: 'Ambient'},
30+
]
31+
})
32+
class AmbientModule {
33+
}
4434

45-
@Component({
46-
selector: 'test-app',
47-
standalone: true,
48-
template: `({{testToken}})`,
49-
imports: [AmbientModule]
50-
})
51-
class StandaloneCmp {
52-
constructor(@Inject(testToken) readonly testToken: String) {}
53-
}
35+
@Component({
36+
selector: 'test-app',
37+
standalone: true,
38+
template: `({{testToken}})`,
39+
imports: [AmbientModule]
40+
})
41+
class StandaloneCmp {
42+
constructor(@Inject(testToken) readonly testToken: String) {}
43+
}
5444

55-
const appRef = await bootstrapApplication(StandaloneCmp, {
56-
providers: [
57-
{provide: testToken, useValue: 'Bootstrap'},
58-
]
59-
});
45+
class SilentErrorHandler extends ErrorHandler {
46+
override handleError() {
47+
// the error is already re-thrown by the application ref.
48+
// we don't want to print it, but instead catch it in tests.
49+
}
50+
}
6051

61-
appRef.tick();
52+
const appRef = await bootstrapApplication(StandaloneCmp, {
53+
providers: [
54+
{provide: testToken, useValue: 'Bootstrap'},
55+
]
56+
});
6257

63-
// make sure that ambient providers "shadow" ones explicitly provided during bootstrap
64-
expect(rootEl.textContent).toBe('(Ambient)');
65-
});
58+
appRef.tick();
59+
60+
// make sure that ambient providers "shadow" ones explicitly provided during bootstrap
61+
expect(document.body.textContent).toBe('(Ambient)');
62+
}));
6663

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

8077
@NgModule({
@@ -119,10 +116,10 @@ describe('bootstrapApplication for standalone components', () => {
119116
expect(e).toBeInstanceOf(Error);
120117
expect((e as Error).message).toContain('No provider for InjectionToken ambient token!');
121118
}
122-
});
119+
}));
123120

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

150147
it('should throw if `BrowserModule` is imported indirectly in the standalone bootstrap scenario',
151-
async () => {
148+
withBody('<test-app></test-app>', async () => {
152149
@NgModule({
153150
imports: [BrowserModule],
154151
})
@@ -177,5 +174,49 @@ describe('bootstrapApplication for standalone components', () => {
177174
expect((e as Error).message)
178175
.toContain('Providers from the `BrowserModule` have already been loaded.');
179176
}
180-
});
177+
}));
178+
179+
it('should trigger an app destroy when a platform is destroyed',
180+
withBody('<test-app></test-app>', async () => {
181+
let compOnDestroyCalled = false;
182+
let serviceOnDestroyCalled = false;
183+
let injectorOnDestroyCalled = false;
184+
185+
@Injectable({providedIn: 'root'})
186+
class ServiceWithOnDestroy {
187+
ngOnDestroy() {
188+
serviceOnDestroyCalled = true;
189+
}
190+
}
191+
192+
@Component({
193+
selector: 'test-app',
194+
standalone: true,
195+
template: 'Hello',
196+
})
197+
class ComponentWithOnDestroy {
198+
constructor(service: ServiceWithOnDestroy) {}
199+
200+
ngOnDestroy() {
201+
compOnDestroyCalled = true;
202+
}
203+
}
204+
205+
const appRef = await bootstrapApplication(ComponentWithOnDestroy);
206+
const injector = (appRef as unknown as {injector: R3Injector}).injector;
207+
injector.onDestroy(() => injectorOnDestroyCalled = true);
208+
209+
expect(document.body.textContent).toBe('Hello');
210+
211+
const platformRef = injector.get(PlatformRef);
212+
platformRef.destroy();
213+
214+
// Verify the callbacks were invoked.
215+
expect(compOnDestroyCalled).toBe(true);
216+
expect(serviceOnDestroyCalled).toBe(true);
217+
expect(injectorOnDestroyCalled).toBe(true);
218+
219+
// Make sure the DOM has been cleaned up as well.
220+
expect(document.body.textContent).toBe('');
221+
}));
181222
});

0 commit comments

Comments
 (0)