Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit bb808d1

Browse files
committed
refactor($compile): remove skipDestroyOnNextJQueryCleanData, remove jq data of all replaced nodes
1 parent 1d41f6f commit bb808d1

File tree

5 files changed

+32
-37
lines changed

5 files changed

+32
-37
lines changed

src/.jshintrc

-2
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,6 @@
9393
"VALIDITY_STATE_PROPERTY": false,
9494
"reloadWithDebugInfo": false,
9595

96-
"skipDestroyOnNextJQueryCleanData": true,
97-
9896
"NODE_TYPE_ELEMENT": false,
9997
"NODE_TYPE_ATTRIBUTE": false,
10098
"NODE_TYPE_TEXT": false,

src/Angular.js

+4-9
Original file line numberDiff line numberDiff line change
@@ -1627,7 +1627,6 @@ function snake_case(name, separator) {
16271627
}
16281628

16291629
var bindJQueryFired = false;
1630-
var skipDestroyOnNextJQueryCleanData;
16311630
function bindJQuery() {
16321631
var originalCleanData;
16331632

@@ -1662,15 +1661,11 @@ function bindJQuery() {
16621661
originalCleanData = jQuery.cleanData;
16631662
jQuery.cleanData = function(elems) {
16641663
var events;
1665-
if (!skipDestroyOnNextJQueryCleanData) {
1666-
for (var i = 0, elem; (elem = elems[i]) != null; i++) {
1667-
events = jQuery._data(elem, "events");
1668-
if (events && events.$destroy) {
1669-
jQuery(elem).triggerHandler('$destroy');
1670-
}
1664+
for (var i = 0, elem; (elem = elems[i]) != null; i++) {
1665+
events = jQuery._data(elem, "events");
1666+
if (events && events.$destroy) {
1667+
jQuery(elem).triggerHandler('$destroy');
16711668
}
1672-
} else {
1673-
skipDestroyOnNextJQueryCleanData = false;
16741669
}
16751670
originalCleanData(elems);
16761671
};

src/jqLite.js

+8-1
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,12 @@ function jqLiteHasData(node) {
189189
return false;
190190
}
191191

192+
function jqLiteCleanData(nodes) {
193+
for (var i = 0, ii = nodes.length; i < ii; i++) {
194+
jqLiteRemoveData(nodes[i]);
195+
}
196+
}
197+
192198
function jqLiteBuildFragment(html, context) {
193199
var tmp, tag, wrap,
194200
fragment = context.createDocumentFragment(),
@@ -564,7 +570,8 @@ function getAliasedAttrName(element, name) {
564570
forEach({
565571
data: jqLiteData,
566572
removeData: jqLiteRemoveData,
567-
hasData: jqLiteHasData
573+
hasData: jqLiteHasData,
574+
cleanData: jqLiteCleanData
568575
}, function(fn, name) {
569576
JQLite[name] = fn;
570577
});

src/ng/compile.js

+17-24
Original file line numberDiff line numberDiff line change
@@ -2497,41 +2497,34 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
24972497
parent.replaceChild(newNode, firstElementToRemove);
24982498
}
24992499

2500-
// TODO(perf): what's this document fragment for? is it needed? can we at least reuse it?
2500+
// Append all the `elementsToRemove` to a fragment. This will...
2501+
// - remove them from the DOM
2502+
// - allow them to still be traversed with .nextSibling
2503+
// - allow a single fragment.qSA to fetch all elements being removed
25012504
var fragment = document.createDocumentFragment();
2502-
fragment.appendChild(firstElementToRemove);
2505+
for (i = 0; i < removeCount; i++) {
2506+
fragment.appendChild(elementsToRemove[i]);
2507+
}
25032508

25042509
if (jqLite.hasData(firstElementToRemove)) {
25052510
// Copy over user data (that includes Angular's $scope etc.). Don't copy private
25062511
// data here because there's no public interface in jQuery to do that and copying over
25072512
// event listeners (which is the main use of private data) wouldn't work anyway.
25082513
jqLite(newNode).data(jqLite(firstElementToRemove).data());
25092514

2510-
// Remove data of the replaced element. We cannot just call .remove()
2511-
// on the element it since that would deallocate scope that is needed
2512-
// for the new node. Instead, remove the data "manually".
2513-
if (!jQuery) {
2514-
delete jqLite.cache[firstElementToRemove[jqLite.expando]];
2515-
} else {
2516-
// jQuery 2.x doesn't expose the data storage. Use jQuery.cleanData to clean up after
2517-
// the replaced element. The cleanData version monkey-patched by Angular would cause
2518-
// the scope to be trashed and we do need the very same scope to work with the new
2519-
// element. However, we cannot just cache the non-patched version and use it here as
2520-
// that would break if another library patches the method after Angular does (one
2521-
// example is jQuery UI). Instead, set a flag indicating scope destroying should be
2522-
// skipped this one time.
2523-
skipDestroyOnNextJQueryCleanData = true;
2524-
jQuery.cleanData([firstElementToRemove]);
2525-
}
2515+
// Remove $destroy event listeners from `firstElementToRemove` since it is being
2516+
// replaced, not destroyed.
2517+
jqLite(firstElementToRemove).off('$destroy');
25262518
}
25272519

2528-
for (var k = 1, kk = elementsToRemove.length; k < kk; k++) {
2529-
var element = elementsToRemove[k];
2530-
jqLite(element).remove(); // must do this way to clean up expando
2531-
fragment.appendChild(element);
2532-
delete elementsToRemove[k];
2533-
}
2520+
// Cleanup any data/listeners on the elements and children.
2521+
// This includes invoking the $destroy event on any elements with listeners.
2522+
jqLite.cleanData(fragment.querySelectorAll('*'));
25342523

2524+
// Update the jqLite collection to only contain the `newNode`
2525+
for (i = 1; i < removeCount; i++) {
2526+
delete elementsToRemove[i];
2527+
}
25352528
elementsToRemove[0] = newNode;
25362529
elementsToRemove.length = 1;
25372530
}

test/ng/compileSpec.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -5619,7 +5619,9 @@ describe('$compile', function() {
56195619

56205620
testCleanup();
56215621

5622-
expect(cleanedCount).toBe(xs.length);
5622+
// The ng-repeat template is removed/cleaned (the +1)
5623+
// and each clone of the ng-repeat template is also removed (xs.length)
5624+
expect(cleanedCount).toBe(xs.length + 1);
56235625

56245626
// Restore the previous jQuery.cleanData.
56255627
jQuery.cleanData = currentCleanData;

0 commit comments

Comments
 (0)