Bug Tracker

Opened 6 years ago

Closed 6 years ago

#14649 closed bug (notabug)

isArrayLike throws on strings (should return false)

Reported by: Nigel Owned by: Rick Waldron
Priority: undecided Milestone: None
Component: unfiled Version: 2.0.3
Keywords: Cc: gibson042
Blocked by: Blocking:

Description

If you call isArrayLike with a string then it should return false, instead it throws an error (e.g. passing the string 'foonana'):

"TypeError: Cannot use 'in' operator to search for '6' in foonana"

Change History (11)

comment:1 Changed 6 years ago by Rick Waldron

Owner: set to Rick Waldron
Status: newassigned

comment:2 Changed 6 years ago by Rick Waldron

Cc: gibson042 added

We could/should add a check for type === "string" here: https://github.com/jquery/jquery/blob/master/src/core.js#L495 which would make jQuery's "array-like" checking future-friendly with emerging language additions, ie. String.prototype.@@iterator, Array.from(string), etc.

Before I go ahead with this, I want Richard to sign off on my rationale above

comment:3 Changed 6 years ago by gibson042

isArrayLike is a private function, and as far as I can tell never receives string input from valid public calls. So my default position is to oppose changes until seeing a demonstration warranting such.

comment:4 in reply to:  3 ; Changed 6 years ago by Rick Waldron

Replying to gibson042:

isArrayLike is a private function, and as far as I can tell never receives string input from valid public calls. So my default position is to oppose changes until seeing a demonstration warranting such.

Good call and agreed.

comment:5 in reply to:  4 ; Changed 6 years ago by Nigel

Replying to rwaldron:

Replying to gibson042:

isArrayLike is a private function, and as far as I can tell never receives string input from valid public calls. So my default position is to oppose changes until seeing a demonstration warranting such.

It means whenever you call $.map or other functions that use isArrayLike you get a horrible error message back. The extra overhead of checking for string is surely negligible for a better return value?

comment:6 in reply to:  5 ; Changed 6 years ago by markelog

Replying to Nigel:

Replying to rwaldron:

Replying to gibson042:

isArrayLike is a private function, and as far as I can tell never receives string input from valid public calls. So my default position is to oppose changes until seeing a demonstration warranting such.

It means whenever you call $.map or other functions that use isArrayLike you get a horrible error message back. The extra overhead of checking for string is surely negligible for a better return value?

Could you please provide a test case using public jQuery API? You could use jsfiddle or similar service for that.

comment:7 Changed 6 years ago by dmethvin

It means whenever you call $.map or other functions that use isArrayLike you get a horrible error message back.

There are no guarantees what you'll get back when you call an API method with incorrect arguments.

comment:8 in reply to:  6 ; Changed 6 years ago by anonymous

Replying to markelog:

Could you please provide a test case using public jQuery API? You could use jsfiddle or similar service for that.

just call $.map with a string: http://jsfiddle.net/aH4mU/

comment:9 in reply to:  7 Changed 6 years ago by Nigel

Replying to dmethvin:

It means whenever you call $.map or other functions that use isArrayLike you get a horrible error message back.

There are no guarantees what you'll get back when you call an API method with incorrect arguments.

isArrayLike, even as a private function, should (IMHO) not throw an exception if passed a non array - it should return false. Surely the whole point of having it is to check if it is array like and do something else if not? Though in its current form it probably blows up if the argument doesn't have a length property anyway.

comment:10 Changed 6 years ago by Nigel

comment:11 in reply to:  8 Changed 6 years ago by gibson042

Resolution: notabug
Status: assignedclosed

Replying to anonymous:

just call $.map with a string: http://jsfiddle.net/aH4mU/

jQuery.map accepts arrays and objects, not strings.

Replying to Nigel:

isArrayLike, even as a private function, should (IMHO) not throw an exception if passed a non array - it should return false. Surely the whole point of having it is to check if it is array like and do something else if not?

The whole point of having it is to identify those objects that should be iterated over by numeric index, generally to differentiate them from objects that should be iterated over with for...in loops. We take pains to avoid passing in strings, and if you circumvent that by calling jQuery methods with invalid input, then (as dmethvin said) "There are no guarantees what you'll get back".

And although we'd never add code to guarantee it, I consider throwing exceptions when a call steps outside the bounds of our documented API to be better behavior than silently propagating misconceptions.

Note: See TracTickets for help on using tickets.