Skip to content

Commit ff0d5a7

Browse files
committed
fix(jQuery): cooperate with other libraries monkey-patching jQuery.cleanData
Some libraries (like jQuery UI) patch jQuery.cleanData as well. This commit makes Angular work correctly even if such external patching was done after the Angular one. Fixes angular#8471
1 parent 60c13a1 commit ff0d5a7

File tree

4 files changed

+75
-32
lines changed

4 files changed

+75
-32
lines changed

src/Angular.js

+20-12
Original file line numberDiff line numberDiff line change
@@ -1454,8 +1454,12 @@ function snake_case(name, separator) {
14541454
});
14551455
}
14561456

1457+
var bindJQueryFired = false;
14571458
function bindJQuery() {
1458-
var originalCleanData;
1459+
if (bindJQueryFired) {
1460+
return;
1461+
}
1462+
14591463
// bind to jQuery if present;
14601464
jQuery = window.jQuery;
14611465
// Use jQuery if it exists with proper functionality, otherwise default to us.
@@ -1472,25 +1476,29 @@ function bindJQuery() {
14721476
inheritedData: JQLitePrototype.inheritedData
14731477
});
14741478

1475-
originalCleanData = jQuery.cleanData;
1476-
// Prevent double-proxying.
1477-
originalCleanData = originalCleanData.$$original || originalCleanData;
1478-
14791479
// All nodes removed from the DOM via various jQuery APIs like .remove()
14801480
// are passed through jQuery.cleanData. Monkey-patch this method to fire
14811481
// the $destroy event on all removed nodes.
1482-
jQuery.cleanData = function(elems) {
1483-
for (var i = 0, elem; (elem = elems[i]) != null; i++) {
1484-
jQuery(elem).triggerHandler('$destroy');
1485-
}
1486-
originalCleanData(elems);
1487-
};
1488-
jQuery.cleanData.$$original = originalCleanData;
1482+
jQuery.cleanData = (function (originalCleanData) {
1483+
return function(elems) {
1484+
if (!angular.$$skipDestroy) {
1485+
for (var i = 0, elem; (elem = elems[i]) != null; i++) {
1486+
jQuery(elem).triggerHandler('$destroy');
1487+
}
1488+
} else {
1489+
angular.$$skipDestroy = false;
1490+
}
1491+
originalCleanData(elems);
1492+
};
1493+
})(jQuery.cleanData);
14891494
} else {
14901495
jqLite = JQLite;
14911496
}
14921497

14931498
angular.element = jqLite;
1499+
1500+
// Prevent double-proxying.
1501+
bindJQueryFired = true;
14941502
}
14951503

