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

Adding ng-jq functionality, the ability to force jqLite or any other lib... #10761

Closed
wants to merge 1 commit into from
Closed

Adding ng-jq functionality, the ability to force jqLite or any other lib... #10761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 61 additions & 1 deletion src/Angular.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
shallowCopy: true,
equals: true,
csp: true,
jq: true,
concat: true,
sliceArgs: true,
bind: true,
Expand Down Expand Up @@ -899,7 +900,61 @@ var csp = function() {
return (csp.isActive_ = active);
};

/**
* @ngdoc directive
* @module ng
* @name ngJq
*
* @element ANY
* @param {string=} the name of the library available under `window`
* to be used for angular.element
* @description
* Use this directive to force the angular.element library. This should be
* used to force either jqLite by leaving ng-jq blank or setting the name of
* the jquery variable under window (eg. jQuery).
*
* Since this directive is global for the angular library, it is recommended
* that it's added to the same element as ng-app or the HTML element, but it is not mandatory.
* It needs to be noted that only the first instance of `ng-jq` will be used and all others
* ignored.
*
* @example
* This example shows how to force jqLite using the `ngJq` directive to the `html` tag.
```html
<!doctype html>
<html ng-app ng-jq>
...
...
</html>
```
* @example
* This example shows how to use a jQuery based library of a different name.
* The library name must be available at the top most 'window'.
```html
<!doctype html>
<html ng-app ng-jq="jQueryLib">
...
...
</html>
```
*/
var jq = function() {
Copy link
Member

Choose a reason for hiding this comment

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

jsHint will catch it (see my other comments), you can search for csp and see where it needs to be added to pass jsHint tests.

if (isDefined(jq.name_)) return jq.name_;
var el;
var i, ii = ngAttrPrefixes.length;
for (i = 0; i < ii; ++i) {
if (el = document.querySelector('[' + ngAttrPrefixes[i].replace(':', '\\:') + 'jq]')) {
break;
}
}

var name;
if (el) {
name = getNgAttribute(el, "jq");
}

return (jq.name_ = name);
};

