Skip to content

Commit

Permalink
feat(jQuery): upgrade to jQuery to 2.1.1
Browse files Browse the repository at this point in the history
The data jQuery method was re-implemented in 2.0 in a secure way. This made
current hacky Angular solution to move data between elements via changing the
value of the internal node[jQuery.expando] stop working. Instead, just copy the
data from the first element to the other one.

Testing cache leaks on jQuery 2.x is not possible in the same way as it's done
in jqLite or in jQuery 1.x as there is no publicly exposed data storage. One
way to test it would be to intercept all places where a jQuery object is created
to save a reference to the underlaying node but there is no single place in the
jQuery code through which all element creation passes (there are various
shortcuts for performance reasons). Instead we rely on jqLite.cache testing
to find potential data leaks.

BREAKING CHANGE: Angular no longer supports jQuery versions below 2.1.1.
  • Loading branch information
mgol committed Jul 31, 2014
1 parent 38db2d4 commit 9e7cb3c
Show file tree
Hide file tree
Showing 13 changed files with 102 additions and 38 deletions.
2 changes: 1 addition & 1 deletion Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ module.exports = function(grunt) {
scenario: {
dest: 'build/angular-scenario.js',
src: [
'bower_components/jquery/jquery.js',
'bower_components/jquery/dist/jquery.js',
util.wrap([files['angularSrc'], files['angularScenario']], 'ngScenario/angular')
],
styles: {
Expand Down
4 changes: 2 additions & 2 deletions angularFiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ var angularFiles = {
],

'karma': [
'bower_components/jquery/jquery.js',
'bower_components/jquery/dist/jquery.js',
'test/jquery_remove.js',
'@angularSrc',
'src/publishExternalApis.js',
Expand Down Expand Up @@ -177,7 +177,7 @@ var angularFiles = {
],

'karmaJquery': [
'bower_components/jquery/jquery.js',
'bower_components/jquery/dist/jquery.js',
'test/jquery_alias.js',
'@angularSrc',
'src/publishExternalApis.js',
Expand Down
2 changes: 1 addition & 1 deletion bower.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "AngularJS",
"devDependencies": {
"jquery": "1.10.2",
"jquery": "2.1.1",
"lunr.js": "0.4.3",
"open-sans-fontface": "1.0.4",
"google-code-prettify": "1.0.1",
Expand Down
5 changes: 3 additions & 2 deletions docs/content/misc/faq.ngdoc
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,15 @@ The size of the file is < 36KB compressed and minified.
Yes, you can use widgets from the [Closure Library](https://developers.google.com/closure/library/)
in Angular.


### Does Angular use the jQuery library?

Yes, Angular can use [jQuery](http://jquery.com/) if it's present in your app when the
application is being bootstrapped. If jQuery is not present in your script path, Angular falls back
to its own implementation of the subset of jQuery that we call {@link angular.element jQLite}.

Due to a change to use `on()`/`off()` rather than `bind()`/`unbind()`, Angular 1.2 only operates with
jQuery 1.7.1 or above. However, Angular does not currently support jQuery 2.x or above.
Angular 1.3 only supports jQuery 2.1 or above. jQuery 1.7 and newer might work correctly with Angular
but we don't guarantee that.


### What is testability like in Angular?
Expand Down
3 changes: 2 additions & 1 deletion docs/content/tutorial/step_12.ngdoc
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ __`app/index.html`.__
```

<div class="alert alert-error">
**Important:** Be sure to use jQuery version `1.10.x`. AngularJS does not yet support jQuery `2.x`.
**Important:** Be sure to use jQuery version 2.1 or newer when using Angular 1.3; jQuery 1.x is
not officially supported.
Be sure to load jQuery before all AngularJS scripts, otherwise AngularJS won't detect jQuery and
animations will not work as expected.
</div>
Expand Down
4 changes: 3 additions & 1 deletion src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -1459,7 +1459,9 @@ function bindJQuery() {
// bind to jQuery if present;
jQuery = window.jQuery;
// Use jQuery if it exists with proper functionality, otherwise default to us.
// Angular 1.2+ requires jQuery 1.7.1+ for on()/off() support.
// Angular 1.2+ requires jQuery 1.7+ for on()/off() support.
// Angular 1.3+ technically requires at least jQuery 2.1+ but it may work with older
// versions. It will not work for sure with jQuery <1.7, though.
if (jQuery && jQuery.fn.on) {
jqLite = jQuery;
extend(jQuery.fn, {
Expand Down
20 changes: 19 additions & 1 deletion src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -2031,7 +2031,25 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
var fragment = document.createDocumentFragment();
fragment.appendChild(firstElementToRemove);
newNode[jqLite.expando] = firstElementToRemove[jqLite.expando];

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

// Remove data of the replaced element. We cannot just call .remove()
// on the element it since that would deallocate scope and event handlers that are still needed
// for the new node. Instead, remove the data "manually".
if (!jQuery) {
delete jqLite.cache[firstElementToRemove[jqLite.expando]];
} else {
// jQuery 2.x doesn't expose the data storage. Use jQuery.cleanData to clean up after the replaced
// element. Note that we need to use the original method here and not the one monkey-patched by Angular
// since the patched method emits the $destroy event causing the scope to be trashed and we do need
// the very same scope to work with the new element.
jQuery.cleanData.$$original([firstElementToRemove]);
}

for (var k = 1, kk = elementsToRemove.length; k < kk; k++) {
var element = elementsToRemove[k];
jqLite(element).remove(); // must do this way to clean up expando
Expand Down
3 changes: 2 additions & 1 deletion src/ngMock/.jshintrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"browser": true,
"globals": {
"angular": false,
"expect": false
"expect": false,
"jQuery": false
}
}
7 changes: 7 additions & 0 deletions src/ngMock/angular-mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -1956,6 +1956,13 @@ angular.mock.e2e.$httpBackendDecorator =


angular.mock.clearDataCache = function() {
// jQuery 2.x doesn't expose data attached to elements. We could use jQuery.cleanData
// to clean up after elements but we'd first need to know which elements to clean up after.
// Skip it then.
if (window.jQuery) {
return;
}

var key,
cache = angular.element.cache;

Expand Down
41 changes: 23 additions & 18 deletions test/helpers/testabilityPatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ beforeEach(function() {
});

afterEach(function() {
var count, cache;

if (this.$injector) {
var $rootScope = this.$injector.get('$rootScope');
var $rootElement = this.$injector.get('$rootElement');
Expand All @@ -46,25 +48,27 @@ afterEach(function() {
$log.assertEmpty && $log.assertEmpty();
}

// complain about uncleared jqCache references
var count = 0;
if (!window.jQuery) {
// jQuery 2.x doesn't expose the cache storage.

// This line should be enabled as soon as this bug is fixed: http://bugs.jquery.com/ticket/11775
//var cache = jqLite.cache;
var cache = angular.element.cache;
// complain about uncleared jqCache references
count = 0;

forEachSorted(cache, function(expando, key){
angular.forEach(expando.data, function(value, key){
count ++;
if (value && value.$element) {
dump('LEAK', key, value.$id, sortedHtml(value.$element));
} else {
dump('LEAK', key, angular.toJson(value));
}
cache = angular.element.cache;

forEachSorted(cache, function (expando, key) {
angular.forEach(expando.data, function (value, key) {
count++;
if (value && value.$element) {
dump('LEAK', key, value.$id, sortedHtml(value.$element));
} else {
dump('LEAK', key, angular.toJson(value));
}
});
});
});
if (count) {
throw new Error('Found jqCache references that were not deallocated! count: ' + count);
if (count) {
throw new Error('Found jqCache references that were not deallocated! count: ' + count);
}
}


Expand Down Expand Up @@ -95,8 +99,9 @@ function dealoc(obj) {
if (obj) {
if (angular.isElement(obj)) {
cleanup(angular.element(obj));
} else {
for(var key in jqCache) {
} else if (!window.jQuery) {
// jQuery 2.x doesn't expose the cache storage.
for (var key in jqCache) {
var value = jqCache[key];
if (value.data && value.data.$scope == obj) {
delete jqCache[key];
Expand Down
8 changes: 0 additions & 8 deletions test/jqLiteSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,6 @@ describe('jqLite', function() {
});

describe('_data', function() {
it('should provide access to the data present on the element', function() {
var element = jqLite('<i>foo</i>');
var data = ['value'];
element.data('val', data);
expect(angular.element._data(element[0]).data.val).toBe(data);
dealoc(element);
});

it('should provide access to the events present on the element', function() {
var element = jqLite('<i>foo</i>');
expect(angular.element._data(element[0]).events).toBeUndefined();
Expand Down
36 changes: 34 additions & 2 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4028,7 +4028,12 @@ describe('$compile', function() {



it('should not leak if two "element" transclusions are on the same element', function() {
it('should not leak if two "element" transclusions are on the same element', function () {
if (jQuery) {
// jQuery 2.x doesn't expose the cache storage.
return;
}

var calcCacheSize = function() {
var size = 0;
forEach(jqLite.cache, function(item, key) { size++; });
Expand Down Expand Up @@ -4056,7 +4061,11 @@ describe('$compile', function() {
});


it('should not leak if two "element" transclusions are on the same element', function() {
it('should not leak if two "element" transclusions are on the same element', function () {
if (jQuery) {
// jQuery 2.x doesn't expose the cache storage.
return;
}
var calcCacheSize = function() {
var size = 0;
forEach(jqLite.cache, function(item, key) { size++; });
Expand Down Expand Up @@ -4086,6 +4095,29 @@ describe('$compile', function() {
});
});

if (jQuery) {
it('should clean up after a replaced element', inject(function ($compile) {
var privateData, firstRepeatedElem;

element = $compile('<div><div ng-repeat="x in xs">{{x}}</div></div>')($rootScope);

$rootScope.$apply('xs = [0,1]');
firstRepeatedElem = element.children('.ng-scope').eq(0);

expect(firstRepeatedElem.data('$scope')).toBeDefined();
privateData = jQuery._data(firstRepeatedElem[0]);
expect(privateData.events).toBeDefined();
expect(privateData.events.$destroy).toBeDefined();
expect(privateData.events.$destroy[0]).toBeDefined();

$rootScope.$apply('xs = null');

expect(firstRepeatedElem.data('$scope')).not.toBeDefined();
privateData = jQuery._data(firstRepeatedElem[0]);
expect(privateData && privateData.events).not.toBeDefined();
}));
}


it('should remove transclusion scope, when the DOM is destroyed', function() {
module(function() {
Expand Down
5 changes: 5 additions & 0 deletions test/ngMock/angular-mocksSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,11 @@ describe('ngMock', function() {


describe('angular.mock.clearDataCache', function() {
if (window.jQuery) {
// jQuery 2.x doesn't expose the cache storage.
return;
}

function keys(obj) {
var keysArr = [];
for(var key in obj) {
Expand Down

0 comments on commit 9e7cb3c

Please sign in to comment.