component's controller $onDestroy hook is not called on manual $scope.$destroy from inside the controller #14376

artaommahe · 2016-04-05T16:01:57Z

If we call $scope.$destroy() then $onDestroy hook never fires, $scope.$on('$destroy') works well
e.x. watch in console https://jsfiddle.net/artaommahe/agL5zuzf/1/

on parent scope destroy both hooks work well

The text was updated successfully, but these errors were encountered:

Narretz · 2016-04-05T17:50:27Z

This seems to happen only in this very specific case where you destroy the controller scope from inside a timeout in the controller. I guess this is a contrived example. What's the real use case you have? Why would you destroy the scope from inside the controller?

Narretz · 2016-04-05T18:13:40Z

So this happens because there is a mismatch between the scopes. The way I see it, we bind our internal $onDestroy on the compile scope (that scope that is passed to compile), but the scope that is passed to the controller is a child of this scope. So when you destroy the scope inside the controller, you are not triggering the destroy handler on its parent.
What additionally confuses me is that the controller scope shouldn't actually be a child of the compile scope, but a child of the root scope (since it's isolate).
The naive fix for the bug at hand would be this:

        (isolateScope || controllerScope).$on('$destroy', function callOnDestroyHook() {
          controllerInstance.$onDestroy();
        });

@petebacondarwin @gkalpak do you have an idea what's going on here?

petebacondarwin · 2016-04-05T18:29:01Z

As far as the example given is concerned, I would say that this is invalid. Destroying the inner scope has got nothing to do with destroying the component itself. The component exists in a scope, which happens to be the compile scope. Assuming you are not doing anything naughty then that scope is destroyed when the component's element is destroyed.

See the following for a realistic example: http://plnkr.co/edit/578Yvv0XU3brYXwco4h5?p=preview

artaommahe · 2016-04-05T23:40:56Z

What's the real use case you have

Dynamically created directives as <body> childs, i use them in production as video holders. They can be closed and as they was added dynamically i must manually destroy element and his scope. On destroy i need to cleanup some external libs usage, so i use $onDestroy hook.
Also current behaviour is invalid due to discrepancy between same(outside) logic - $scope.$on('$destroy') and $onDestroy perceived as equal things with equal behaviour.

artaommahe · 2016-04-07T22:57:10Z

@petebacondarwin Hm, can you review issue resolution due to real world example that i described above? My jsfiddle from first post was about bug repoducing, not about real case from project. This problem is actual for dinamically created directives.

thorn0 · 2016-04-29T21:30:40Z

$scope.$on('$destroy') and $onDestroy perceived as equal things with equal behaviour.

True. I just moved the code from the former to the latter without a second thought. And only now that I read this issue I realized that it's not that simple.

ramunsk · 2016-06-15T03:50:45Z

Not sure if related but selecting a directive node in Chrome devtools and executing angular.element($0).remove() does not trigger $onDestroy on directive controller.

petebacondarwin · 2016-06-15T07:42:28Z

$onDestroy gets called when the "containing" scope, i.e the scope in which the component lives, is destroyed.

So:

destroying an isolate scope that the component creates will not trigger $onDestroy
removing the element from the DOM will not trigger $onDestroy

CarlLeth · 2016-07-12T17:34:50Z

Why does removing the element from the DOM not trigger $onDestroy? The component is destroyed. You're saying that $onDestroy does not always get called when the component is destroyed (in fact, in the most common way components are destroyed). Why did you name it $onDestroy? Shouldn't it be called $occasionallyOnDestroy?

I'm subscribing to an event in my component and I want to unsubscribe when it's destroyed (removed from the DOM), but $onDestroy is never called. $scope.$on('$destroy') works fine. I'm not doing anything fancy or unusual. I'm adding a component to the DOM, then removing it later. $onInit gets called every time I add it. Why does $onDestroy not get called when I remove it?

petebacondarwin · 2016-07-12T18:03:53Z

Shouldn't it be called $occasionallyOnDestroy?

No need to be flippant :-)

As it clearly states in the docs: https://docs.angularjs.org/api/ng/service/$compile

Called on a controller when its containing scope is destroyed.

So, $onDestroy will only be called when the scope that contains it, that in which the component lives, is destroyed.

