Immutable.js like Records about seamless-immutable

[rtfeldman]

We talked about adding a way to support opting in but it's very low on my priority list at the moment. 😃

If you wanted to make a Pull Request for that, though, I would definitely be open to it!

[virajsanghvi]

I'm likely going to try this approach this weekend, but there's some weirdness in that multiple arguments to Immutable() are considered an Array and I'd rather not break existing consumers. Another option I'm thinking about is a special method that does this on Immutable like isMutable, but that's also not ideal.

Because my options for implementation of this feature are subpar, what do you think about supporting the original intent of #25, but logging a warning when encountering a prototype in development? I'm guessing there are probably a lot of folks that rely on this not happening at the moment? If so, changing that behavior would be breaking, so this probably isn't feasible either, but thought I'd ask.

[rtfeldman]

Yeah, I don't like the idea of breaking current invariants for this.

However, I could see adding a method like this:

// Takes an array of allowed prototypes, and returns a new version of `Immutable`
// which will preserve those prototypes when it encounters them.
Immutable.withPreservedPrototypes

Then you could just go:

var Immutable = require("seamless-immutable").withPreservedPrototypes([ ... ]);

[virajsanghvi]

My main issues with that approach are:

1. If someone created a project specific version of seamless-immutable with this approach, they'd have to keep that list up to date
2. If someone was encapsulating the creation of these objects, it gets tricky if another component creates other Immutable objects where these are nested children. (This is probably unavoidable)

How would you feel about something like:

// Similar to Immutable(), but during clone of objects, attaches a __immutable_preserve_prototype 
// tag to the cloned object. The application of this tag is used elsewhere in the library to ensure 
// the prototype is preserved on clones.
Immutable.withPreservedPrototype(obj) // could be better named

If the entire library recognized this tag and acted appropriately, you could use Immutable normally, wrap objects in arrays, and get the expected result. This handles #1, and mostly #2 (having to have factories to create Immutable objects of a specific type is required in either case).

Anyways, let me know what you think of that. I'm not completely sold on it, but feels easiest to consume and implement. Happy to hear out other options. Will try to implement one of these in the next day.

[rtfeldman]

Hm...what would a code example look like with this in action?

[virajsanghvi]

Sorry for the late response. I just tried out something similar to what I'm thinking: virajsanghvi@5e2b065

Just wanted to be on the same page before cleaning this up/adding tests for PR. See the test for a general idea of how it would be used.

Basically the idea is a custom constructor method used explicitly for the purpose of preserving the prototype. We can use the invariant that all existing Immutable objects have Object.prototype to determine when we've encountered an object created from this custom constructor, and preserve its prototype during cloning.

There are three main points of contention that I can see:

• Is the performance implication of Object.getPrototypeOf ok in the common case? I imagine it just returns __proto__, but not sure.
• What is the relation of ImmutableWithPrototype to Immutable? Should they share a common constructor, so you could pass Arrays, dates, etc to ImmutableWithPrototype, or should it just take in Objects and throw otherwise? (The current implementation of copying Immutable was just to try this out)
• The name ImmutableWithPrototype kind of sucks- I'm open to suggestions.

Anyways, let me know what you think. Also, if you have a benchmark suite I could use, I'd like to see the overall perf implications of Object.getPrototypeOf and Object.create.

[rtfeldman]

I think a cleaner approach would be:

1. Have Immutable accept a second argument for options, like merge does (this would be a breaking change, as currently it assumes if it gets multiple arguments you want an array; I'm fine with that breaking hange)
2. If it gets a second argument of {prototype: foo} then use foo as the prototype for the resulting object.
3. Add a method instantiateEmptyObject() (in the same way the other custom methods are added) and have it either return {} if no prototype was specified, and Object.create(options.prototype) if one was specified.
4. Have the various methods call this.instantiateEmptyObject() instead of just using {}

Thoughts?

[virajsanghvi]

I'm a fan. This was preferable, but I thought you were against changing the interface for Immutable per a previous comment (I may have described it unclearly). Anyways, I'll get this up in the next few days when I have a free hour.

[rtfeldman]

To clarify, what I meant by not wanting to break current invariants is just that there'd be no chance any existing uses of Immutable would suddenly "play by different rules" and possibly introduce surprising bugs.

I'm okay with a breaking API change, though, especially for something that I suspect is not used very often. 😃

[srolel]

Is this a priority? I would love to have this, could make a PR.

[virajsanghvi]

Forgot to link here, but submitted #51 a couple days ago which handles what we talked about above.

[rtfeldman]

Thanks! I saw it but haven't had time to take a proper look because it's crunch time at work and I'm swamped.

I'll definitely try to take a look this week!

[virajsanghvi]

Closing as the functionality to support this is now available in 3.0.0