Skip to content

Commit 24d729f

Browse files
committed
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: ```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 feee36c commit 24d729f

File tree

3 files changed

+124
-24
lines changed

3 files changed

+124
-24
lines changed

Diff for: 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.

Diff for: src/jqLite.js

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

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

@@ -346,6 +368,8 @@ function jqLiteOff(element, type, fn, unsupported) {
346368
}
347369
});
348370
}
371+
372+
removeIfEmptyData(element);
349373
}
350374

351375
function jqLiteRemoveData(element, name) {
@@ -355,17 +379,11 @@ function jqLiteRemoveData(element, name) {
355379
if (expandoStore) {
356380
if (name) {
357381
delete expandoStore.data[name];
358-
return;
382+
} else {
383+
expandoStore.data = {};
359384
}
360385

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

@@ -615,6 +633,7 @@ forEach({
615633
cleanData: function jqLiteCleanData(nodes) {
616634
for (var i = 0, ii = nodes.length; i < ii; i++) {
617635
jqLiteRemoveData(nodes[i]);
636+
jqLiteOff(nodes[i]);
618637
}
619638
}
620639
}, function(fn, name) {

Diff for: 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(a).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+
477+
selected.data('prop', 'value');
478+
jqLite(a).data('prop', 'new value');
479+
480+
jqLite.cleanData(selected);
481+
482+
browserTrigger(a, 'click');
483+
expect(log).toBe('');
484+
485+
expect(jqLite(a).data('prop')).toBeUndefined();
486+
expect(jqLite(b).data('prop')).toBeUndefined();
487+
expect(jqLite(c).data('prop')).toBeUndefined();
488+
});
489+
490+
it('should remove user data & event handlers on cleanData()', function() {
491+
var selected = jqLite([a, b, c]);
492+
493+
var log = '';
494+
var elm = jqLite(b);
495+
elm.on('click', function() {
496+
log += 'click;';
497+
});
498+
jqLite.cleanData(selected);
499+
500+
browserTrigger(a, 'click');
501+
expect(log).toBe('');
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)