Bug Tracker

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#9994 closed enhancement (wontfix)

Optimization improvement in jQuery.extend

Reported by: anonymous Owned by: Rick Waldron
Priority: low Milestone:
Component: core Version: 1.6.2
Keywords: Cc:
Blocked by: Blocking:

Description

On line 313 of core.js:

https://github.com/jquery/jquery/blob/master/src/core.js#L313

there is a check for:

typeof target === "boolean"

However, target has been set by:

target = arguments[0] || {},

so in case it's a boolean, it's always true (if arguments[0] was false, target is {}). Checking for true is much faster than a typeof check and then comparing strings, especially on IE:

http://jsperf.com/true-vs-typeof

Thanks for considering this optimization.

Change History (9)

comment:1 Changed 10 years ago by Rick Waldron

Component: unfiledcore
Milestone: None1.6.3
Owner: set to Rick Waldron
Priority: undecidedlow
Status: newassigned
Last edited 10 years ago by Rick Waldron (previous) (diff)

comment:2 Changed 10 years ago by timmywil

Seems to me that this will not set the target correctly if both false and a target is passed to extend. That may not happen very often, but would be a regression.

$.extend( false, { some: "target" }, { more: "stuff" } );

comment:3 Changed 10 years ago by dmethvin

More often than I would have expected, that's fer sure.

http://google.com/codesearch#search/&q=\$\.extend\(\s*false

There are probably more of the form $.extend(iCanHazDeep, defaults, options) but they're too hard to grep.

comment:4 Changed 10 years ago by Rick Waldron

If arguments[0] is any kind of falsy, it's not boolean by the time it reaches that condition.

comment:5 Changed 10 years ago by Rick Waldron

Sorry, by "it" I meant the target var

comment:6 Changed 10 years ago by Rick Waldron

Resolution: wontfix
Status: assignedclosed

Turns out this is a performance hit, so no thanks.

comment:7 Changed 10 years ago by timmywil

This is now neither here nor there, but let me explain further. I don't think this would work anyway because target needs to get assigned to arguments[1] even if arguments[0] is false. It can't just be an empty plain object. The only place that happens is from within the typeof conditional, so it can't change unless that assignment is added somewhere else and at that point we're talking about changing more than just the conditional.

comment:8 Changed 10 years ago by dmethvin

Milestone: 1.6.3

comment:9 Changed 10 years ago by Rick Waldron

#10867 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.