jQuery Extend does not properly perform a deep copy on an instance of a class

[anonymous]

I discovered recently that performing a deep copy with $.extend does not work with classes.

If i create an object, and then assign a class to a property of that object, and do a deep copy to a new object, and change something in the original object, the new object reflects the changed value (or vice versa)

For example

function myclass() {

this.property1 = "abc"; this.property2 = "def";

}

var a = {};

a.myclass = new myclass(); a.test = "abc"; var b = {}; $.extend(true, b, a);

alert("class Property\na: " + a.myclass.property1 + "\nb: " + b.myclass.property1 + "\n\nNormal Property\na: " + a.test + "\nb: " + b.test); b.myclass.property1 = "ghi"; b.test = "def"; alert("class Property\na: " + a.myclass.property1 + "\nb: " + b.myclass.property1 + "\n\nNormal Property\na: " + a.test + "\nb: " + b.test);

the test property correctly has two different values after I've set one, but the prop1 of myclass always mirror each other.

I've tested this in jquery 1.4.2 and 1.6.2 in Firefox 3.6.17, IE 8, and Chrome 13

[Rick Waldron]

Thanks for taking the time to contribute to the jQuery project! Please provide a complete reduced test case on jsFiddle to help us assess your ticket!

Additionally, be sure to test against the jQuery Edge version to ensure the issue still exists. To get you started, use this boilerplate: ​http://jsfiddle.net/FrKyN/ Open the link and click to "Fork" (in the top menu) to get started.

[trac-o-bot]

Because we get so many tickets, we often need to return them to the initial reporter for more information. If that person does not reply within 14 days, the ticket will automatically be closed, and that has happened in this case. If you still are interested in pursuing this issue, feel free to add a comment with the requested information and we will be happy to reopen the ticket if it is still valid. Thanks!

[Rick Waldron]

#11557 is a duplicate of this ticket.

[Rick Waldron]

Reopening per #11557

[gibson042]

This strikes me as all kinds of trouble à la #8653 and #11002. The ​test case linked to by #11557 is in fact confirming that custom objects are copied by reference rather than recursively extended.

Deep extending of non-plain objects is better left to custom code, unless we want an endless stream of tickets about which properties/methods get copied, whether a created copy should be a plain object or new class instance, what to do to about prototypes, and so on ad infinitum.

[dmethvin]

Amen, brother gibson042. Even ECMA punted on cloning arbitrary constructed objects.

[Rick Waldron]

After looking through our existing tests and the code cost this might incur, I agree 100% with gibson042 and dmethvin

There is a difference between an instance object, created by a constructor, and plain objects. The key part of the issue is this "whether a created copy should be a plain object or new class instance, what to do to about prototypes, and so on ad infinitum." from gibson042

[[email protected]…]

I suggest at least making this behaviour (limitation) explicit in the docs (​http://api.jquery.com/jQuery.extend/)

[dmethvin]

#12105 is a duplicate of this ticket.

[[email protected]…]

How about custom class instances would need to have a defined 'deepCopy' hook method which jQuery would look for and replace with whatever is returned by that method, then we can keep the logic of what to do on a copy in within the class.