Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

onDestroy on ComponentRef isn't called in production mode #40105

Closed
magnusbakken opened this issue Dec 14, 2020 · 2 comments
Closed

onDestroy on ComponentRef isn't called in production mode #40105

magnusbakken opened this issue Dec 14, 2020 · 2 comments
Assignees
Labels
area: core Issues related to the framework runtime core: lifecycle hooks P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: has PR
Milestone

Comments

@magnusbakken
Copy link

馃悶 bug report

Affected Package

The issue is caused by package @angular/core version 11.0.4.

Is this a regression?

This behavior is new in 11.0.4. I assume it was caused by #39876. What confuses me is that that PR was supposed to fix #39365, which describes almost the exact same thing I'm seeing now, but I only encountered this issue after upgrading to 11.0.4.

Description

The callback passed to onDestroy on ComponentRef doesn't get called when the dynamic component is disposed, but only when running in production mode.

Here's a basic example using the overlay from the CDK, with disposal of the dynamic component trigged by the OverlayRef.

app.component.ts:

export class AppComponent {
  public constructor(private readonly overlay: Overlay) { }
  
  public onClick(): void {
    const overlayRef = this.overlay.create();
    const providers: StaticProvider[] = [{ provide: OverlayRefToken, useValue: overlayRef }];
    const injector = Injector.create({ providers });
    const componentPortal = new ComponentPortal(DynamicComponent, null, injector);
    const componentRef = overlayRef.attach(componentPortal);
    componentRef.onDestroy(() => console.log('closed')); // This is the important part.
  }
}

dynamic.component.ts:

export class DynamicComponent {
  public constructor(@Inject(OverlayRefToken) private readonly overlayRef: OverlayRef) { }

  public onClick(): void {
    this.overlayRef.dispose();
  }
}

export const OverlayRefToken = new InjectionToken('OVERLAY_REF');

The console.log call only triggers in dev mode.

馃敩 Minimal Reproduction

I was unable to reproduce the problem with a StackBlitz, maybe because the way files are handled in production mode is somehow different there. Instead I made a sample repo using ng new, with only the @angular/cdk package added to the standard modules.

  1. Clone this repository: https://github.com/magnusbakken/angular-ondestroy-problem
  2. Run ng serve --configuration production (I've tested that it also happens when hosting the output of ng build statically on a web server, but again only with --configuration production.)
  3. Click the Open button.
  4. Click the Close button that appears and note that nothing is logged to the console.

If run with regular ng serve in dev mode the line is logged to the console as expected. It's also logged if everything is kept the same but @angular/core is downgraded to 11.0.3.

From what I can tell it's the "production": true setting in angular.json that causes the problem. If I change that value to false and build in production mode the callback gets called.

馃實 Your Environment

Angular Version:

Angular CLI: 11.0.4
Node: 12.16.1
OS: win32 x64

Angular: 11.0.4
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router
Ivy Workspace: Yes

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1100.4
@angular-devkit/build-angular   0.1100.4
@angular-devkit/core            11.0.4
@angular-devkit/schematics      11.0.4
@angular/cdk                    11.0.2
@schematics/angular             11.0.4
@schematics/update              0.1100.4
rxjs                            6.6.3
typescript                      4.0.5
@petebacondarwin petebacondarwin added area: core Issues related to the framework runtime core: lifecycle hooks P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Dec 14, 2020
@ngbot ngbot bot added this to the Backlog milestone Dec 14, 2020
@magnusbakken magnusbakken changed the title ngOnDestroy on ComponentRef isn't called in production mode onDestroy on ComponentRef isn't called in production mode Dec 14, 2020
@mhevery
Copy link
Contributor

mhevery commented Dec 14, 2020

Investigated and working on a fix.

mhevery added a commit to mhevery/angular that referenced this issue Dec 14, 2020
PR angular#39876 introduced an error where the `onDestroy` of `ComponentRef`
would only get called if `ngDevMode` was set to true. This was because
in dev mode we would freeze `TCleanup` to verify that no more
static cleanup would get added to `TCleanup` array. This ensured
that `TCleanup` was always present in dev mode. In production the
`TCleanup` would get created only when needed. The resulting cleanup
code was incorrectly indented and would only run if `TCleanup` was
present causing this issue.

Fix angular#40105
mhevery added a commit to mhevery/angular that referenced this issue Dec 14, 2020
PR angular#39876 introduced an error where the `onDestroy` of `ComponentRef`
would only get called if `ngDevMode` was set to true. This was because
in dev mode we would freeze `TCleanup` to verify that no more
static cleanup would get added to `TCleanup` array. This ensured
that `TCleanup` was always present in dev mode. In production the
`TCleanup` would get created only when needed. The resulting cleanup
code was incorrectly indented and would only run if `TCleanup` was
present causing this issue.

Fix angular#40105
mhevery added a commit to mhevery/angular that referenced this issue Dec 14, 2020
PR angular#39876 introduced an error where the `onDestroy` of `ComponentRef`
would only get called if `ngDevMode` was set to true. This was because
in dev mode we would freeze `TCleanup` to verify that no more
static cleanup would get added to `TCleanup` array. This ensured
that `TCleanup` was always present in dev mode. In production the
`TCleanup` would get created only when needed. The resulting cleanup
code was incorrectly indented and would only run if `TCleanup` was
present causing this issue.

Fix angular#40105
mhevery added a commit to mhevery/angular that referenced this issue Dec 14, 2020
PR angular#39876 introduced an error where the `onDestroy` of `ComponentRef`
would only get called if `ngDevMode` was set to true. This was because
in dev mode we would freeze `TCleanup` to verify that no more
static cleanup would get added to `TCleanup` array. This ensured
that `TCleanup` was always present in dev mode. In production the
`TCleanup` would get created only when needed. The resulting cleanup
code was incorrectly indented and would only run if `TCleanup` was
present causing this issue.

Fix angular#40105
mhevery added a commit to mhevery/angular that referenced this issue Dec 15, 2020
PR angular#39876 introduced an error where the `onDestroy` of `ComponentRef`
would only get called if `ngDevMode` was set to true. This was because
in dev mode we would freeze `TCleanup` to verify that no more
static cleanup would get added to `TCleanup` array. This ensured
that `TCleanup` was always present in dev mode. In production the
`TCleanup` would get created only when needed. The resulting cleanup
code was incorrectly indented and would only run if `TCleanup` was
present causing this issue.

Fix angular#40105
josephperrott pushed a commit that referenced this issue Dec 22, 2020
PR #39876 introduced an error where the `onDestroy` of `ComponentRef`
would only get called if `ngDevMode` was set to true. This was because
in dev mode we would freeze `TCleanup` to verify that no more
static cleanup would get added to `TCleanup` array. This ensured
that `TCleanup` was always present in dev mode. In production the
`TCleanup` would get created only when needed. The resulting cleanup
code was incorrectly indented and would only run if `TCleanup` was
present causing this issue.

Fix #40105

PR Close #40120
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime core: lifecycle hooks P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: has PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants