fix(jqLite): make removeData() not remove event handlers

This change aligns jqLite with jQuery.

Fixes angular#15869

BREAKING CHANGE:

Before this commit removeData() invoked on an element removed its event
handlers as well. If you want to trigger a full cleanup of an element, change:

  elem.removeData();

to:

  angular.element.cleanData([elem]);

In most cases, though, cleaning up after an element is supposed to be done
only when it's removed from the DOM as well; in such cases the following:

  elem.remove();

will remove event handlers as well.

Author: mgol

Diff for: src/Angular.js

@@ -1924,25 +1924,25 @@ function bindJQuery() {
       injector: JQLitePrototype.injector,
       inheritedData: JQLitePrototype.inheritedData
     });
-
-    // All nodes removed from the DOM via various jQuery APIs like .remove()
-    // are passed through jQuery.cleanData. Monkey-patch this method to fire
-    // the $destroy event on all removed nodes.
-    originalCleanData = jQuery.cleanData;
-    jQuery.cleanData = function(elems) {
-      var events;
-      for (var i = 0, elem; (elem = elems[i]) != null; i++) {
-        events = jQuery._data(elem, 'events');
-        if (events && events.$destroy) {
-          jQuery(elem).triggerHandler('$destroy');
-        }
-      }
-      originalCleanData(elems);
-    };
   } else {
     jqLite = JQLite;
   }
 
+  // All nodes removed from the DOM via various jqLite/jQuery APIs like .remove()
+  // are passed through jqLite/jQuery.cleanData. Monkey-patch this method to fire
+  // the $destroy event on all removed nodes.
+  originalCleanData = jqLite.cleanData;
+  jqLite.cleanData = function(elems) {
+    var events;
+    for (var i = 0, elem; (elem = elems[i]) != null; i++) {
+      events = jqLite._data(elem).events;
+      if (events && events.$destroy) {
+        jqLite(elem).triggerHandler('$destroy');
+      }
+    }
+    originalCleanData(elems);
+  };
+
   angular.element = jqLite;
 
   // Prevent double-proxying.

Diff for: src/jqLite.js

@@ -310,6 +310,28 @@ function jqLiteDealoc(element, onlyDescendants) {
   }
 }
 
+function isEmptyObject(obj) {
+  var name;
+
+  for (name in obj) {
+    return false;
+  }
+  return true;
+}
+
+function removeIfEmptyData(element) {
+  var expandoId = element.ng339;
+  var expandoStore = expandoId && jqCache[expandoId];
+
+  var events = expandoStore && expandoStore.events;
+  var data = expandoStore && expandoStore.data;
+
+  if ((!data || isEmptyObject(data)) && (!events || isEmptyObject(events))) {
+    delete jqCache[expandoId];
+    element.ng339 = undefined; // don't delete DOM expandos. IE and Chrome don't like it
+  }
+}
+
 function jqLiteOff(element, type, fn, unsupported) {
   if (isDefined(unsupported)) throw jqLiteMinErr('offargs', 'jqLite#off() does not support the `selector` argument');
 
@@ -346,6 +368,8 @@ function jqLiteOff(element, type, fn, unsupported) {
       }
     });
   }
+
+  removeIfEmptyData(element);
 }
 
 function jqLiteRemoveData(element, name) {
@@ -355,17 +379,11 @@ function jqLiteRemoveData(element, name) {
   if (expandoStore) {
     if (name) {
       delete expandoStore.data[name];
-      return;
+    } else {
+      expandoStore.data = {};
     }
 
-    if (expandoStore.handle) {
-      if (expandoStore.events.$destroy) {
-        expandoStore.handle({}, '$destroy');
-      }
-      jqLiteOff(element);
-    }
-    delete jqCache[expandoId];
-    element.ng339 = undefined; // don't delete DOM expandos. IE and Chrome don't like it
+    removeIfEmptyData(element);
   }
 }
 
@@ -615,6 +633,7 @@ forEach({
   cleanData: function jqLiteCleanData(nodes) {
     for (var i = 0, ii = nodes.length; i < ii; i++) {
       jqLiteRemoveData(nodes[i]);
+      jqLiteOff(nodes[i]);
     }
   }
 }, function(fn, name) {

Diff for: test/jqLiteSpec.js

@@ -420,6 +420,29 @@ describe('jqLite', function() {
       selected.removeData('prop2');
     });
 
+    it('should not remove event handlers on removeData()', function() {
+      var log = '';
+      var elm = jqLite(a);
+      elm.on('click', function() {
+        log += 'click;';
+      });
+
+      elm.removeData();
+      browserTrigger(a, 'click');
+      expect(log).toBe('click;');
+    });
+
+
+    it('should allow to set data after removeData() with event handlers present', function() {
+      var elm = jqLite(a);
+      elm.on('click', function() {});
+      elm.data('key1', 'value1');
+      elm.removeData();
+      elm.data('key2', 'value2');
+      expect(elm.data('key1')).not.toBeDefined();
+      expect(elm.data('key2')).toBe('value2');
+    });
+
 
     it('should add and remove data on SVGs', function() {
       var svg = jqLite('<svg><rect></rect></svg>');