14961504
/**

src/ng/compile.js

+9-5
Original file line numberDiff line numberDiff line change
@@ -2043,11 +2043,15 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
20432043
if (!jQuery) {
20442044
delete jqLite.cache[firstElementToRemove[jqLite.expando]];
20452045
} else {
2046-
// jQuery 2.x doesn't expose the data storage. Use jQuery.cleanData to clean up after the replaced
2047-
// element. Note that we need to use the original method here and not the one monkey-patched by Angular
2048-
// since the patched method emits the $destroy event causing the scope to be trashed and we do need
2049-
// the very same scope to work with the new element.
2050-
jQuery.cleanData.$$original([firstElementToRemove]);
2046+
// jQuery 2.x doesn't expose the data storage. Use jQuery.cleanData to clean up after
2047+
// the replaced element. The cleanData version monkey-patched by Angular would cause
2048+
// the scope to be trashed and we do need the very same scope to work with the new
2049+
// element. However, we cannot just cache the non-patched version and use it here as
2050+
// that would break if another library patches the method after Angular does (one
2051+
// example is jQuery UI). Instead, set a flag indicating scope destroying should be
2052+
// skipped this one time.
2053+
angular.$$skipDestroy = true;
2054+
jQuery.cleanData([firstElementToRemove]);
20512055
}
20522056

20532057
for (var k = 1, kk = elementsToRemove.length; k < kk; k++) {

test/helpers/testabilityPatch.js

+5
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,11 @@ function dealoc(obj) {
112112

113113
function cleanup(element) {
114114
element.off().removeData();
115+
if (window.jQuery) {
116+
// jQuery 2.x doesn't expose the cache storage; ensure all element data
117+
// is removed during its cleanup.
118+
jQuery.cleanData([element]);
119+
}
115120
// Note: We aren't using element.contents() here. Under jQuery, element.contents() can fail
116121
// for IFRAME elements. jQuery explicitly uses (element.contentDocument ||
117122
// element.contentWindow.document) and both properties are null for IFRAMES that aren't attached

test/ng/compileSpec.js

+41-15
Original file line numberDiff line numberDiff line change
@@ -4096,26 +4096,52 @@ describe('$compile', function() {
40964096
});
40974097

40984098
if (jQuery) {
4099-
it('should clean up after a replaced element', inject(function ($compile) {
4100-
var privateData, firstRepeatedElem;
4099+
describe('cleaning up after a replaced element', function () {
4100+
var $compile;
4101+
beforeEach(inject(function (_$compile_) {
4102+
$compile = _$compile_;
4103+
}));
41014104

4102-
element = $compile('<div><div ng-repeat="x in xs">{{x}}</div></div>')($rootScope);
4105+
function testCleanup() {
4106+
var privateData, firstRepeatedElem;
41034107

4104-
$rootScope.$apply('xs = [0,1]');
4105-
firstRepeatedElem = element.children('.ng-scope').eq(0);
4108+
element = $compile('<div><div ng-repeat="x in xs">{{x}}</div></div>')($rootScope);
41064109

4107-
expect(firstRepeatedElem.data('$scope')).toBeDefined();
4108-
privateData = jQuery._data(firstRepeatedElem[0]);
4109-
expect(privateData.events).toBeDefined();
4110-
expect(privateData.events.$destroy).toBeDefined();
4111-
expect(privateData.events.$destroy[0]).toBeDefined();
4110+
$rootScope.$apply('xs = [0,1]');
4111+
firstRepeatedElem = element.children('.ng-scope').eq(0);
41124112

4113-
$rootScope.$apply('xs = null');
4113+
expect(firstRepeatedElem.data('$scope')).toBeDefined();
4114+
privateData = jQuery._data(firstRepeatedElem[0]);
4115+
expect(privateData.events).toBeDefined();
4116+
expect(privateData.events.$destroy).toBeDefined();
4117+
expect(privateData.events.$destroy[0]).toBeDefined();
41144118

4115-
expect(firstRepeatedElem.data('$scope')).not.toBeDefined();
4116-
privateData = jQuery._data(firstRepeatedElem[0]);
4117-
expect(privateData && privateData.events).not.toBeDefined();
4118-
}));
4119+
$rootScope.$apply('xs = null');
4120+
4121+
expect(firstRepeatedElem.data('$scope')).not.toBeDefined();
4122+
privateData = jQuery._data(firstRepeatedElem[0]);
4123+
expect(privateData && privateData.events).not.toBeDefined();
4124+
}
4125+
4126+
it('should work without external libraries (except jQuery)', testCleanup);
4127+
4128+
it('should work with another library patching jQuery.cleanData after Angular', function () {
4129+
var cleanedCount = 0;
4130+
var currentCleanData = jQuery.cleanData;
4131+
jQuery.cleanData = function (elems) {
4132+
cleanedCount += elems.length;
4133+
// Don't return the output and expicitly pass only the first parameter
4134+
// so that we're sure we're not relying on either of them. jQuery UI patch
4135+
// behaves in this way.
4136+
currentCleanData(elems);
4137+
};
4138+
4139+
testCleanup();
4140+
4141+
// Restore the previous jQuery.cleanData.
4142+
jQuery.cleanData = currentCleanData;
4143+
});
4144+
});
41194145
}
41204146

41214147

0 commit comments

Comments
 (0)