Ticket #10867 (closed enhancement: wontfix)
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: | ||
| Blocking: | Blocked by: |
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, ...)
Change History
comment:2 Changed 18 months ago by rwaldron
- Owner set to gibson042
- Status changed from new to pending
Where does it say that passing false should have this behavior?
comment:3 Changed 18 months ago by gibson042
- Status changed from pending to 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)).
comment:4 Changed 18 months ago by rwaldron
- Priority changed from undecided to low
- Resolution set to duplicate
- Status changed from new to closed
- Component changed from unfiled to core
Sorry, but we'e already been down this road. A quick ticket search for "extend false" brings up the original
comment:6 Changed 18 months ago by gibson042
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.
comment:7 Changed 18 months ago by rwaldron
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"
comment:8 Changed 18 months ago by timmywil
- Keywords 1.8-discuss added
- Status changed from closed to reopened
- Resolution duplicate deleted
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.
comment:9 Changed 18 months ago by timmywil
- Status changed from reopened to open
- Type changed from bug to enhancement
comment:10 Changed 18 months ago by Erik_A
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;
}
comment:11 Changed 18 months ago by dmethvin
- Keywords needsdocs added; 1.8-discuss removed
- Status changed from open to closed
- Resolution set to wontfix
comment:12 Changed 15 months ago by timmywil
#11411 is a duplicate of this ticket.
Please follow the bug reporting guidlines and use jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

https://github.com/jquery/jquery/pull/615