Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

R.clone: deep copy including constructor to use instanceof #2783

Open
exsoflowe opened this issue Feb 5, 2019 · 7 comments
Open

R.clone: deep copy including constructor to use instanceof #2783

exsoflowe opened this issue Feb 5, 2019 · 7 comments

Comments

@exsoflowe
Copy link

exsoflowe commented Feb 5, 2019

Is there possibility in Ramda to copy object to save constructor and use instanceof?

For example lodash has cloneDeep which returns true for instanceof:

class Point {
  constructor(title) {
    this.title = title;
  }
}
const source = new Point('example');
const copy = _.cloneDeep(source);
console.log(copy instanceof Point); // true

Ramda clone returns false for instanceof:

class Point {
  constructor(title) {
    this.title = title;
  }
}
const source = new Point('example');
const copy = R.clone(source);
console.log(copy instanceof Point); // false

R.map(R.clone, source) doesn't work.

Thanks.

@exsoflowe exsoflowe changed the title R.clone: deep copy include constructor to use instanceof R.clone: deep copy including constructor to use instanceof Feb 6, 2019
@CrossEye
Copy link
Member

CrossEye commented Feb 6, 2019

For reasons I discussed last week (#2775 (comment)), I am not much interested in this. Note that the same example used there would have the similar issues with lodash's cloneDeep as well.

Lodash is a general-purpose library, meant to help with any of your programming needs. Ramda is more specialized, focused strictly on making it easier to use some of the techniques found in Functional Programming languages. While we have done some small work over the years to work with OOP, it has never been central, and I think we're slowly removing that. It just doesn't fit with the design goals.

@danielesegato
Copy link

@CrossEye sorry but your argument in #2775 (comment) is a bit narrow minded.

Of course you can't clone deep in ALL situation, but to just create a copy that completely ignore everything from ES6 (the last 7 years of development of the language) just because you cannot guarantee it will create a perfect copy 100% of the times you instead decide to basically NEVER create an actual copy for whoever is using features introduced 7 years ago.

No one doing something like you described in #2775 (comment) would be surprised that a library cannot deep clone the object they created once you show him that you just cannot.

I also completely reject the argument of flexibility: flexibility means I should be able to use language features, which include OOP since 7 years ago.

Nowhere I saw in your homepage readme something that said "hey we do not support ES6 and forward", which is why I'm here posting this.

@CrossEye
Copy link
Member

@danielsegato:

This really has nothing to do with ES6. Classes introduced there are mostly syntactic sugar atop the existing prototypal inheritance model. The problem I was trying to describe is that objects can be complex. ES6 classes don't change this fundamental complexity. Moreover, there are many other viable ways in JS to make objects.

No one doing something like you described in #2775 (comment) would be surprised that a library cannot deep clone the object they created.

But do they know all the object creation details of every function in every library they use, of the code from every teammate on their project and from sister teams deep within the bowels of their large company? Probably not. If someone else on their project created this:

const ConstrainedRectangle = (w, h) => {
  const ratio = h / w // todo: deal with division by zero
  return {
    get width () {return w},
    set width (width) {w = width; h = width * ratio; return this;},
    get height () {return h},
    set height (height) {h = height; w = height / ratio; return this;},
    area () {return h * w},
    perimeter () {return 2 * h + 2 * w}
  } 
}

const r1 = ConstrainedRectangle (5, 3) 
console .log (r1 .area ())  //=> 15  (5 * 3)
r1.height = 6
console .log (r1 .area ())  //=> 60  (10 * 6)

and your code cloned such rectangles, the clones would not properly capture the constraining ratio.

And if there's a real possibility that the user doesn't know such things, then it's far worse for Ramda, which knows nothing at all about your construction techniques. This is the real worry. If we start down that rabbit hole, where do we stop?

That is not to say I'm entirely opposed to going further, to do what we can with the prototype chain. But it would have to come with few downsides, as it doesn't fundamentally solve the underlying problem.

Yes, lodash has its solution to this. It won't fix the example above or those described in the linked issue, but it does offer some additional capability. Should Ramda do the same? Certainly not just because lodash does. lodash is designed to be a general-purpose utility library. Ramda focuses more narrowly on FP concerns. Although Ramda in its early days tried to help bridge the OOP/FP gap, we've mostly given up on that. Only very old functions even attempt to keep the this context.

So what lodash does is not much of a motivation.

But there is also absolutely nothing to prevent users to pick and choose functions they want from different libraries. Just because one is using Ramda in a project, there is no reason for her not to also use lodash.clone.

I also completely reject the argument of flexibility: flexibility means I should be able to use language features, which include OOP since 7 years ago.

The point about flexibility was not that Ramda was flexible but that the underlying language techniques were so varied that we could never be able to cover them all.

Nowhere I saw in your homepage readme something that said "hey we do not support ES6 and forward", which is why I'm here posting this.

Again, this is not about ES6. This is about FP versus OOP. Ramda's functions support simple data structures, but we don't try to extend this to more complex ones involving local closures or the prototype chain.

Think for a minute about assoc. We could very easily write a version of assoc like this:

const assoc = curry ((name, value, object) => {
  const newObj = deepClone (object)
  newObj [name] = value
  return newObj
})

If we were to fix clone to handle prototypes, then presumably users would expect assoc to have the same behavior. We could do that, but we would break another contract that assoc offers: structural sharing, that is, for instance, assoc ('a', val, obj) .b is reference-equal to obj .b. This is important in FP to avoid bloating memory usage. Would we have to give up that guarantee?

It's these sorts of concerns that make us able to mostly punt on any further OOP integration.

However, if you feel strongly about this and want to create a PR for it, I can promise that we'll give it serious consideration.

@danielesegato
Copy link

@CrossEye thank you for your answer.

I completely understand the added complexity of handling classes.

If we start down that rabbit hole, where do we stop?

I believe it should be clearly stated in the homepage where do you stop.
Right now it's plain objects. And I'd have love to find it in the README of this project something along the line of "Rambda doesn't support classes".

I believe immutability is a must for any functional programming library and thus you can set some constraints like that classes must not be mutable and explicitly state that you are not gonna support mutations.

I was really surprised when an immutable class I had become a plain object with a simple clone.

I firmly believe that if you take a strong stance like this in a library it should be explained in the library homepage, or at least briefly mentioned.

However, if you feel strongly about this and want to create a PR for it, I can promise that we'll give it serious consideration.

I wouldn't be able to do this. But kudos for the proposal.

@CrossEye
Copy link
Member

@danielesegato:

I firmly believe that if you take a strong stance like this in a library it should be explained in the library homepage, or at least briefly mentioned.

If you have some suggested wording for this, please share it, either here or in a PR for Ramda.md. I certainly have no objection to including such a warning, but I'm not really sure what to say or whether it belongs in the README or in clone documentation or in some other place.

@danielesegato
Copy link

@CrossEye

If you have some suggested wording for this, please share it,

something along the line of:

If you use classes or other prototypal inheritance models (OOP) be aware that Ramda does not support it. You'll have to serialize and deserialize your classes before and after using Ramda functions. You can read more information on the rationale for this design decision here and here. (links you think better describe the reasons for your design decision)

I'm sure you can come up with a better wording for the same concepts. I'm relatively new to the javascript / typescript world.

@CrossEye
Copy link
Member

CrossEye commented Mar 1, 2021

@danielesegato

If you use classes or other prototypal inheritance models (OOP) be aware that Ramda does not support it. You'll have to serialize and deserialize your classes before and after using Ramda functions.

The trouble is that this isn't really true. You can use many Ramda functions -- probably most -- with prototypal objects. It's those that return altered versions of structures, things like assoc, assocPath, evolve, set, and over, and of course clone, that have this restriction. That's why I have a hard time seeing this as a README change, although if we can find an appropriate warning, this would be a good place for it.

Perhaps what we need is a note in the relevant functions of something more like:

This function does not maintain the prototype chain of input objects.

You also have gotten me thinking about how much it would take to maintain the chain better than we do currently. I will take some time to look into it, but I'm not particularly hopeful that it will be worth doing in Ramda, as Ramda's goals are to help with FP programming and not to be a multi-paradigm utility library. Still I will try to spend some time this week.

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

No branches or pull requests

4 participants