Bug Tracker

Opened 8 years ago

Closed 7 years ago

Last modified 6 years ago

#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 8 years ago by Rick Waldron

Component: unfiledcore
Owner: set to anonymous
Status: newpending

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.

comment:2 Changed 7 years ago by trac-o-bot

Resolution: invalid
Status: pendingclosed

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:3 Changed 7 years ago by Rick Waldron

#11557 is a duplicate of this ticket.

comment:4 Changed 7 years ago by Rick Waldron

Milestone: None1.next
Priority: undecidedhigh
Resolution: invalid
Status: closedreopened

Reopening per #11557

comment:5 Changed 7 years ago by Rick Waldron

Owner: changed from anonymous to Rick Waldron
Status: reopenedassigned

comment:6 Changed 7 years ago by 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.

comment:7 Changed 7 years ago by dmethvin

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

comment:8 Changed 7 years ago by Rick Waldron

Resolution: wontfix
Status: assignedclosed

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 7 years ago by wachunga@…

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

comment:10 Changed 7 years ago by dmethvin

#12105 is a duplicate of this ticket.

comment:11 Changed 6 years ago by ian@…

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.

Note: See TracTickets for help on using tickets.