Skip to content

Bug 7260 - Allowing length prop to be used in jQuery.each #336

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 4 commits into from

Conversation

jboesch
Copy link
Contributor

@jboesch jboesch commented Apr 19, 2011

With jQuery.map supporting objects in 1.6 (and works with props named 'length') I think we should consider adding 'length' prop support to jQuery.each. Here is Dan Heberden's isArray check (that I'm using) used in the jQuery.map commit: https://github.com/jquery/jquery/pull/299/files#L0R713

I discussed a little here: http://bugs.jquery.com/ticket/7260
Fiddle: http://jsfiddle.net/jboesch26/Ungsa/

This would allow looping over objects with length props. This newEach seems to be slightly faster as well (in all except IE): http://jsperf.com/each-object-supporting-length-key/6

I realize this is a touchy change, as jQuery.each is used absolutely everywhere... so any thoughts on this would be rockin' :)

@@ -610,13 +610,15 @@ jQuery.extend({
} else {
if ( isObj ) {
for ( name in object ) {
if ( callback.call( object[ name ], name, object[ name ] ) === false ) {
v = object[ name ];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I'm storing it in a var is because when I ran tests, it proved to be slightly more performant. Hey, every ms counts right!.. especially in jQuery.each.

@rwaldron
Copy link
Member

I'm not sure I see the reason for adding another step to the loop stack. What does this look like with JUST support for a length property and no modifications to the rest of the logic?

@jboesch
Copy link
Contributor Author

jboesch commented Apr 21, 2011

The reason I added another step to assign the variable, is because it proved to be slightly faster than doing a lookup on the object twice. In this perf test http://jsperf.com/each-object-supporting-length-key/6, newEach just contains the logic for allowing length prop to be used, in newEach2 it contains the logic for allowing length prop as well as the variable assignments in the loops.

@jboesch
Copy link
Contributor Author

jboesch commented Apr 23, 2011

Reverted those local vars in the loop stack.

@jboesch
Copy link
Contributor Author

jboesch commented Apr 28, 2011

Note to self: Add jQuery.isPlainObject as a last check
#356

@rkatic
Copy link
Contributor

rkatic commented Apr 29, 2011

I started a discussion on isArrayLike thing: http://forum.jquery.com/topic/jquery-isarraylike-for-consistency

@jboesch
Copy link
Contributor Author

jboesch commented Apr 29, 2011

Closing because this doesn't need to be addressed right away. Also, im so tired of talking about length prop support for jQuery.map and other ppl proposing isArrayLike stuff. Peace.

@jboesch jboesch closed this Apr 29, 2011
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants