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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Dynamic Dialog not being properly destroyed on close #17123

Merged

Conversation

stewieoO
Copy link
Contributor

Fixes #16402

I'm not 100% sure why this works considering the component ref gets destroyed before calling it's change detector, but as far as i can tell it just works.

Copy link

vercel bot commented Dec 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
primeng ✅ Ready (Inspect) Visit Preview Dec 18, 2024 9:04pm
primeng-v18 ✅ Ready (Inspect) Visit Preview Dec 18, 2024 9:04pm

Copy link

vercel bot commented Dec 18, 2024

@stewieoO is attempting to deploy a commit to the primetek Team on Vercel.

A member of the Team first needs to authorize it.

@vercel vercel bot temporarily deployed to Preview – primeng-v18 December 18, 2024 21:04 Inactive
@mertsincan mertsincan added the Status: Discussion Issue or pull request needs to be discussed by Core Team label Dec 25, 2024
@cetincakiroglu
Copy link
Contributor

Hi,

Thanks a lot for the effort and support!

@cetincakiroglu cetincakiroglu merged commit 96ed38d into primefaces:master Dec 26, 2024
2 of 3 checks passed
@cetincakiroglu cetincakiroglu removed the Status: Discussion Issue or pull request needs to be discussed by Core Team label Dec 26, 2024
@ThoSap
Copy link
Contributor

ThoSap commented Dec 30, 2024

@stewieoO @cetincakiroglu the issue has been present and has been caused since the Angular 18.2.2 release and is unrelated to the mentioned PrimeNG 17.18.10 release in #16402.

My zoneless PrimeNG application worked perfectly fine with PrimeNG 17.18.15 until I upgraded from Angular 18.2.0 to 18.2.13.

I thoroughly tracked down the root cause of the bug, and it seems to be a change in the @angular/[email protected] package.
If I upgrade ALL @angular/* dependencies to 18.2.13 and pin @angular/core to 18.2.1, my dynamic dialog properly closes and destroys the overlay mask.

The change that causes this is an afterRender hook change in @angular/core, which is not even mentioned in the Angular 18.2.2 changelog:
angular/angular@800f6c8
angular/angular@6059ca8
angular/angular@b80af11

After this change even some Angular tests and the angular.dev website had to add an additional ref destroy call, the same as we did with the workaround in #16402 (comment), see:
angular/angular@df42d2b
angular/angular@5e9661d

Related PRs:
angular/angular#57453
angular/angular#57504
angular/angular#57546
angular/angular#57552
angular/angular#57554

@stewieoO
Copy link
Contributor Author

stewieoO commented Jan 2, 2025

@stewieoO @cetincakiroglu the issue has been present and has been caused since the Angular 18.2.2 release and is unrelated to the mentioned PrimeNG 17.18.10 release in #16402.

My zoneless PrimeNG application worked perfectly fine with PrimeNG 17.18.15 until I upgraded from Angular 18.2.0 to 18.2.13.

I thoroughly tracked down the root cause of the bug, and it seems to be a change in the @angular/[email protected] package. If I upgrade ALL @angular/* dependencies to 18.2.13 and pin @angular/core to 18.2.1, my dynamic dialog properly closes and destroys the overlay mask.

The change that causes this is an afterRender hook change in @angular/core, which is not even mentioned in the Angular 18.2.2 changelog: angular/angular@800f6c8 angular/angular@6059ca8 angular/angular@b80af11

After this change even some Angular tests and the angular.dev website had to add an additional ref destroy call, the same as we did with the workaround in #16402 (comment), see: angular/angular@df42d2b angular/angular@5e9661d

Related PRs: angular/angular#57453 angular/angular#57504 angular/angular#57546 angular/angular#57552 angular/angular#57554

Oh, that is interesting. The changes to the angular tests you mentioned only do the .destroy() though, not the .detectChanges()
I don't remember exactly but i think this is only "broken" when you create a component from a global service and attach it to the body.
If you create a component from inside another component and/or attach it somewhere inside the application tree, it was removed without the additional detectChanges() call

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Component: DynamicDialog not working with zonejs less application
4 participants