Side navigation
#10867 closed enhancement (wontfix)
Opened November 22, 2011 09:32PM UTC
Closed December 06, 2011 06:56PM UTC
Last modified October 15, 2012 10:04PM UTC
jQuery.extend replaces the target object when deep extend is explicitly false
Reported by: | gibson042 | Owned by: | gibson042 |
---|---|---|---|
Priority: | low | Milestone: | None |
Component: | core | Version: | 1.7.1 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
jQuery.extend
has an optional leading parameter that toggles deep (recursive) merging. But when it is set to false
, the function creates, extends, and returns a new object instead of the specified target.
Problematic call: jQuery.extend(false, objTarget, objExtension, ...)
Attachments (0)
Change History (13)
Changed November 22, 2011 09:42PM UTC by comment:1
Changed November 23, 2011 01:24AM UTC by comment:2
owner: | → gibson042 |
---|---|
status: | new → pending |
Where does it say that passing false should have this behavior?
Changed November 23, 2011 04:32AM UTC by comment:3
status: | pending → new |
---|
It's implied from both documentation and code.
http://api.jquery.com/jQuery.extend/ specifies the jQuery.extend( [deep], target, object1 [, objectN] )
signature:
- **deep** If true, the merge becomes recursive (aka. deep copy).
- **target** The object to extend. It will receive the new properties.
And further specifies:
Keep in mind that the target object (first argument) will be modified, and will also be returned from $.extend()
.
Since there is a change in behavior *if* **deep** is true, it should be possible to specify it non-true, meaning that a jQuery.extend(false, deep3, deep4)
call has a **target** of deep3
. And since documentation specifies that the target object is modified and returned, it should further be true that jQuery.extend(false, deep3, deep4) === deep3
.
I also call your attention to line 311, which tests if ( typeof target === "boolean" )
; rather pointless after target = arguments[0] || {}
since it might as well be if ( target === true )
.
All that aside, the change makes jQuery.extend more consistent, smaller, and slightly more useful (since without it, jQuery.extend(false, deep3, deep4)
is equivalent to jQuery.extend({}, deep3, deep4)
).
Changed November 23, 2011 05:05PM UTC by comment:4
component: | unfiled → core |
---|---|
priority: | undecided → low |
resolution: | → duplicate |
status: | new → closed |
Sorry, but we'e already been down this road. A quick ticket search for "extend false" brings up the original
Changed November 23, 2011 05:22PM UTC by comment:6
With all due respect, the tickets are distinct.
#9994 was asking for the replacement of if ( typeof target === "boolean" )
with if ( target === true )
.
I am asking for the replacement of target = arguments[0] || {}
with target = arguments[0]
, to support dmethvin's $.extend(iCanHazDeep, defaults, options)
example from comment 3.
Changed November 23, 2011 05:23PM UTC by comment:7
For clarification...
"duplicate in that they both seek to arbitrarily change jQuery.extend() where no change is needed, with regard to the left most argument"
Changed November 23, 2011 06:19PM UTC by comment:8
keywords: | → 1.8-discuss |
---|---|
resolution: | duplicate |
status: | closed → reopened |
rwaldron and I have discussed this and we're reopening for further discussion.
Granted passing false is not documented, but I'm of the opinion that it seems a fair assumption that false can be passed if true can be passed, especially given the possibility of a boolean var being passed. I'd rather have it do nothing special when false is passed than create a new object. In other words, these should not be equivalent, but they currently are:
$.extend(false, obj, otherObj); $.extend({}, obj, otherObj);
It should not have an affect on the original source object that is returned.
Besides, the pull has no performance (actually a micro-optimization) or size issues so that's mostly why I'm fine with it.
Changed November 23, 2011 06:20PM UTC by comment:9
status: | reopened → open |
---|---|
type: | bug → enhancement |
Changed November 25, 2011 12:02PM UTC by comment:10
A few days before, I also ran into this issue and fixed it myself.
I replaced the declaration of target, i and depth variable declaration with the following:
deep = arguments[0] === true, i = 1 & (deep === arguments[0]), target = arguments[i++] || {};
What's happening in my code is the following: first the deep argument is assigned and set to true if only the first argument is actually a boolean and equals true.
The variable i is set to 1 and bitwise compared to whether or not, deep is of the same type of the fist argument. If the first argument is a boolean, then 1 & true becomes 1. (If arg0 is a object, deep === arg0 is false, thus is not a boolean. But if arg0 is false, deep === arg0 makes true).
Finally the target value is assigned: if arg0 is not a boolean, i === 0, else i === 1. After setting the target is set, i is incremented.
The deep, i and target variables containes now correct values and makes the check of target redundant. The following should be removed:
// Handle a deep copy situation if (typeof target === "boolean") { deep = target; target = arguments[1] || {}; // skip the boolean and the target i = 2; }
Changed December 06, 2011 06:56PM UTC by comment:11
keywords: | 1.8-discuss → needsdocs |
---|---|
resolution: | → wontfix |
status: | open → closed |
Changed February 28, 2012 05:41PM UTC by comment:12
#11411 is a duplicate of this ticket.
Changed October 15, 2012 10:04PM UTC by comment:13
keywords: | needsdocs |
---|
https://github.com/jquery/jquery/pull/615