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.

Sign up for GitHub

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

Jump to bottom

downgraded components within upgraded are not destroyed #22392

Closed
jbedard opened this issue Feb 23, 2018 · 2 comments · Fixed by Xenira/Shashki#73
Closed

downgraded components within upgraded are not destroyed #22392

jbedard opened this issue Feb 23, 2018 · 2 comments · Fixed by Xenira/Shashki#73
Labels
type: bug/fix
Milestone
needsTriage

Comments

@jbedard
Copy link
Contributor

jbedard commented Feb 23, 2018 •
edited
Loading

I'm submitting a...

[x] Bug report

Current behavior

ngOnDestroy is not called in some ng-upgrade cases. Also causes the entire DOM of the route to leak, at least in the example outlined.

Expected behavior

All on-destroy methods should be invoked in both Angular and AngularJS when using ng-upgrade.

Minimal reproduction of the problem with instructions

watch the console for all component create/destroy events
https://nested-upgrade.stackblitz.io (using router)
https://nested-upgrade2.stackblitz.io (simpler just using ngIf)

Essentially...

<downgraded-root>
   <router-outlet></router-outlet>
   <upgraded-component>         <--- used-as or nested-within the route component
       <downgraded-component>   <--- ngOnDestroy is not called
   </upgraded-component>
</downgraded-root>

The downgraded-component $destroy seems to never be called. Note that the upgraded-component ngOnDestroy is called though, so it seems like something is lost between those two?

Note that this has nothing to do with the router, that was just my original use case. Anything that adds/removes a component works.
gkalpak added a commit to gkalpak/angular that referenced this issue Feb 23, 2018
@gkalpak
fix(upgrade): correctly destroy nested downgraded component
78fe92e 
Previously, when a downgraded component was destroyed in a way that did
not trigger the `$destroy` event on the element (e.g. when a parent
element was removed from the DOM by Angular, not AngularJS), the
`ComponentRef` was not destroyed and unregistered.
This commit fixes it by listening for the `$destroy` event on both the
element and the scope.

Fixes angular#22392
@gkalpak gkalpak mentioned this issue Feb 23, 2018
Closed
3 tasks
@gkalpak
Copy link
Member

gkalpak commented Feb 23, 2018

Good catch, @jbedard!
(Fix: #22400)

gkalpak added a commit to gkalpak/angular that referenced this issue Feb 23, 2018
@gkalpak
fix(upgrade): correctly destroy nested downgraded component
048a4f5 
Previously, when a downgraded component was destroyed in a way that did
not trigger the `$destroy` event on the element (e.g. when a parent
element was removed from the DOM by Angular, not AngularJS), the
`ComponentRef` was not destroyed and unregistered.
This commit fixes it by listening for the `$destroy` event on both the
element and the scope.

Fixes angular#22392
gkalpak added a commit to gkalpak/angular that referenced this issue Feb 23, 2018
@gkalpak
fix(upgrade): correctly destroy nested downgraded component
67c5427 
Previously, when a downgraded component was destroyed in a way that did
not trigger the `$destroy` event on the element (e.g. when a parent
element was removed from the DOM by Angular, not AngularJS), the
`ComponentRef` was not destroyed and unregistered.
This commit fixes it by listening for the `$destroy` event on both the
element and the scope.

Fixes angular#22392
@ngbot ngbot bot added this to the needsTriage milestone Feb 26, 2018
gkalpak added a commit to gkalpak/angular that referenced this issue Feb 27, 2018
@gkalpak
fix(upgrade): correctly destroy nested downgraded component
2183d02 
Previously, when a downgraded component was destroyed in a way that did
not trigger the `$destroy` event on the element (e.g. when a parent
element was removed from the DOM by Angular, not AngularJS), the
`ComponentRef` was not destroyed and unregistered.
This commit fixes it by listening for the `$destroy` event on both the
element and the scope.

Fixes angular#22392
alexeagle pushed a commit that referenced this issue Feb 28, 2018
@gkalpak @alexeagle
fix(upgrade): correctly destroy nested downgraded component (#22400)
4aef9de 
Previously, when a downgraded component was destroyed in a way that did
not trigger the `$destroy` event on the element (e.g. when a parent
element was removed from the DOM by Angular, not AngularJS), the
`ComponentRef` was not destroyed and unregistered.
This commit fixes it by listening for the `$destroy` event on both the
element and the scope.

Fixes #22392

PR Close #22400
smdunn pushed a commit to smdunn/angular that referenced this issue Feb 28, 2018
@smdunn809
fix(upgrade): correctly destroy nested downgraded component (angular#…
d5d88af 
…22400)

Previously, when a downgraded component was destroyed in a way that did
not trigger the `$destroy` event on the element (e.g. when a parent
element was removed from the DOM by Angular, not AngularJS), the
`ComponentRef` was not destroyed and unregistered.
This commit fixes it by listening for the `$destroy` event on both the
element and the scope.

Fixes angular#22392

PR Close angular#22400
This was referenced Mar 15, 2018
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced Mar 15, 2018
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
leo6104 pushed a commit to leo6104/angular that referenced this issue Mar 25, 2018
@gkalpak @leo6104
fix(upgrade): correctly destroy nested downgraded component (angular#…
0324faa 
…22400)

Previously, when a downgraded component was destroyed in a way that did
not trigger the `$destroy` event on the element (e.g. when a parent
element was removed from the DOM by Angular, not AngularJS), the
`ComponentRef` was not destroyed and unregistered.
This commit fixes it by listening for the `$destroy` event on both the
element and the scope.

Fixes angular#22392

PR Close angular#22400
@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 Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
type: bug/fix
Projects
None yet
Milestone
needsTriage
Development

Successfully merging a pull request may close this issue.

Bump @angular/core from 4.4.6 to 5.2.9 in /frontend Xenira/Shashki
fix(upgrade): correctly destroy nested downgraded component gkalpak/angular
build: bump @angular/common from 5.2.5 to 5.2.9 agendaodonto/webapp
build: bump @angular/router from 5.2.5 to 5.2.9 agendaodonto/webapp
build: bump @angular/core from 5.2.5 to 5.2.9 agendaodonto/webapp
build: bump @angular/compiler-cli from 5.2.5 to 5.2.9 agendaodonto/webapp
build: bump @angular/platform-browser-dynamic from 5.2.5 to 5.2.9 agendaodonto/webapp
build: bump @angular/platform-browser from 5.2.5 to 5.2.9 agendaodonto/webapp
build: bump @angular/forms from 5.2.5 to 5.2.9 agendaodonto/webapp
build: bump @angular/http from 5.2.5 to 5.2.9 agendaodonto/webapp
2 participants
@jbedard @gkalpak