function concat(array1, array2, index) {
return array1.concat(slice.call(array2, index));
Expand Down Expand Up @@ -1472,7 +1527,12 @@ function bindJQuery() {
}

// bind to jQuery if present;
jQuery = window.jQuery;
var jqName = jq();
jQuery = window.jQuery; // use default jQuery.
if (isDefined(jqName)) { // `ngJq` present
jQuery = jqName === null ? undefined : window[jqName]; // if empty; use jqLite. if not empty, use jQuery specified by `ngJq`.
}

// Use jQuery if it exists with proper functionality, otherwise default to us.
// 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
Expand Down
1 change: 1 addition & 0 deletions test/.jshintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"shallowCopy": false,
"equals": false,
"csp": false,
"jq": false,
"concat": false,
"sliceArgs": false,
"bind": false,
Expand Down
88 changes: 88 additions & 0 deletions test/AngularSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,94 @@ describe('angular', function() {
});


describe('jq', function() {
var element;

beforeEach(function() {
element = document.createElement('html');
});

afterEach(function() {
delete jq.name_;
Copy link
Member

Choose a reason for hiding this comment

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

Just being curious: Why first set it to undefined and then delete it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gkalpak Sorry, force of habit from a time when delete was buggy in javascript. I guess that's fixed now since Angular supports IE 9 and above.

});

it('should return undefined when jq is not set, no jQuery found (the default)', function() {
expect(jq()).toBe(undefined);
});

it('should return empty string when jq is enabled manually via [ng-jq] with empty string', function() {
Copy link
Member

Choose a reason for hiding this comment

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

I don't get the purpose of the functionality tested here :(
(Same goes for all similar tests below.)

Copy link
Member

Choose a reason for hiding this comment

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

It's to differentiate between an ng-jq attribute without value that means "just use jqLite and not setting it at all which means "use the native detection mechanism".

Not the prettiest APIs of all, I agree, but it's used just in one place really so I didn't picked at it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gkalpak @mzgol Again, open to suggestions. This is my first time coding angular itself and I'm not sure what would be the best approach.

element.setAttribute('ng-jq', '');
spyOn(document, 'querySelector').andCallFake(function(selector) {
if (selector === '[ng-jq]') return element;
});
expect(jq()).toBe('');
});

it('should return empty string when jq is enabled manually via [data-ng-jq] with empty string', function() {
element.setAttribute('data-ng-jq', '');
spyOn(document, 'querySelector').andCallFake(function(selector) {
if (selector === '[data-ng-jq]') return element;
});
expect(jq()).toBe('');
expect(document.querySelector).toHaveBeenCalledWith('[data-ng-jq]');
});

it('should return empty string when jq is enabled manually via [x-ng-jq] with empty string', function() {
element.setAttribute('x-ng-jq', '');
spyOn(document, 'querySelector').andCallFake(function(selector) {
if (selector === '[x-ng-jq]') return element;
});
expect(jq()).toBe('');
expect(document.querySelector).toHaveBeenCalledWith('[x-ng-jq]');
});

it('should return empty string when jq is enabled manually via [ng:jq] with empty string', function() {
element.setAttribute('ng:jq', '');
spyOn(document, 'querySelector').andCallFake(function(selector) {
if (selector === '[ng\\:jq]') return element;
});
expect(jq()).toBe('');
expect(document.querySelector).toHaveBeenCalledWith('[ng\\:jq]');
});

it('should return "jQuery" when jq is enabled manually via [ng-jq] with value "jQuery"', function() {
element.setAttribute('ng-jq', 'jQuery');
spyOn(document, 'querySelector').andCallFake(function(selector) {
if (selector === '[ng-jq]') return element;
});
expect(jq()).toBe('jQuery');
expect(document.querySelector).toHaveBeenCalledWith('[ng-jq]');
});

it('should return "jQuery" when jq is enabled manually via [data-ng-jq] with value "jQuery"', function() {
element.setAttribute('data-ng-jq', 'jQuery');
spyOn(document, 'querySelector').andCallFake(function(selector) {
if (selector === '[data-ng-jq]') return element;
});
expect(jq()).toBe('jQuery');
expect(document.querySelector).toHaveBeenCalledWith('[data-ng-jq]');
});

it('should return "jQuery" when jq is enabled manually via [x-ng-jq] with value "jQuery"', function() {
element.setAttribute('x-ng-jq', 'jQuery');
spyOn(document, 'querySelector').andCallFake(function(selector) {
if (selector === '[x-ng-jq]') return element;
});
expect(jq()).toBe('jQuery');
expect(document.querySelector).toHaveBeenCalledWith('[x-ng-jq]');
});

it('should return "jQuery" when jq is enabled manually via [ng:jq] with value "jQuery"', function() {
element.setAttribute('ng:jq', 'jQuery');
spyOn(document, 'querySelector').andCallFake(function(selector) {
if (selector === '[ng\\:jq]') return element;
});
expect(jq()).toBe('jQuery');
expect(document.querySelector).toHaveBeenCalledWith('[ng\\:jq]');
});
});
Copy link
Member

Choose a reason for hiding this comment

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

I believe there should be also some tests verifying the correct behaviour when jQuery is set on window (either with or without ngJq).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gkalpak I agree, but when I tried to do said test, it would fail all other tests since the real jQuery was being disrupted, which is used in other tests. I tried using the angularInit tests as an example since I need to test out the bindJquery function. Any direction would be appreciated.

Copy link
Member

Choose a reason for hiding this comment

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

@mboudreau: I was thinking something "non-disruptive". Maybe verifying what jqLite is bound to, both with and without jQuery being loaded (you can differentiate by checking the _jqLiteMode global.

But I don't feel too strongly about it :)



describe('parseKeyValue', function() {
it('should parse a string into key-value pairs', function() {
expect(parseKeyValue('')).toEqual({});
Expand Down