Skip to content

the type of argument can be an ArrayLikeObject #801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

yadimon
Copy link
Contributor

@yadimon yadimon commented Aug 28, 2015

grep:
not only array can be accepted, but ArrayLikeObject too:

var $selected = $('.selected');
$.grep($selected, function(e, i){return true;})

if you look at the implementation
https://github.com/jquery/jquery/blob/2.1-stable/src/core.js#L384
it should be fully supported.
like by merge:
https://github.com/jquery/jquery/blob/2.1-stable/src/core.js#L370

@arthurvr
Copy link
Member

@timmywil Is this officially supported or simply a handy side effect?

@gnarf
Copy link
Member

gnarf commented Aug 30, 2015

jQuery.grep is only tested against real arrays

However it is passed a jQuery object (ArrayLikeObject) inside the code for jQuery.fn.filter

I'd say this is expected and 👍 on landing this update. Though we should also add a test case for filtering some array like? cc @timmywil

@AurelioDeRosa
Copy link
Member

Once jquery/jquery#2605 is merged, I'll merge this PR.

@AurelioDeRosa AurelioDeRosa self-assigned this Sep 20, 2015
@@ -4,7 +4,7 @@
<desc>Finds the elements of an array which satisfy a filter function. The original array is not affected.</desc>
<signature>
<added>1.0</added>
<argument name="array" type="Array">
<argument name="array" type="ArrayLikeObject">
<desc>The array to search through.</desc>
Copy link
Member

Choose a reason for hiding this comment

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

@yadimon Can you improve this sentence by specifying that it can be also an Array-like object?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants