Skip to content

Commit b7d396b

Browse files
authored
fix(jqLite): make removeData() not remove event handlers
This change aligns jqLite with jQuery. Fixes angular#15869 Closes angular#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.
1 parent 71d2c14 commit b7d396b

File tree

3 files changed

+124
-24
lines changed

3 files changed

+124
-24
lines changed

src/Angular.js

+15-15
Original file line numberDiff line numberDiff line change
@@ -1924,25 +1924,25 @@ function bindJQuery() {
19241924
injector: JQLitePrototype.injector,
19251925
inheritedData: JQLitePrototype.inheritedData
19261926
});
1927-
1928-
// All nodes removed from the DOM via various jQuery APIs like .remove()
1929-
// are passed through jQuery.cleanData. Monkey-patch this method to fire
1930-
// the $destroy event on all removed nodes.
1931-
originalCleanData = jQuery.cleanData;
1932-
jQuery.cleanData = function(elems) {
1933-
var events;
1934-
for (var i = 0, elem; (elem = elems[i]) != null; i++) {
1935-
events = jQuery._data(elem, 'events');
1936-
if (events && events.$destroy) {
1937-
jQuery(elem).triggerHandler('$destroy');
1938-
}
1939-
}
1940-
originalCleanData(elems);
1941-
};
19421927
} else {
19431928
jqLite = JQLite;
19441929
}
19451930

1931+
// All nodes removed from the DOM via various jqLite/jQuery APIs like .remove()
1932+
// are passed through jqLite/jQuery.cleanData. Monkey-patch this method to fire
1933+
// the $destroy event on all removed nodes.
1934+
originalCleanData = jqLite.cleanData;
1935+
jqLite.cleanData = function(elems) {
1936+
var events;
1937+
for (var i = 0, elem; (elem = elems[i]) != null; i++) {
1938+
events = jqLite._data(elem).events;
1939+
if (events && events.$destroy) {
1940+
jqLite(elem).triggerHandler('$destroy');
1941+
}
1942+
}
1943+
originalCleanData(elems);
1944+
};
1945+
19461946
angular.element = jqLite;
19471947

19481948
// Prevent double-proxying.

src/jqLite.js

+28-9
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,28 @@ function jqLiteDealoc(element, onlyDescendants) {
311311
}
312312
}
313313

314+
function isEmptyObject(obj) {
315+
var name;
316+
317+
for (name in obj) {
318+
return false;
319+
}
320+
return true;
321+
}
322+
323+
function removeIfEmptyData(element) {
324+
var expandoId = element.ng339;
325+
var expandoStore = expandoId && jqCache[expandoId];
326+
327+
var events = expandoStore && expandoStore.events;
328+
var data = expandoStore && expandoStore.data;
329+
330+
if ((!data || isEmptyObject(data)) && (!events || isEmptyObject(events))) {
331+
delete jqCache[expandoId];
332+
element.ng339 = undefined; // don't delete DOM expandos. IE and Chrome don't like it
333+
}
334+
}
335+
314336
function jqLiteOff(element, type, fn, unsupported) {
315337
if (isDefined(unsupported)) throw jqLiteMinErr('offargs', 'jqLite#off() does not support the `selector` argument');
316338

@@ -347,6 +369,8 @@ function jqLiteOff(element, type, fn, unsupported) {
347369
}
348370
});
349371
}
372+
373+
removeIfEmptyData(element);
350374
}
351375

352376
function jqLiteRemoveData(element, name) {
@@ -356,17 +380,11 @@ function jqLiteRemoveData(element, name) {
356380
if (expandoStore) {
357381
if (name) {
358382
delete expandoStore.data[name];
359-
return;
383+
} else {
384+
expandoStore.data = {};
360385
}
361386

362-
if (expandoStore.handle) {
363-
if (expandoStore.events.$destroy) {
364-
expandoStore.handle({}, '$destroy');
365-
}
366-
jqLiteOff(element);
367-
}
368-
delete jqCache[expandoId];
369-
element.ng339 = undefined; // don't delete DOM expandos. IE and Chrome don't like it
387+
removeIfEmptyData(element);
370388
}
371389
}
372390

@@ -616,6 +634,7 @@ forEach({
616634
cleanData: function jqLiteCleanData(nodes) {
617635
for (var i = 0, ii = nodes.length; i < ii; i++) {
618636
jqLiteRemoveData(nodes[i]);
637+
jqLiteOff(nodes[i]);
619638
}
620639
}
621640
}, function(fn, name) {

test/jqLiteSpec.js

+81
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,87 @@ describe('jqLite', function() {
420420
selected.removeData('prop2');
421421
});
422422

423+
it('should not remove event handlers on removeData()', function() {
424+
var log = '';
425+
var elm = jqLite(a);
426+
elm.on('click', function() {
427+
log += 'click;';
428+
});
429+
430+
elm.removeData();
431+
browserTrigger(a, 'click');
432+
expect(log).toBe('click;');
433+
});
434+
435+
it('should allow to set data after removeData() with event handlers present', function() {
436+
var elm = jqLite(a);
437+
elm.on('click', function() {});
438+
elm.data('key1', 'value1');
439+
elm.removeData();
440+
elm.data('key2', 'value2');
441+
expect(elm.data('key1')).not.toBeDefined();
442+
expect(elm.data('key2')).toBe('value2');
443+
});
444+
445+
it('should allow to set data after removeData() without event handlers present', function() {
446+
var elm = jqLite(a);
447+
elm.data('key1', 'value1');
448+
elm.removeData();
449+
elm.data('key2', 'value2');
450+
expect(elm.data('key1')).not.toBeDefined();
451+
expect(elm.data('key2')).toBe('value2');
452+
});
453+
454+
455+
it('should remove user data on cleanData()', function() {
456+
var selected = jqLite([a, b, c]);
457+
458+
selected.data('prop', 'value');
459+
jqLite(b).data('prop', 'new value');
460+
461+
jqLite.cleanData(selected);
462+
463+
expect(jqLite(a).data('prop')).toBeUndefined();
464+
expect(jqLite(b).data('prop')).toBeUndefined();
465+
expect(jqLite(c).data('prop')).toBeUndefined();
466+
});
467+
468+
it('should remove event handlers on cleanData()', function() {
469+
var selected = jqLite([a, b, c]);
470+
471+
var log = '';
472+
var elm = jqLite(b);
473+
elm.on('click', function() {
474+
log += 'click;';
475+
});
476+
jqLite.cleanData(selected);
477+
478+
browserTrigger(b, 'click');
479+
expect(log).toBe('');
480+
});
481+
482+
it('should remove user data & event handlers on cleanData()', function() {
483+
var selected = jqLite([a, b, c]);
484+
485+
var log = '';
486+
var elm = jqLite(b);
487+
elm.on('click', function() {
488+
log += 'click;';
489+
});
490+
491+
selected.data('prop', 'value');
492+
jqLite(a).data('prop', 'new value');
493+
494+
jqLite.cleanData(selected);
495+
496+
browserTrigger(b, 'click');
497+
expect(log).toBe('');
498+
499+
expect(jqLite(a).data('prop')).toBeUndefined();
500+
expect(jqLite(b).data('prop')).toBeUndefined();
501+
expect(jqLite(c).data('prop')).toBeUndefined();
502+
});
503+
423504

424505
it('should add and remove data on SVGs', function() {
425506
var svg = jqLite('<svg><rect></rect></svg>');

0 commit comments

Comments
 (0)