Bug Tracker

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#15137 closed bug (notabug)

$.extend passes deep properties of plain objects by reference

Reported by: evgn Owned by:
Priority: undecided Milestone: None
Component: unfiled Version: 2.1.1
Keywords: Cc:
Blocked by: Blocking:

Description

$.extend passes deep properties of plain objects by reference instead of copying them. This happens only when the "deep" option is not set to "true".

See http://jsfiddle.net/E3qk9/

Code to reproduce :

var a = {
        X: 'good',
        Y: {
            Z: 'good'
        }
    },
    b = $.extend({}, a);

b.X = 'bad';
b.Y.Z = 'bad';

// a.X equals "good" but a.Y.Z has been modified to "bad" :
console.log(JSON.stringify(a));

Change History (4)

comment:1 in reply to:  description Changed 8 years ago by scottgonzalez

Resolution: notabug
Status: newclosed

Replying to evgn:

This happens only when the "deep" option is not set to "true".

Exactly. That's why the deep option exists.

comment:2 Changed 8 years ago by evgn

I understand the logic, but this is a documentation issue then. According to the documentation, the deep option is about recursivity in the merge strategy, there is no word about the fact that objects are passed by reference instead of copied.

Furthermore, the doc seems to suggest the opposite if don't read extra-carefully. In the example, it says :

If, however, you want to preserve both of the original objects, you can do so by passing an empty object as the target

Well, the original objects will not be preserved "in the future". They will be modified if the result of the extending is. And later again :

// Merge defaults and options, without modifying defaults
var settings = $.extend( {}, defaults, options );

In the example, the defaults are strings and integers, I get it now, but I didn't know that was a super important detail. Because if one of the properties had been an object, the defaults would be modified if settings were modified. And defaults are not supposed to be modified, are they. Inadvertently overriding the defaults is precisely what I was doing when I encountered this issue and spent 2 hours looking for the cause.

Do you have the possibility to readdress this ticket as a documentation issue or do I have to create a new one ? Thank you.

comment:3 Changed 8 years ago by dmethvin

This isn't an issue with jQuery. We assume a basic level of knowledge of JavaScript and cannot explain all language issues each time we describe a jQuery API. That would make the documentation very unwieldy. Since this is the first time that I can recall anyone being confused on these issues, I don't think there's a need to clarify anything.

comment:4 Changed 8 years ago by evgn

I'm a seasoned frontend developer and I know that objects are passed by reference to functions. However there is no way I could guess exactly what $.extend does internally with them according to the current documentation.

In deep copy mode, values of merged objects whose key do not exist in the target object could as well be passed by reference (since there is no need to extend a target property), but they're not. That also is an arbitrary unobvious and undocumented choice that lead me to believe that the same thing was happening in shallow mode.

My proposition is a very humble addition : only 5 words that say everything.

The values are not merged.

replaced by

The values are not merged and are passed by reference.

And if you are in a generous mood,

On a deep extend, Object and Array are extended,

could also become a more explicit

On a deep extend, Object and Array are extended with copied properties,

I'd be grateful if you could give it a thought, at least for the first addition.

Note: See TracTickets for help on using tickets.