Primeng: p-dialog doesn't destroy inner component when has been closed

[opetriienko]

I'm submitting a ... (check one with "x")

[x] bug report
[ ] feature request
[ ] support request

Plunkr Case (Bug Reports)
https://stackblitz.com/edit/angular-bgkhqj

Current behavior
If we have an inner component inside the p-dialog:
1) ngOnInit of inner component is invoked even if dialog wasn't opened;
2) ngOnDestroy of inner component is not invoked when dialog was closed.

Expected behavior
1) ngOnInit is invoked when p-dialog is opened only.
2) ngOnDestroy of inner component is invoked when dialog was closed.

Minimal reproduction of the problem with instructions
Add p-dialog component with some inner component inside in the html (is already in stackblitz example).
Do toggle visibility of the dialog and see console logs added in stackblitz example.

What is the motivation / use case for changing the behavior?
Not an expected behavior. Performance improvement.

Please tell us about your environment:

• Angular version: 6.1.0
• PrimeNG version: 6.0.2

• Browser: [all]

• Language: [TypeScript 2.9.2]

• Node (for AoT issues): node --version = 8.11.3

[sourabh8003]

@opetriienko: It's not a bug in p-dialog, ngOnInit is a life cycle hook called by Angular2 to indicate that Angular is done creating the component that's why you are getting the messages printed on browser's console.

[opetriienko]

@sourabh8003, so technically there is no DOM element of this inner component when a dialog is closed, but it always lives in a components tree. It means if I have some component with a form inside the dialog, I can't set up the form in ngOnInit hook, I have to manually recreate(reset) the form when p-dialog has been opened/closed. Is it really an expected behavior?

[ctrl-brk]

Yep. That's what I do.

[opetriienko]

It sounds weird to me when the DOM element of the component is removed, but the component by itself still lives in a components tree. That's a quote from https://www.primefaces.org/primeng-6-0-2-released/

With 6.0.2, overlays fully utilize modern Angular animations API in a lazy manner meaning they do not exist in the DOM and only get created when they are needed such as on user click, similarly they get destroyed altogether instead of hidden with css. This improves the performance of the pages having many components with overlays significantly. In addition, a new common animation is applied to all overlay components for a unified user experience.

I've extended and overridden the previous version of p-dialog, so I removed the container of the dialog after animation is finished. That's what I expected in 6.02, I haven't seen 6.02 sources so far. But it seems I have to do the same by my own again.

[sourabh8003]

@opetriienko : You can initialize the form on OnInit if you are using angular's reactive form & initialization should happen inside of a OnChanges method if data comes from any parent component.

[opetriienko]

@sourabh8003 thanks for helping, but that's not the issue here at all. I know how to reset the form 😉
Are you an author of PrimeNG?
From my perspective, we have to destroy(according to Angular lifecycle) all content inside the overlay, but now we just remove a DOM element without destroying from Angular component tree.
If an author thinks in a different way, please, just close the issue.

[cagataycivici]

I think this is the expected behavior of Angular. Dialog does not what is inside of it, Angular manages the lifecycle of inner components, dialog just has an ngIf for its main container element.

[JBusch]

I don't think this should be expected behavior. Not firing ngOnDestory on close raises many issues. For example, you can not manage unsubscribes properly.

[DavyDeDurpel]

I totally agree with @JBusch

The problem with p-Dialog is that it just behaves as a display: block/none at this moment. So it is only really useful for static content. But it's not clear from the documentation that this is what a developer should expect from it.

It would be nice if the documentation is updated to warn developers about this behaviour.

At the same time it might be wise to have a look at the implementation of the dialog in Angular Material:
https://material.angular.io/components/dialog/overview

Their dialog implementation works as any developer would expect. The component you pass as content is only initialized when showing the dialog and destroyed when hiding the dialog. This way you can easily reuse components without having to implement all sorts of lifecycle hooks to get the same behaviour. At the same time it also fixes the animation behaviour that is mentioned in another ticket.

I really like PrimeNG. It's a multitude of times better than Angular Material. But the dialog component at this moment really sucks. (sorry for the language)

[rohkoli94]

So basically,visible property when set to false,it only hides the inner component (means inner component is present in DOM) ,whereas setting *ngIf condition to inner component will remove it from DOM(thus destroys inner component) when it is False.
Cheers!

[vangras]

I totally agree with @DavyDeDurpel

PrimeNG is far better than Angular Material except for the dialog. I think the dialog should be completely re-implemented in a way similar to the Material Dialog.