Memory leak when hotTable's parent scope is destroyed.

[Ruffle]

Hi, I often use the directive inside controllers whoose scope is later destroyed. I'm not an expert but it seems that, in this case, the elements are not properly removed from the DOM which causes a memory leak. I discovered that, in the vanilla handsontable code, you can call the .destroy() method on a hot instance to remove it from the DOM so I added element.on("$destroy", function() { scope.hotInstance.destroy(); }); at the end of hotTable's "compile" (line 584), which fixed the memory leak.

I don't know if this could break other parts of your code but I just wanted to let you know there's potential problem here.

[Rex90]

I'm also having serious leakes because i destroy my controllers holding this directive. i tried adding what you said but I get this error when i toggled routes (in ui router)

angular.js:13424 Error: This method cannot be called because this Handsontable instance has been destroyed
    at Core.postMortem (handsontable.full.js:5962)
    at Object.updateHandsontableSettings (ngHandsontable.js:157)
    at removeColumnSetting (ngHandsontable.js:475)
    at ngHandsontable.js:440
    at Scope.$broadcast (angular.js:17348)
    at Scope.$destroy (angular.js:16966)
    at cleanOld (angular-ui-router.js:3978)
    at angular-ui-router.js:3984
    at processQueue (angular.js:15757)
    at angular.js:15773

[terite]

👍 this directive needs to clean up after itself using the element.on("$destroy" method shown in the ticket.

Right now we have to work around that by manually destroying hotInstance using the hotRegisterer

[Ruffle]

@armensg I tested the line of code I previously mentioned and I got a similar error. I don't remember under what circumstances it worked (just got back from holidays ^^) but there are other ways to solve this.

Just as terite pointed out you have to destroy the hotInstance manually using hotRegisterer. I searched my code and It looks like what I ended up doing was creating another directive which you can add to <hot-table> as an attribute:

angular.module('app').directive('hotAutoDestroy',["hotRegisterer",function(hotRegisterer) {
    return {
        restrict: 'A',
        link: function (scope, element, attr){
            element.on("$destroy", function() {
                var hotInstance = hotRegisterer.getInstance(attr.hotId);
                hotInstance.destroy();
            });
        }
    }
}]);

then:

<hot-table hot-auto-destroy>

[Rex90]

@Ruffle0 actually I am not sure its working. I still have performance drop after and many events not getting removed. I modified the directive a bit to remove the post-mortem message.

angular.module('dealer.directives').directive('hotAutoDestroy',["hotRegisterer",function(hotRegisterer) {
    return {
        restrict: 'A',
        link: function (scope, element, attr){
            element.on("$destroy", function() {
                try{
                    console.debug("Going to destroy hot instance");
                    var hotInstance = hotRegisterer.getInstance(attr.hotId);
                    if(!isNaN(hotInstance)){
                       hotInstance.destroy(); 
                    }
                    else{
                        // this is when post mortem message is thrown.
                        console.log("Failed to destroy hot-instance");
                    }
                }
                catch(er){
                    console.log(er);
                }
            });
        }
    };
}]);

[Rex90]

would be useful if there as a function to remove event listeners from the page - since the destrory function is failing to do this.

[wtfdaemon]

Did this ever get resolved? We're seeing something similar and it's keeping us from using handsontable. Shouldn't destroy() remove the event listeners explicitly?