ComponentRef<T> onDestroyed() callback does not get called

[ghost]

🐞 bug report

Affected Package

The issue is caused by package @angular/core

Is this a regression?

Yes, the previous version in which this bug was not present was: Pre-Ivy or if you disable Ivy

Description

Problem: ComponentRef.onDestroyed callback does not get called

A component gets dynamically created via ComponentFactoryResolver. It is then inserted into the view.

@ViewChild("anchor", { read: ViewContainerRef })
anchor: ViewContainerRef;

this.compRef = this.cfr
    .resolveComponentFactory(DynamicComponent)
    .create(this.injector);

this.compRef.onDestroy(() =>
    console.log("DynamicComponent onDestroyed callback")
);

this.anchor.insert(this.compRef.hostView);

As you can see, the onDestroyed callback is attached. If the component gets destroyed by its ComponentRef, the callback gets called correctly like so:

this.compRef.destroy();

if the component gets destroyed by clearing the anchor, the callback does not get called:

this.anchor.clear();

As far as I can tell, the onDestroyed callback ONLY gets called when the component gets destroyed by its ComponentRef. Whenever the component gets destroyed by clearing parent views or navigating away or data binding, the callback does not get called.

It works in NG 8.2.14 and it works if you set enableIvy: false in tsconfig.json.

🔬 Minimal Reproduction

https://stackblitz.com/edit/angular-ivy-ev4v2y

🌍 Your Environment

Angular Version:

Angular CLI: 10.1.6
Node: 12.18.1
OS: win32 x64

Angular: 10.1.5
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... platform-webworker, router
Ivy Workspace: No

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1001.6
@angular-devkit/build-angular   0.1001.6
@angular-devkit/core            10.1.6
@angular-devkit/schematics      10.1.6
@angular/cdk                    10.2.4
@angular/cli                    10.1.6
@angular/material               10.2.4
@schematics/angular             10.1.6
@schematics/update              0.1001.6
rxjs                            6.6.3
typescript                      4.0.3

[jessicajaniuk]

Thanks for the report and for the reproduction! We've done an investigation and this is related to an item mentioned on the Ivy Compatibility guide, specifically the one related to no longer being able to overwrite hooks on directive instances. The investigation found a difference in how Ivy handles componentRef and componentInstance compared to ViewEngine. In Ivy, the callback provided in the componentRef.onDestroy call is stored on the componentRef instance and is not available to the view that ViewContainerRef manipulates. Further investigation will be needed.

A workaround is to manually call compRef.destroy when you want to ensure the lifecycle hook is called.

[ghost]

@jessicajaniuk Thanks for the info. Is it possible to get the componentRef instance in the view via dependency injection or something? This way I just could call componentRef.destroy() whenever ngOnDestroy() gets called. That would make things easy.

Pseudo code ahead:

...
export class SomeComponent implements OnDestroy {

  constructor(private compRef: ComponentRef<SomeComponent>) { }

  public ngOnDestroy(): void {
    this.compRef.destroy();
  }
}

This would be cool... if that's not possible I would have to set the `componentRef`` instance manually via a setter or method.

[jessicajaniuk]

@FSDRE Is your pseudocode example referring to the child component scope or the parent component that's using the ComponentFactoryResolver? I'm a little unclear on that. In the case of your stackblitz reproduction, the ComponentFactoryResolver returns the componentRef. So you'd have access to the destroy method from that ref instance and could call it manually when calling anchor.clear(). That would ensure the ngOnDestroy method is called in the child component, which is the suggested workaround.

[ghost]

@jessicajaniuk The pseudocode is the dynamically created component. The thing is, the reproduction is only that. In the real application I do not always destory the component via compRef.destroy() or anchor.clear().
For example, I add the component dynamically to a parent. At some point the parent of the parent of the parent gets destroyed by an *ngIf data binding thereby also destroying the dynamically created component a few layers deeper. I simply don't always know when the dynamically created component will get destroyed or differently said: It is not always the component itself or it's anchor that destroys it. I rely on the onDestroyed hook being called. The only thing that right now knows when the component gets destroyed in Ivy is the view's ngOnDestroy hook. But that does not help me since I do not have the compRef instance in the view.
Our app surely is an edge-case because the entire UI get's built from JSON and therefor is 95% dynamically created. But nevertheless it is currently unusable with Ivy since code that ran before does not run now (the onDestroyed hook), and that is pretty bad. We are talking about 50-60 hooks throughout the app not running their code anymore. You can imagine what that means... nothing works 😄