Manually removing an element from the DOM without dealing with its DOM will lead to memory leaks, since the component bindings will have created watches on the containing scope. The correct way to add and remove things from the DOM is demonstrated inside directives such as ngRepeat, ngIf and ngSwitch, where we create a new scope for the components that we wish to add and remove. Then we can safely remove the component, and its containing scope without causing memory leaks.

artaommahe · 2016-07-12T18:23:29Z

@petebacondarwin please look at jsfiddle from first post - there is explicit $scope.$destroy() call and $onDestroy() hook does not work.
Ofc it's artificial example, in real app i dynamically create components with new scopes and then remove them from dom with their scope destroy, and $onDestroy() never fires.

CarlLeth · 2016-07-12T19:32:47Z

removing the element from the DOM will not trigger $onDestroy

Ok, I had misunderstood this comment to mean that $onDestroy will be not called if you remove it from the DOM in any way, such as by setting a containing ng-if's condition to false, for instance. In fact this is exactly the problem I was seeing. This certainly was the case in 1.5.0, but it appears to be fixed in later versions.

To anyone else having this problem: update to 1.5.3 or later.
JSFiddle demonstrating the (now fixed) issue: https://jsfiddle.net/7cy4rwy9/

gkalpak · 2016-07-12T19:41:51Z

@CarlLeth, the $onDestroy hook was introduced in 1.5.3. In prior version, $onDestroy is just a regular method on the controller. Angular won't call it.

@artaommahe, your example shows $destroying the scope created for the component's template. Not the scope which the component "lives in". As @petebacondarwin explained, the proper way of destroying a component/directive involves removing its "surrounding" scope (on which there have been watchers for its bindings set up).

gkalpak · 2016-09-07T12:55:06Z

Closing since everything seems to work as expected (and we didn't hear from the OP in a while).
_{Feel free to continue the discussion below, if there is still anything that needs clarification.}

szaccaria · 2016-09-12T22:55:38Z

Hi, in this days I stumbled in the same problem @artaommahe
I hit my head to the wall for many time... after I had tried write something.
Here my code.

http://jsfiddle.net/zaquas/sn2br6ka/ (try move the windows, close it and watch in console, use the right button mouse to create new o push the button... the new window will be create in a stack, move for clear)

I don't know if this is the right way for destroy the dinamic component, but it is the best that I can do.

I would not use the "ng-if" directive as suggest @petebacondarwin because I would use the two component "myCanvas" and "myWindow" also in plain html, like:

Something here

without polluting the tag "my-window" with "ng-if" directive connect with $ctrl of my-canvas, that some my user may not understand...

The problem now is that the componet in plain html inside my-canvas is wrap with the tag and the other inject with the button "add win" is without this... I would have the windows tags in the same level.
Can somebody help me?

I would note that i destroy the parent scope of window, that is the child scope of canvas. Is right?

Thanks for read this.
Stefano

szaccaria · 2016-09-13T08:28:54Z

Other problem...

http://jsfiddle.net/zaquas/wezb90b1/

Try to add item, close it and watch in console
Why when I destroyed the parent $scope, the $scope and $ctrl of child item continues to work?
Note: after push the "close item" button, the $onDestroy callback is fire, but if you try again push the "close button" fire again the function "close()" binding. I hope that this not will be fire!

But now i found something of interest...

The ng-click="$ctrl.close()" bind the call with the dom event, so when I click it, the parent $scope is destroyed, so the child $scope, but the bind remain in the dom.
Look at

$scope.$on('$destroy', function() { console.log('$scope.$on $destroy'); });

Why the $ctrl is not destroyed?

Obviously if you uncomment the code //$element.remove(); the element go away but remain for me the problem

perhaps this good link http://www.bennadel.com/blog/2883-how-the-destroy-event-affects-the-scope-tree-in-angularjs.htm has answered the question

petebacondarwin · 2016-09-15T09:44:33Z

@szaccaria - so from looking at your first fiddle, I would say that you are not using Angular correctly here. You should not be adding elements dynamically inside your myCanvas controller. Instead I would expose an array of "window" objects in your controller that you render using ng-repeat in your template. Then Angular will take care of creating and destroying scopes for you. http://jsfiddle.net/f6uycp44/

Of course this doesn't quite help with the one you passed in via transclusion but that can be fixed...

bernardoadc · 2016-11-16T14:03:01Z

I'm having a similar problem, but with DOM. Not using routing but ng-include, when I switch between views a component DOM remains from the previous view, even tough the $onDestroy event was called. $element.remove() doesnt do nothing. It seems like the controller was properly destroyed, but not the DOM. Any thoughts on this behavior?

petebacondarwin · 2016-11-16T14:34:25Z

@khullah can you please open a new issue and provide a running example of your problem?

bernardoadc · 2016-11-16T21:43:01Z

Hi, thanks for the reply. So, I build a plunker but the problem wasn't reproduced. Digging it further I found out that the succeeding view also had the same component somewhere, but he wasn't initializing properly, masking the real problem. So not related to this thread, sorry.

Narretz added component: $compile frequency: low severity: broken expected use branch: 1.5.x labels Apr 5, 2016

Narretz added this to the 1.5.x milestone Apr 5, 2016

Narretz changed the title ~~component's controller $onDestroy hook is not called on manual $scope.$destroy~~ component's controller $onDestroy hook is not called on manual $scope.$destroy from inside the controller Apr 5, 2016

petebacondarwin added the resolution: invalid label Apr 5, 2016

gkalpak mentioned this issue Sep 7, 2016

$onDestroy not called on Controller but $scope.$on($destroy) is #15073

Closed

gkalpak closed this as completed Sep 7, 2016

thorn0 mentioned this issue Sep 7, 2016

bindToController should work without controllerAs #15088

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

component's controller $onDestroy hook is not called on manual $scope.$destroy from inside the controller #14376

component's controller $onDestroy hook is not called on manual $scope.$destroy from inside the controller #14376

artaommahe commented Apr 5, 2016

Narretz commented Apr 5, 2016

Narretz commented Apr 5, 2016

petebacondarwin commented Apr 5, 2016

artaommahe commented Apr 5, 2016

artaommahe commented Apr 7, 2016

thorn0 commented Apr 29, 2016

ramunsk commented Jun 15, 2016

petebacondarwin commented Jun 15, 2016

CarlLeth commented Jul 12, 2016

petebacondarwin commented Jul 12, 2016 •

edited

Loading

artaommahe commented Jul 12, 2016 •

edited

Loading

CarlLeth commented Jul 12, 2016 •

edited

Loading

gkalpak commented Jul 12, 2016 •

edited

Loading

gkalpak commented Sep 7, 2016

szaccaria commented Sep 12, 2016

szaccaria commented Sep 13, 2016 •

edited

Loading

petebacondarwin commented Sep 15, 2016

bernardoadc commented Nov 16, 2016

petebacondarwin commented Nov 16, 2016

bernardoadc commented Nov 16, 2016

component's controller $onDestroy hook is not called on manual $scope.$destroy from inside the controller #14376

component's controller $onDestroy hook is not called on manual $scope.$destroy from inside the controller #14376

Comments

artaommahe commented Apr 5, 2016

Narretz commented Apr 5, 2016

Narretz commented Apr 5, 2016

petebacondarwin commented Apr 5, 2016

artaommahe commented Apr 5, 2016

artaommahe commented Apr 7, 2016

thorn0 commented Apr 29, 2016

ramunsk commented Jun 15, 2016

petebacondarwin commented Jun 15, 2016

CarlLeth commented Jul 12, 2016

petebacondarwin commented Jul 12, 2016 • edited Loading

artaommahe commented Jul 12, 2016 • edited Loading

CarlLeth commented Jul 12, 2016 • edited Loading

gkalpak commented Jul 12, 2016 • edited Loading

gkalpak commented Sep 7, 2016

szaccaria commented Sep 12, 2016

szaccaria commented Sep 13, 2016 • edited Loading

petebacondarwin commented Sep 15, 2016

bernardoadc commented Nov 16, 2016

petebacondarwin commented Nov 16, 2016

bernardoadc commented Nov 16, 2016

petebacondarwin commented Jul 12, 2016 •

edited

Loading

artaommahe commented Jul 12, 2016 •

edited

Loading

CarlLeth commented Jul 12, 2016 •

edited

Loading

gkalpak commented Jul 12, 2016 •

edited

Loading

szaccaria commented Sep 13, 2016 •

edited

Loading