R.cloneDeep is more like "make JSON-safe" about ramda HOT 10 CLOSED

i did a bit of research on this problem. conclusion: there is not a good solution for cloning an object in javascript. there are a number of techniques that make different compromises. For our impl, I think we want:

1. detect/avoid cycles. This rules out JSON approach. But without weak maps, this makes the cloning algorithm a nasty O(n^2). With weak maps, could get O(n log n) I believe, which is a big difference.
2. include functions (this again rules out JSON approach)
3. copy Date objects

after that, there's a lot of squishiness. This problem really brings the limitations of the language into relief.

1. what about custom constructed objects? re-construct them on the copy?
2. what about primitive wrappers (e.g. Boolean, Number, Striung, etc.)? re-construct them?
3. what about prototype properties? ignore?
4. .... more ....

Conclusion: I am going to defer crossing this Rubicon until I am pushed into it. If anyone wants to suggest an implementation, or even submit a pull request with a candidate implementation, I am all ears.

from ramda.

removed cloneDeep.

from ramda.

We'll think about this a bit.

I don't think it's possible for us to use the structured cloning algorithm directly. That's for language implementors, and depends upon having access to internal Object implementation details.

Ramda has so far been mostly used for working with data objects, so this hasn't been an issue. I can see how this might be a problem. Are there any particular deep-clone-in-JS implementations you particularly admire that we could look to for inspiration should we choose to go in that direction?

from ramda.

I was thinking more like "follows the structured cloning algorithm where possible". e.g. allow Date, RegExp, etc.

http://lodash.com/docs#cloneDeep should offer some ideas.

While a new R.cloneDeep behavior would be nice, it's most important the behavior is accurately detailed in the docs. If it's using JSON, the docs should include examples illustrating the omission of certain properties.

from ramda.

mea culpa. I put this quick-and-dirty impl in there to support a little project i was toying with. I didn't want to write a type-checking object traversal so I just slapped this in there. I agree with @CrossEye, it would be nice to use an existing deep copy implementation.

from ramda.

@buzzdecafe @CrossEye No worries, I totally understand. "Make it Work. Make it Right. Make it Fast.", "YAGNI", etc, etc.

I just added this to capture the primary issue that the docs ("Create a deep copy of an object") don't match the behavior. There are many ways to resolve it; I just didn't want the issue to get lost.

from ramda.

@jfsiii understood, and agreed.

from ramda.

I totally agree. That's why I jumped to see how Ramda chose to implement it. Choice 1, rename the function, still remains an option.

As long as the docs match the behavior, I'm fine with closing this.

from ramda.

Deep cloning is something everyone does a little differently, so I think it makes sense to leave it out of the library.

I'd be in favor of renaming or removing the function.

from ramda.

@saikobee, i think removing cloneDeep may be the smartest move.

from ramda.