rtfeldman / seamless-immutable

Immutable data structures for JavaScript which are backwards-compatible with normal JS Arrays and Objects.
BSD 3-Clause "New" or "Revised" License
5.37k stars 195 forks source link

Saving prototypes of deep objects #124

Closed ghost closed 8 years ago

ghost commented 8 years ago

Can you make option for saving prototypes of deep objects? It can be made like that. But one test fails.

      for (var key in obj) {
        if (Object.getOwnPropertyDescriptor(obj, key)) {
          var
            objKey        = obj[key],
            prototype     = objKey instanceof Object ?
              objKey.constructor.prototype : null;
          //
          clone[key] = Immutable(objKey, { prototype : prototype },
            stackRemaining);
        }
      }

Fixed: now fails only one test.

forty2 commented 8 years ago

I would also find this useful, so I took a shot at it over here: https://github.com/forty2/seamless-immutable/tree/child-prototypes

It seems to be working for me, and all tests pass (including the one I added for the new behavior).

If @rtfeldman agrees this is a good idea, I'll open a pull request.

Edit: it's more difficult than I originally thought -- it doesn't do the right thing after calling set. I'll see what I can do about that...

Edit 2: After a few hours of playing with this, I'm no longer convinced it's a good idea. So, yeah, basically pretend I wasn't here... :wink:

rtfeldman commented 8 years ago

This seems like a "here there be dragons" case. What's the motivation here? Why is this needed?

ghost commented 8 years ago

I develop a simulation library. It uses a single state to perform simulation and a set of saved immutables states. So it will need two features.

  1. Call methods of immutable objects and its properties to analize saved states.
  2. Make saved state mutable to perform simulations with it.

Plus there is non-obvious behavior:

  class A {}
  class B {
    constructor(){
      this._prop = new A;
    }
  }
  let b       = new B,
      bClone  = Immutable(b).asMutable(),
      bPropConstr       = b._prop.constructor,
      bClonePropConstr  = bClone._prop.constructor,
      equal   = bPropConstr === bClonePropConstr;
  console.log(equal); // false
rtfeldman commented 8 years ago

Call methods of immutable objects and its properties to analize saved states.

Why do those need to be methods? Why not implement them as standalone functions that accept the immutable data as arguments?

ghost commented 8 years ago

Why do those need to be methods? Why not implement them as standalone functions that accept the immutable data as arguments?

These methods can be not pure, use another methods of the object and methods from parent class etc. By skipping prototypes all inner logic turns off. So we should work with object like with json and not like with class instance.

rtfeldman commented 8 years ago

Sorry, but it seems like the right answer here is to stick to using Immutable values as plain old data, which makes great sense to me. I'm gonna close this.

Thank you for the discussion!