Make can.extend's deep copy clone CanJS classes #311

Closed
ccummings opened this Issue Mar 13, 2013 · 6 comments

Comments

Projects
None yet
5 participants
@ccummings
Contributor

ccummings commented Mar 13, 2013

If you use can.extend to perform a deep copy, CanJS classes like Observe, Observe.List and compute are not cloned. Instead you get the same class back:

var orig = {
    obs: new can.Observe({id:1})
},
copy = can.extend(true, {}, orig);

console.log(orig.obs._cid) //-> ".observe1"
console.log(copy.obs._cid) //-> ".observe1"

can.extend should invoke a clone() method on each property it is copying (if it exists).
Subsequently, each CanJS class would need a clone method that will produce a deep copy of itself.

@ccummings

This comment has been minimized.

Show comment
Hide comment
@ccummings

ccummings Jul 18, 2013

Contributor

The pull #435 contains a .clone() for can.Observe. Good starting point for this.

Contributor

ccummings commented Jul 18, 2013

The pull #435 contains a .clone() for can.Observe. Good starting point for this.

@airhadoken

This comment has been minimized.

Show comment
Hide comment
@airhadoken

airhadoken Sep 10, 2013

Contributor

The clone() function in #435 will work in the place of can.extend as long as you don't expect instances of the same classes -- the library creates a Clone subclass of each class to add the merge() operation. Just pass true as an argument to Observe.prototype.clone to effect a deep copy.

Contributor

airhadoken commented Sep 10, 2013

The clone() function in #435 will work in the place of can.extend as long as you don't expect instances of the same classes -- the library creates a Clone subclass of each class to add the merge() operation. Just pass true as an argument to Observe.prototype.clone to effect a deep copy.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Sep 20, 2013

Contributor

can.extend(true, ...) doesn't exist in can already right? We don't support deep extend in all libraries?

Contributor

justinbmeyer commented Sep 20, 2013

can.extend(true, ...) doesn't exist in can already right? We don't support deep extend in all libraries?

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Sep 24, 2013

Contributor

Looks like this works only really for jQuery. We should probably make this consistent for all libraries.

Contributor

daffl commented Sep 24, 2013

Looks like this works only really for jQuery. We should probably make this consistent for all libraries.

@ghost ghost assigned ccummings Sep 27, 2013

@nottoseethesun

This comment has been minimized.

Show comment
Hide comment
@nottoseethesun

nottoseethesun Jan 21, 2014

+1 on daffl's comment ( #311 (comment) ).

+1 on daffl's comment ( #311 (comment) ).

@ccummings

This comment has been minimized.

Show comment
Hide comment
@ccummings

ccummings Apr 28, 2014

Contributor

Closing this since we won't be adding this feature to Control. Component will have this behavior through the use of the define plugin landing in 2.1. There will be a dev warning added as part of #704.

Contributor

ccummings commented Apr 28, 2014

Closing this since we won't be adding this feature to Control. Component will have this behavior through the use of the define plugin landing in 2.1. There will be a dev warning added as part of #704.

@ccummings ccummings closed this Apr 28, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment