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

feat(isArray): make angular.isArray support Array subclasses #15541

Closed
wants to merge 1 commit into from
Closed

feat(isArray): make angular.isArray support Array subclasses #15541

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
9 changes: 5 additions & 4 deletions src/Angular.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,7 @@ function isArrayLike(obj) {

// NodeList objects (with `item` method) and
// other objects with suitable length characteristics are array-like
return isNumber(length) &&
(length >= 0 && ((length - 1) in obj || obj instanceof Array) || typeof obj.item === 'function');
return isNumber(length) && (length >= 0 && (length - 1) in obj || typeof obj.item === 'function');
Copy link
Member

Choose a reason for hiding this comment

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

We are relaxing the condition a bit, but it is OK imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isArray is used in a couple of lines above. So if obj instanceof Array, the function hits return on that line.

Copy link
Member

@gkalpak gkalpak Dec 22, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I know. But previously there was also the restriction that isNumber(length) and length >= 0, which is gone now.

What I am saying is that isArrayLike (and anything depending on it) is slightly affected by the change in isArray.


}

Expand Down Expand Up @@ -635,12 +634,14 @@ function isDate(value) {
* @kind function
*
* @description
* Determines if a reference is an `Array`. Alias of Array.isArray.
* Determines if a reference is an `Array`.
*
* @param {*} value Reference to check.
* @returns {boolean} True if `value` is an `Array`.
*/
var isArray = Array.isArray;
function isArray(arr) {
return arr instanceof Array || Array.isArray(arr);
Copy link

Choose a reason for hiding this comment

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

Array.isArray(arr) is unreachable here, since by definition any array exotic object is instanceof Array.

Copy link
Member

Choose a reason for hiding this comment

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

Not if it comes from a different context (e.g. another Window object). E.g.:

var win = window.open();
var arr = new win.Array();

console.log(arr instanceof Array);   // false
console.log(Array.isArray(arr));   // true

Copy link

Choose a reason for hiding this comment

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

good call

}

/**
* @description
Expand Down
10 changes: 10 additions & 0 deletions test/AngularSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1251,6 +1251,16 @@ describe('angular', function() {
});
});

describe('isArray', function() {

it('should return true if passed an object prototypically inherited from Array.prototype', function() {
function FooArray() {}
FooArray.prototype = [];
expect(isArray(new FooArray())).toBe(true);
});

});

describe('isArrayLike', function() {

it('should return false if passed a number', function() {
Expand Down