fix(upgrade): leak when angular1 destroys element#6460
Conversation
|
Fixes #6401 ?=140406f |
|
Hi, thanks! |
|
@tbosch: added test. |
|
This needs to be merged- without it, any angular1 app that uses angular2 components will never clean up those components. |
|
@tbosch: mind giving this another look? |
|
@tbosch Please review. We need this fix seriously. |
|
9 betas and 2 months later still no feedback here. Is there anything I need to change for it to get merged faster? It's absolutely critical for a few apps I've been working on. angular1->2 upgraded apps simply can't function right without this fix - anything that uses an angular1 And it's not just resources, for example I have some logic that shows a modal when a certain websocket event arrives, and there is no way to unsubscribe from it when the user navigates away. The Paging more people (sorry): @tbosch @vsavkin @IgorMinar |
|
|
||
| registerCleanup() { | ||
| this.element.bind('$remove', () => this.viewManager.destroyRootHostView(this.hostViewRef)); | ||
| this.element.bind('$destroy', () => this.viewManager.destroyRootHostView(this.hostViewRef)); |
There was a problem hiding this comment.
Nitpick: .on() is supposed to be preferred with jQuery.
(Without jQuery, .bind() is just an alias to .on().)
|
FWIW, it LGTM 😃 |
|
@andreialecu sorry about the delay.. this should be out with the next beta. |
|
Merging PR #6460 on behalf of @alxhub to branch presubmit-alxhub-pr-6460. |
|
@andreialecu This is merged now via #7935 (I squashed your commits into one). Thanks! |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes #6401
Not sure why this is bound to
$remove, the proper event is$destroyaccording to https://docs.angularjs.org/api/ng/function/angular.element