#10014 closed bug (wontfix)
jQuery Extend does not properly perform a deep copy on an instance of a class
Reported by: | anonymous | Owned by: | Rick Waldron |
---|---|---|---|
Priority: | high | Milestone: | 1.next |
Component: | core | Version: | 1.6.2 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
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
Change History (11)
comment:1 Changed 12 years ago by
Component: | unfiled → core |
---|---|
Owner: | set to anonymous |
Status: | new → pending |
comment:2 Changed 11 years ago by
Resolution: | → invalid |
---|---|
Status: | pending → closed |
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!
comment:4 Changed 11 years ago by
Milestone: | None → 1.next |
---|---|
Priority: | undecided → high |
Resolution: | invalid |
Status: | closed → reopened |
Reopening per #11557
comment:5 Changed 11 years ago by
Owner: | changed from anonymous to Rick Waldron |
---|---|
Status: | reopened → assigned |
comment:6 Changed 11 years ago by
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 extend
ing 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.
comment:7 Changed 11 years ago by
Amen, brother gibson042. Even ECMA punted on cloning arbitrary constructed objects.
comment:8 Changed 11 years ago by
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
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
comment:9 Changed 11 years ago by
I suggest at least making this behaviour (limitation) explicit in the docs (http://api.jquery.com/jQuery.extend/)
comment:11 Changed 10 years ago by
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.
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.