Preserve object prototypes

[rtfeldman]

Currently, if you pass an object with a prototype to Immutable, that prototype is discarded. It seems reasonable to preserve that prototype instead, with the understanding that not all of the methods it exposes will necessarily still work.

This would address #24

[crudh]

I'm no expert at this but as far as I can tell something like this could work:

In the Immutable function, the new object is created with:
var clone = {};

Instead the following can be done:
var clone = Object.create(Object.getPrototypeOf(obj);

But that won't work for Date, and as far as I can tell Date would require a special case in the Immutable function. So combined we would get:

...
} else if (obj instanceof Date) {
  var clone = new Date(obj);
  return makeImmutable(clone)
} else {
  var clone = Object.create(Object.getPrototypeOf(obj);
  ...

But Object.create is a lot slower than {}. This makes it faster:

var prototype = Object.getPrototypeOf(obj);
var clone = prototype ? Object.create(prototype) : {};

A bit slower in Chrome but still much slower in Firefox (but mainly since FF is much faster at doing {}). Source

But I don't know how much the performance of this single thing affects the entire process of making an object immutable.

So am I on the right track? And is there a faster/better way of checking for a prototype?

[rtfeldman]

Upon further investigation, there are pitfalls to the prototype-only approach. For example, Date (the motivating example, no less!) won't actually work as intended using this approach, because of how Date stores its internal data.

In light of that, I'm leaning toward this not being a good idea after all. It seems like it could lead to surprising bugs, where you can call most methods on most prototypes and have everything work as expected, but good luck tracking down the first bug that comes up because a prototype method relied on a non-enumerable hidden member...

[crudh]

Yes, that sounds reasonable.

But instead of a generic solution for prototypes maybe a special case for Date handling could be useful? Since it is an built in type that is frequently used, and by using the new Date(obj) to clone it there shouldn't be any technical problem?

[rtfeldman]

That seems totally reasonable, yeah. We already do an instanceof check for Array, so we could certainly do another for Date and handle as appropriate.

[rtfeldman]

Resolved by #31

[dariocravero]

Hi all,

I understand the need to keep things as tight as possible but this is a big issue when you're trying to use custom types that use the prototype.

For instance, I'm building a lookup mechanism on top of route name matcher called Houkou and I would like to keep the object of patterns immutable.

However, the way I see it, whatever happens within the Houkou object is its responsibility.
The problem with the current approach is that every Houkou object I instantiate gets its prototype stripped away. And not only its own prototype but those of its properties are gone too. So, for instance, I'm loosing validate, match and deeper in the structure its regex's methods.

Even though I can assign these myself when creating my Houkou instance, it quickly becomes quite unmanageable when you have to track every dependency on the library that you're using for methods that need to be rewired... :(

I understand the goals of seamless-immutable. Probably a solution in this case would be to deal with RegExp objects in the same way #31 deals with Date objects. A simpler approach would be to allow an immutable object to be created up to certain level or to exclude certain objects.

What are your thoughts on this?
Thanks,
Darío

[rtfeldman]

The core issue with prototypes is #25 (comment) - specifically, that there's no way to detect whether prototypes are relying on hidden properties, which cannot be discovered and cloned. The resulting bugs could have really nasty symptoms to identify; things as vague as "the cloned version doesn't quite work the same way as the original for some reason."

I could see offering a workaround similar to custom mergers, where you could pass a custom cloning function as an argument to Immutable(), and within that do whatever you like (at your own risk, of course) with regards to prototype and anything else.

Then if you wanted that prototype behavior everywhere you could just set Immutable to be some function that wraps the original Immutable() with a call that always passes your custom cloning function.

Thoughts?

[dariocravero]

@rtfeldman I'm sorry I haven't come back to you on this before.
I think that might be a good starting point. That would at least allow us to whitelist more complex objects at will.

[rtfeldman]

Cool. This is admittedly not a high priority for me in terms of implementation, but I'm on board with the approach! 😃

[dariocravero]

Good stuff! Don't worry, I'll see if I can give it a go at some stage. As a
matter of fact, this constrain made me reevaluate my data structures and
allowed me to simplify things big time, so it was already a win :)

On Wed, 24 Jun 2015 22:58 Richard Feldman notifications@github.com wrote:

Cool. This is admittedly not a high priority for me in terms of
implementation, but I'm on board with the approach! [image: 😃]

—
Reply to this email directly or view it on GitHub
#25 (comment)
.