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

This change aligns jqLite with jQuery.

Fixes #15869
Closes #16512

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:

```js
elem.removeData();
```

to:

```js
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:

```js
elem.remove();
```

will remove event handlers as well.
This commit is contained in:
Michał Gołębiowski-Owczarek
2018-04-03 21:49:52 +02:00
committed by GitHub
parent 71d2c147a8
commit b7d396b8b6
3 changed files with 124 additions and 24 deletions
+15 -15
View File
@@ -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.
+28 -9
View File
@@ -311,6 +311,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');
@@ -347,6 +369,8 @@ function jqLiteOff(element, type, fn, unsupported) {
}
});
}
removeIfEmptyData(element);
}
function jqLiteRemoveData(element, name) {
@@ -356,17 +380,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);
}
}
@@ -616,6 +634,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) {
+81
View File
@@ -420,6 +420,87 @@ 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 allow to set data after removeData() without event handlers present', function() {
var elm = jqLite(a);
elm.data('key1', 'value1');
elm.removeData();
elm.data('key2', 'value2');
expect(elm.data('key1')).not.toBeDefined();
expect(elm.data('key2')).toBe('value2');
});
it('should remove user data on cleanData()', function() {
var selected = jqLite([a, b, c]);
selected.data('prop', 'value');
jqLite(b).data('prop', 'new value');
jqLite.cleanData(selected);
expect(jqLite(a).data('prop')).toBeUndefined();
expect(jqLite(b).data('prop')).toBeUndefined();
expect(jqLite(c).data('prop')).toBeUndefined();
});
it('should remove event handlers on cleanData()', function() {
var selected = jqLite([a, b, c]);
var log = '';
var elm = jqLite(b);
elm.on('click', function() {
log += 'click;';
});
jqLite.cleanData(selected);
browserTrigger(b, 'click');
expect(log).toBe('');
});
it('should remove user data & event handlers on cleanData()', function() {
var selected = jqLite([a, b, c]);
var log = '';
var elm = jqLite(b);
elm.on('click', function() {
log += 'click;';
});
selected.data('prop', 'value');
jqLite(a).data('prop', 'new value');
jqLite.cleanData(selected);
browserTrigger(b, 'click');
expect(log).toBe('');
expect(jqLite(a).data('prop')).toBeUndefined();
expect(jqLite(b).data('prop')).toBeUndefined();
expect(jqLite(c).data('prop')).toBeUndefined();
});
it('should add and remove data on SVGs', function() {
var svg = jqLite('<svg><rect></rect></svg>');