How to safely clean up AngularJS event binding in a directive

Question

I have an Angular directive that sets an element's height equal to the inner height of the browser window (+/- a given offset). This directive responds to the window's "resize" event and adjusts its height accordingly. When the scope of my directive emits the '$destory' event, I remove the binding to the "resize" event (I think leaving it in place would cause some issues, correct me if I'm wrong).

I don't know how to do this event detachment in a "safe" way. What if I have multiple instances of this directive throughout my app and what if I have other directives that attach to the 'resize' event?

JQuery has the concept of event namespace which seems like a good solution, but Angular's implementation (JQLite) does not support this. I'd rather not use JQuery since I'm already using Angular, so what do I do?

Here's the code for my directive as it is today

window.angular.module('arcFillClient', [])
    .directive('arcFillClientY', ['$window',
        function ($window) {

            function link($scope, el, attrs) {

                var setHeight,
                    onResize,
                    cleanUp;

                setHeight = function (offSetY) {
                    var newHeight;
                    offSetY = offSetY || 0;
                    newHeight = Math.max($window.innerHeight + parseInt(offSetY, 10)) + 'px';
                    el.height(newHeight);
                };

                onResize = function () {
                    var offset;
                    offset = attrs.arcFillClientY || 0;
                    setHeight(offset);
                };

                attrs.$observe('arcFillClientY', setHeight);
                window.angular.element($window).on('resize', onResize);

                cleanUp = function () {
                    window.angular.element($window).off('resize');
                };

                $scope.$on('$destroy', cleanUp);
            }
            return {
                link: link
            };

UPDATE Looks like a case of RTFM, but just in case anyone else wanders in here, here is some more info. Passing the original function (in my case OnResize) to the .off() works to isolate the scope of the .off() function. From the docs:

A handler can also be removed by specifying the function name in the handler argument. When jQuery {ahem... JQLite} attaches an event handler, it assigns a unique id to the handler function.

Here's the updated cleanUp function from my directive:

cleanUp = function () {
    window.angular.element($window).off('resize', onResize);
};

Thanks tasseKATT, Karolis and Hans for your contributions.

score 30 · Accepted Answer · answered Apr 12 '14 at 18:37

30

Pass the same function reference to off as you pass to on:

window.angular.element($window).off('resize', onResize);

Instead of:

window.angular.element($window).off('resize');

Demo - Passing function reference to off: http://plnkr.co/edit/1rfVPNXl6TrEcuYvzPAj?p=preview

Demo - Not passing function reference to off: http://plnkr.co/edit/IsLqSLAzNcHqDnhMty7Q?p=preview

The demos contain two directives both listening to the window resize event. Use the vertical separator between the code and the preview to trigger the event.

You will notice that if you destroy one the other will keep working when passing function reference to off. If you don't, both will stop working.

answered Apr 12 '14 at 18:37

tasseKATT

38,470
8
84
65

More information on the subject for those interested: http://stackoverflow.com/a/27016855/2887841 – tasseKATT Feb 20 '15 at 06:18
I had a mind-f4ck about this. When you apply the directive on 10 elements, does that mean that the window has 10 resize eventlisteners with onResize function? Or is the onResize function overwritten each time it's set.... – poashoas Apr 11 '15 at 15:02

Hans · Answer 2 · 2014-04-12T15:04:21.710

I had the same question a few weeks ago.

After looking through the jqLite source (https://github.com/angular/angular.js/blob/master/src/jqLite.js), we see that the on method adds the event and the off method removes the event via the jqLiteOff function.

Looking deeper, we see jqLiteRemoveData calls jqLiteOff. jqLiteRemoveData is called by jqLiteDealoc. jqLiteDealoc gets called in a few places: jqLiteEmpty, html, replaceWith, and remove. jqLiteEmpty gets assigned to the element's empty method, which clears the element in jQuery. html, replaceWith and remove are jQuery mimics.

Doing a search on where remove() is called on an element, we see that it is used on most, if not all, DOM manipulation logic. You can see it in ngIf, ngSwitch, ngInclude and ngView.

So I think Angular does handle event listener cleanup, as long as you use jqLite to attach events and call remove() appropriately in your own DOM manipulation logic. Using jQuery to wrap an element would mess up a lot of processes, including event listener cleanup, but I guess you are already fully aware of that since you are using angular.element.

Hans, this is valuable insight, thank you. I've now dug into the source code myself and it seems that, as you mentioned, the event listeners will be cleaned up by Angular whenever I use remove() (either explicitly or implicitly via a built-in directive). However, since my listener is on the $window (or rather a JQLite wrapped version thereof) it will probably never be removed, right? This means that when my element is destroyed I end up with a listener with an undefined handler, don't I? Sorry, if I'm missing something. — Bill Sourour, Apr 12 '14 at 17:30
You should still have a working listener if it wasn't deregistered. The objects referenced through closure by the listener will also never get garbage collected. — thynctank, May 28 '14 at 22:02

score 2 · Answer 3 · answered Apr 12 '14 at 14:20

2

For one, there is absolutely nothing wrong with using JQuery and AngularJS together.

That aside, what I like to do is have a body directive which listens to window.on('resize', ...) and writes the size into $rootScope.windowSize. Then have another directive on the element, to $watch("windowSize", ...) and set to width as needed. (You don't actually have to expose the size in $scope - you can instead use require).

answered Apr 12 '14 at 14:20

Karolis Juodelė

3,708
1
19
32

I didn't mean to imply that there is anything "wrong" with using JQuery and Angular together, just that I would prefer to avoid it for now. I think your solution will work, but it relies on the app and the directive knowing too much about each other. Is there a solution that keeps my directive more self-contained? – Bill Sourour Apr 12 '14 at 14:29
@BillSparks, the app and the directive don't know anything. It's two directives that are related. A different approach could use a service for reference counting, but I suppose JQuery would be most sensible. – Karolis Juodelė Apr 12 '14 at 17:18
sorry I misused the term "app". The gist of what I am saying is I'd like my directive to be well encapsulated so that I can share it. So, I don't want to create additional dependencies. Also, I think your solution just pushes the problem up a level. Sorry, if I'm missing something. – Bill Sourour Apr 12 '14 at 17:36
@BillSparks, if you use two directives, you only need to listen once - the 'top' directive can be shared by multiple 'bottom' directives. And if you listen once, you also clean up once. Though. Does `off` in jqLite actually not take the the handler argument? The docs only say *"Does not support namespaces or selectors"*... What if you did `.off('resize', onResize)`? – Karolis Juodelė Apr 12 '14 at 18:34
Karolis, actually your last comment js the key. Passing in the original method to '.off' will accomplish what I want. Thanks! – Bill Sourour Apr 12 '14 at 20:46

How to safely clean up AngularJS event binding in a directive

3 Answers3

Linked