[Achilles1515]

A lot of things to unpack here.

First, the documentation for componentRef.onDestroy() is incredibly lacking.

Not a single mention about being able to call this method multiple times and each of the callbacks are placed in an array and iterated over when destroyed. And strictly speaking, the behavior exhibited in this issue report appears to be consistent with the documentation, which states the callback function is called when the destroy() method is invoked. The confusing part is that this only occurs in user code. The documentation should make this clear and also mention when the callbacks are executed in relation to the ngOnDestroy hook being called, as this is another point of disparity between the engines:

View Engine: ngOnDestroy() --- then --> onDestroy()
Ivy: onDestroy() --- then --> ngOnDestroy()

Second, and probably even more alarming, is that the viewRef.onDestroy() callbacks aren't being invoked either in Ivy.
In the StackBlitz, change this line:

this.compRef.onDestroy(() =>
        console.log("DynamicComponent onDestroyed callback")
      );

to

// this.compRef.onDestroy(() =>
this.compRef.hostView.onDestroy(() =>
        console.log("DynamicComponent onDestroyed callback")
      );

You would also expect this callback to be invoked in both destruction scenarios, but it is called in neither in Ivy!

angular/packages/core/src/render3/instructions/shared.ts

Lines 753 to 761 in d939b5f

	export function storeCleanupWithContext(
	tView: TView, lView: LView, context: any, cleanupFn: Function): void {
	const lCleanup = getLCleanup(lView);
	lCleanup.push(context);
	if (tView.firstCreatePass) {
	getTViewCleanup(tView).push(cleanupFn, lCleanup.length - 1);
	}
	}

I don't know why the

if (tView.firstCreatePass){
}

conditional is even there and it is blocking the callbacks from being stored. Manually setting firstCreatePass to true will "fix" the issue and allows the callbacks to be invoked.

(this.compRef.hostView as any)._lView[1].firstCreatePass = true;

So @FSDRE , the final workaround looks like this:

const TVIEW = 1;
const tView = (this.compRef.hostView as any)._lView[TVIEW];
tView.firstCreatePass = true;
// Put callback on the host view instead of the component ref.
this.compRef.hostView.onDestroy(() =>
  console.log("DynamicComponent onDestroyed callback")
);
tView.firstCreatePass = false; //...I guess?

And to add even more confusion and gotchas, the calling order here in Ivy is ngOnDestroy() --> viewRef.onDestroy(), which is opposite of the componentRef.destroy() calling order.

---

The investigation found a difference in how Ivy handles componentRef and componentInstance compared to ViewEngine. In Ivy, the callback provided in the componentRef.onDestroy call is stored on the componentRef instance and is not available to the view that ViewContainerRef manipulates.

@jessicajaniuk A similar problem (internal view not knowing about it's ViewRef) exists with this memory leak I reported about two months ago that has zero discussion.
#38648

[ghost]

@Achilles1515 Thank you very much for your input! That's some "big oooof's" right there. I am not keen on implementing such hacky workarounds in production code that might break again in a couple updates. I think I will register my onDestroy()callbacks directly in the components and call them whenever ngOnDestroy() gets called. Or I simply revert to View Engine because I have a much better feeling with it when deploying my app to production.

Is there an EOL date for VE?

I have reported at least 3 similar messed up issues regarding Ivy and so far I don't see anything happening other than auto-locking. Currently Ivy creates more problems than it solves, unfortunately. Maybe the release was a bit rushed.

[AndrewKushnir]

Hi, just want to give a quick update: the issue with the onDestroy callback not being invoked in Ivy should be resolved by #39876. Thank you.

[AndrewKushnir]

Closing this ticket since the corresponding PR #39876 was merged and the changes will be available after the next patch release (v11.0.4). Thank you.

[angular-automatic-lock-bot]

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.}