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

feat(jQuery): upgrade to jQuery to 2.1.1 #7311

Merged
merged 1 commit into from
Jul 31, 2014
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gruntfile.js
Original file line number Diff line number Diff line change
@@ -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: {
4 changes: 2 additions & 2 deletions angularFiles.js
Original file line number Diff line number Diff line change
@@ -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',
@@ -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',
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",
5 changes: 3 additions & 2 deletions docs/content/misc/faq.ngdoc
Original file line number Diff line number Diff line change
@@ -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?
3 changes: 2 additions & 1 deletion docs/content/tutorial/step_12.ngdoc
Original file line number Diff line number Diff line change
@@ -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>
4 changes: 3 additions & 1 deletion src/Angular.js
Original file line number Diff line number Diff line change
@@ -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, {
20 changes: 19 additions & 1 deletion src/ng/compile.js
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion src/ngMock/.jshintrc
Original file line number Diff line number Diff line change
@@ -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
@@ -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;

41 changes: 23 additions & 18 deletions test/helpers/testabilityPatch.js
Original file line number Diff line number Diff line change
@@ -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');
@@ -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);
}
}


@@ -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];
8 changes: 0 additions & 8 deletions test/jqLiteSpec.js
Original file line number Diff line number Diff line change
@@ -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() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests sth we shouldn't rely on. jQuery 1 made user data available on ._data(elem).data which is also available via .data(elem). jQuery 2 completely separated those storages so the former way does no longer work. It's a good thing, though; why rely on the private ._data(elem).data that could break in the future if we can just do .data(elem) and be safe?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we originally had this in place just to test that our implementation was compatible with jQuery. This is no longer the case and the _data method we expose is not needed by angular or jqLite so we should just get rid of it. can you think of a reason to keep it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'll get rid of it.

EDIT: Hmm, actually, ngAnimate requires _data. But what's tested here is not the _data method itself but the implementation detail that user data storage is implemented as a value under the key "data" in the private data storage. jqLite data is still implemented in this way, is it worth to refactor it now?

EDIT 2: The only line in the whole Angular code base that uses _data is this one:

var elementEvents = angular.element._data(runner.node);

There doesn't seem to be an easy way around that, though, so we might need to keep it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that line checks if there are event listeners registered on a particular node. Wouldn't this stop working with jQuery? should we be checking for that in a different way?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's there for performance reasons. There is no other way (and for that matter, there's no official way at all) to check if there are event listeners attached to an element (similarly, there is no such way to check for native, non-jQuery events).

The change was introduced in commit cf5e463.

It might be that we should create a fast path for triggerHandler in both jqLite & jQuery so that they both check for event handlers before doing the rest so that this hack in Angular is not needed. We'd be able to remove the jqLite._data method then.

In jQuery it won't happen before 2.2, though, and Angular 1.3 will be released before that happens.

var element = jqLite('<i>foo</i>');
expect(angular.element._data(element[0]).events).toBeUndefined();
36 changes: 34 additions & 2 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
@@ -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++; });
@@ -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++; });
@@ -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() {
5 changes: 5 additions & 0 deletions test/ngMock/angular-mocksSpec.js
Original file line number Diff line number Diff line change
@@ -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) {