Skip to content

Commit

Permalink
fix(core): unsubscribe from the onError when the root view is remov…
Browse files Browse the repository at this point in the history
…ed (#39940)

At the moment, when creating a root module, a subscription to the
`onError` subject is also created. It captures the scope where `NgModuleRef`
is created and prevents it from being garbage collected. Also note that this
`NgModuleRef` has a reference to the root module instance (e.g. `AppModule`),
which also prevents it from being GC'd.

PR Close #39940
  • Loading branch information
arturovt authored and mhevery committed Dec 3, 2020
1 parent f01c713 commit 5a3a154
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 8 deletions.
17 changes: 11 additions & 6 deletions packages/core/src/application_ref.ts
Expand Up @@ -350,12 +350,17 @@ export class PlatformRef {
if (!exceptionHandler) {
throw new Error('No ErrorHandler. Is platform module (BrowserModule) included?');
}
moduleRef.onDestroy(() => remove(this._modules, moduleRef));
ngZone!.runOutsideAngular(() => ngZone!.onError.subscribe({
next: (error: any) => {
exceptionHandler.handleError(error);
}
}));
ngZone!.runOutsideAngular(() => {
const subscription = ngZone!.onError.subscribe({
next: (error: any) => {
exceptionHandler.handleError(error);
}
});
moduleRef.onDestroy(() => {
remove(this._modules, moduleRef);
subscription.unsubscribe();
});
});
return _callAndReportToErrorHandler(exceptionHandler, ngZone!, () => {
const initStatus: ApplicationInitStatus = moduleRef.injector.get(ApplicationInitStatus);
initStatus.runInitializers();
Expand Down
20 changes: 18 additions & 2 deletions packages/core/test/acceptance/bootstrap_spec.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ApplicationRef, COMPILER_OPTIONS, Component, destroyPlatform, NgModule, TestabilityRegistry, ViewEncapsulation} from '@angular/core';
import {ApplicationRef, COMPILER_OPTIONS, Component, destroyPlatform, NgModule, NgZone, TestabilityRegistry, ViewEncapsulation} from '@angular/core';
import {expect} from '@angular/core/testing/src/testing_internal';
import {BrowserModule} from '@angular/platform-browser';
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
Expand Down Expand Up @@ -227,6 +227,22 @@ describe('bootstrap', () => {
}));
});

describe('PlatformRef cleanup', () => {
it('should unsubscribe from `onError` when Injector is destroyed',
withBody('<my-app></my-app>', async () => {
const TestModule = createComponentAndModule();

const ngModuleRef = await platformBrowserDynamic().bootstrapModule(TestModule);
const ngZone = ngModuleRef.injector.get(NgZone);

expect(ngZone.onError.observers.length).toBe(1);

ngModuleRef.destroy();

expect(ngZone.onError.observers.length).toBe(0);
}));
});

onlyInIvy('options cannot be changed in Ivy').describe('changing bootstrap options', () => {
beforeEach(() => {
spyOn(console, 'error');
Expand Down Expand Up @@ -365,4 +381,4 @@ export class MultipleSelectorsAppComponent {
bootstrap: [MultipleSelectorsAppComponent],
})
export class MultipleSelectorsAppModule {
}
}

0 comments on commit 5a3a154

Please sign in to comment.