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

Commit

Permalink
fix(isArrayLike): correctly handle string primitives
Browse files Browse the repository at this point in the history
Closes #3356
  • Loading branch information
mernen authored and btford committed Oct 2, 2013
1 parent fc8034b commit 5b8c788
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
11 changes: 5 additions & 6 deletions src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,22 +87,21 @@ if (isNaN(msie)) {
/**
* @private
* @param {*} obj
* @return {boolean} Returns true if `obj` is an array or array-like object (NodeList, Arguments, ...)
* @return {boolean} Returns true if `obj` is an array or array-like object (NodeList, Arguments, String ...)
*/
function isArrayLike(obj) {
if (obj == null || isWindow(obj)) {
return false;
}

var length = obj.length;

if (obj.nodeType === 1 && length) {
return true;
}

return isArray(obj) || !isFunction(obj) && (

This comment has been minimized.

Copy link
@mernen

mernen Oct 3, 2013

Author Contributor

@btford Since my original typeof guard didn't make it through, this !isFunction(object) test should be kept!

I propose this statement:

return isArray(obj) || isString(obj) || !isFunction(obj) && (
    length === 0 || typeof length === "number" && length > 0 && (length - 1) in obj
);

(exact same as before my PR, only adding || isString(obj))

This comment has been minimized.

Copy link
@btford

btford Oct 3, 2013

Contributor

Is there a case for which the current behavior would do the wrong thing? If so, would you like to send a PR for this change along with the test? 😄

This comment has been minimized.

Copy link
@dcherman

dcherman Oct 3, 2013

Contributor

@btford Ya, this implementation would report a function with no arguments as being array-like unless I'm mistaken.

isArrayLike( function() {} )
length === 0 || typeof length === "number" && length > 0 && (length - 1) in obj
);
return isString(obj) || isArray(obj) || length === 0 ||
typeof length === 'number' && length > 0 && (length - 1) in obj;
}

/**
Expand Down Expand Up @@ -599,7 +598,7 @@ function isLeafNode (node) {
* * If a destination is provided, all of its elements (for array) or properties (for objects)
* are deleted and then all elements/properties from the source are copied to it.
* * If `source` is not an object or array (inc. `null` and `undefined`), `source` is returned.
* * If `source` is identical to 'destination' an exception will be thrown.
* * If `source` is identical to 'destination' an exception will be thrown.
*
* Note: this function is used to augment the Object type in Angular expressions. See
* {@link ng.$filter} for more information about Angular arrays.
Expand Down
7 changes: 7 additions & 0 deletions test/AngularSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,13 @@ describe('angular', function() {
expect(log).toEqual(['0:a', '1:b', '2:c']);
});

it('should handle string values like arrays', function() {
var log = [];

forEach('bar', function(value, key) { log.push(key + ':' + value)});
expect(log).toEqual(['0:b', '1:a', '2:r']);
});


it('should handle objects with length property as objects', function() {
var obj = {
Expand Down

0 comments on commit 5b8c788

Please sign in to comment.