Immer: Idea: introduce limited support for immutable, cloneable classes

[mweststrate]

We could introduce limited support for classes, by preserving the prototype of an object when cloning objects. (See als the comments here)

There are a few potential gotchas however

• Not all data structures can safely be cloned by just preserving the prototype (e.g. most built-in types like, Map, Buffer, HTMLElement etc etc)
  
  
  
  • So, cloneable classes should be marked specially, like class X extends ImmerCloneable or X[immerCloneable] = true
  
  
• It becomes more unclear what kind of data structures can be cloned, and what not
• Object.create needs polyfilling?

[oriSomething]

Wouldn't be better to add the ability of plugins to support whatever data structure you like instead of expanding support of immer?

[yamsellem]

@oriSomething I've worked on a Javascript class encoding library lately, carnaval, which uses some concept of custom encoding, middlewares and providers; maybe it could help on this idea of 'expanding support'.

It's basic usage is to encode a class into JSON, or decode JSON into a class, but there is some manual configuration which can be provided to handle special cases like embedded class / arrays, momentjs objects, and so on.

I don't know if it will help, but I like the idea you shared about 'expanding' immer, and it made me think of that. Take care.

[be5invis]

Maybe we could handle it in three types:

• Class provided shallow copy
• Class provided deep copy
• The class said that "properties copy + keeping prototype" is OK (POD-like)

[benjamingr]

+1 for plugins

[aleclarson]

The new Immer class added in v1.9.0 is the perfect spot for customizing this behavior. And hopefully it wouldn't be too hard to override the typings if you're a Flow or TypeScript user.

I think Immer should support classes by default. Here's how:

• a unique symbol is exported which Immer-compatible classes must have in their prototype
• class constructors are _never_ re-invoked on copy
• prototypes are _never_ proxied

Beyond those details, these class instances would be treated identically to plain objects. This means deep mutations, object recycling, auto-freezing, and ES5 support are all included.

Side note: Just like plain objects, non-enumerable properties will _not_ be supported. Likewise, binding functions to these class instances is off limits.

If anyone has objections, please say so. I will be implementing this in the near future.

[mweststrate]

When thinking about Immer, and what belongs to the scope of immer, we can split all concepts in two domains: Thinks that are considered to be part of a 'state tree', or, handled by immer, and things that arent. (Note that this the criterium isn't immutability or not, functions for example are mutable)

Not "immerable"

• Functions
• Function scopes
• All primitives
• Prototypes
• All class instances (objects with a prototype)
• Non-numeric properties on arrays

Immerable

• Any plain object members
• Numeric (or number like) indices on arrays

---

To move class instances from non-immerable to immerable, there are a few options, first one: preserve prototype, further consider as plain object

1. Support creating drafts for class instances, upon draft, create a new object with the same prototype
2. Copy over all _own_ members (I don't enumerable or not shouldn't have any influence on this; non-enumerable properties can still capture state or be assigned etc)

The problem is, that it doesn't work for the most simple of examples: for example a Map object is not cloneable this way (Object.getOwnPropertyNames(new Map) is empty, the state of a Map is not captured in properties)

Further, the amount of edge cases is unending. For example, what if your object is supposed to be registered somewhere in the constructor? It will break.

Or even more common: binds a function in its constructor? (maybe even accidentally, by using x = () => {}). In that case, in the clone, the functions are bound to the original object, not to the clone. Imagine how hard that is to debug, your stepping into a function and suddenly you are working with an other object, but even in the debugger that is really hard to observe if you aren't actively looking for it!....

So the big amount of possibilities in which you can mess this up, and the amount of rules that have you have to impose, make classes support in immer imho a small minefield, that can be avoided by one much more simple rule: _If you want to build an immutable tree, don't use classes, (which sole purpose is to encapsulate state from the outside world so that the class, and the class only has control over what happens to it's internal state, and what parts of it are revealed)_.

---

The second solution, a custom clone symbol, make this a bit less worse, at at least things aren't cloned by default and it gives back the responsibility to the class. Still some concerns though:

1. It still doesn't solve the Map case (unless built-in Map is patched, yikes)
2. I would be interested to see to have an example of a more complex class implementing this, because as I can see, it isn't a matter of just cloning. That in itself is not enough for Immer to do it's job; if you want to prevent full deepclones (yikes as well), the clone (or draft) has to take apply the full immer logic of tracking dirtiness of relevant pieces of state, replacing fields in there parent as soon as they are modified, etc. (And ideally, patches proper patches should be produced as well). So I think this approach might be more involved than it seems at first sight, but I am curious to see some worked out examples of how such logic should look like.

[aleclarson]

We _could_ add an Immer option that lets users decide if prototypes are proxied. But honestly, that sounds like "bad practice" to be mutating prototypes.

[aleclarson]

@mweststrate I think it'll be easier to implement than you expect. Nice write-up! 👏

Also, I'd be fine with supporting non-enumerable properties. Plain objects also lack support for non-enumerable properties at the moment, so I was rolling with that.

[be5invis]

@mweststrate I may support a shallow interface like this:

interface ImmerShallowCloneable<A> {
    [ImmerCloneSymbol](mapper: <T>(from:T) => T): A
}

Then when performing clone, pass Immer's logic into this method.

[aleclarson]

Or even more common: binds a function in its constructor? (maybe even accidentally, by using x = () => {}). In that case, in the clone, the functions are bound to the original object, not to the clone. Imagine how hard that is to debug, your stepping into a function and suddenly you are working with an other object, but even in the debugger that is really hard to observe if you aren't actively looking for it!....

@mweststrate That's already off limits in Immer. You __cannot__ currently bind a method to its plain object and pass it into a produce function. And I mentioned this in my proposal. 😄

[aleclarson]

interface ImmerShallowCloneable {
    [ImmerCloneSymbol](mapper: <T>(from:T) => T) {}
}

@be5invis I could see that being useful for re-binding functions. What else would you use it for? 🤔

[be5invis]

@aleclarson
Many possible ways. And it is just a restricted form of functor.

[aleclarson]

So the big amount of possibilities in which you can mess this up, and the amount of rules that have you have to impose, make classes support in immer imho a small minefield, that can be avoided by one much more simple rule: _If you want to build an immutable tree, don't use classes, (which sole purpose is to encapsulate state from the outside world so that the class, and the class only has control over what happens to it's internal state, and what parts of it are revealed)._

@mweststrate It's up to developers to evaluate the trade-offs between using classes and not using them. Prototypes are just so damn useful. :)

[aleclarson]

@be5invis I just realized.. Your idea can be implemented with the current version! 🤯

import {Immer} from 'immer'

export const ON_COPY = Symbol()

const {produce} = new Immer({
  // This function is called for _every_ changed object in the state tree.
  onCopy(state) {
    if (state.base[ON_COPY]) {
      state.copy = state.base[ON_COPY](state.copy)
    }
  }
})

export { produce }

The state.base object is the previous state.

The state.copy object is the next state.

Just need class support. 😊

Further, the amount of edge cases is unendless. For example, what if your object is supposed to be registered somewhere in the constructor? It will break.

@mweststrate This could be a workaround for that edge case. But in most cases, __not__ calling the constructor on copy is the expected behavior, IMO.

[mweststrate]

And I mentioned this in my proposal. smile

Yeah I didn't miss that! Just added it to make the comment more independently readable. Saying "out of scope", is actually not that easy. I am afraid that people first and foremost read "classes are supported". Then run into trouble, then waste some time, and only then read our carefully crafted readme that already warned that their (super common) use case is not supported. From experience, aiming for the least amount of surprise is super important when designing an API / library, even when this imposes restrictions it can be valuable.

Prototypes are just so damn useful. :)

Given all the limitations that classes + immer would have, I think that in practice everything that is put on the prototype, could in practice just be a static module function as well. (And don't get me wrong, I love encapsulating using prototypes, I wrote MobX to make that happen, my fear is mostly around the confusion that would be caused by the half-half classes we would be forcing to be created)

To call or not call constructors is quite a topic, take for example this:

class User {
   name,
   profile,
   age = 10

   constructor(name) {
       this.name = name
       this.fetchProfile("users/"+ name).then(v => {
          this.profile = v
       }
    }
}

Now imagine an instance of this object somewhere in your tree. And the lifecycle of this object is roughly:

1. We create an instance michel = new User("michel") and put it somewhere in our tree
2. We start an immer producer to update the age of michel (which now lives somewhere in the middle of our tree)
3. The promise resolves

Now we can observe the following behavior during step 2:

1. We can clone the User, and handle the name and agefield probably correctly
2. We don't run the constructor. That is fine. But, the fetchProfile of the "original" user instance is still running! It will update the "old" user, but those changes will never arrive in the _new_ user object that was produced by our producer
3. Or, if we would run the constructor, then the changes would arrive, but we would be fetching the profile twice!

So these kind of trouble are what make me doubtful about supporting classes. Because in practice, it means that class instances should be immutable. (Ok, well, that is why we are using immer, but naturally, classes are mutable concepts, that encapsulate state).

So I think _at least_ we should narrow the scope to: _immutable classes with a prototype_, which is not the same thing as classes in general. (Maybe this was the intention the whole time, but at least calling it out now ;-))

[aleclarson]

The second solution, a custom clone symbol, make this a bit less worse, at at least things aren't cloned by default and it gives back the responsibility to the class.

Yeah, I love the symbol approach! You can target a class or a single instance. 🔥

It still doesn't solve the Map case (unless built-in Map is patched, yikes)

I think built-in types like Map/Set/Buffer/WeakMap are better suited as "polyfill" packages that plug into Immer via the Immer class. But I could be under-estimating their importance, so we'll see.

Given all the limitations that classes + immer would have, I think that in practice everything that is put on the prototype, could in practice just be a static module function as well. (And don't get me wrong, I love encapsulating using prototypes, I wrote MobX to make that happen, my fear is mostly around the confusion that would be caused by the half-half classes we would be forcing to be created)

If _less careful_ developers (ie: those who don't write tests 😉) want to be more careful, they can still reap the benefits of methods by leveraging factory functions:

function User(name, age) {
  return {
    name,
    age,
    onBirthday() {
      // Just don't use arrow syntax! ;)
      this.age++
    }
  }
}

Immer will copy the function properties, no problem. 👍

I bet V8 optimizes that pretty nicely. (but definitely test it if you care)

_{Or they could just write tests (or use custom lint rules) ;P}

edit: Actually, maybe custom lint rules __are__ the right solution. 😆

[mweststrate]

Hmm, a section in the readme recommending factory functions is a good idea in _any_ case :). V8 will optimize most of that pretty good (at least an internal class), It might still create a closure per instance though, but not 100% sure.

[offtopic]
More optimized would be the following, but I think the different doesn't matter in 98% of the cases:

function User(name, age) {
  return {
    name,
    age,
    onBirthday
  }
}

function onBirthday() {
      // Just don't use arrow syntax! ;)
      this.age++
}

[/offtopic]

[aleclarson]

@mweststrate We could write a Babel plugin that hoists methods like that. :D

We would need the Immer symbol as a heuristic, though.

[mweststrate]

I wouldn't be surprised if this already exists (or even is done by minifiers, babel, or V8 itself), hoist functions that don't access anything from closure.

[aleclarson]

To call or not call constructors is quite a topic, take for example this:

class User {
   name,
   profile,
   age = 10

   constructor(name) {
       this.name = name
       this.fetchProfile("users/"+ name).then(v => {
          this.profile = v
       }
    }
}

How common is this? Maybe it's best that we break that abomination. :laughing:

So these kind of trouble are what make me doubtful about supporting classes. Because in practice, it means that class instances should be immutable. (Ok, well, that is why we are using immer, but naturally, classes are mutable concepts, that encapsulate state).

So I think _at least_ we should narrow the scope to: _immutable classes with a prototype_, which is not the same thing as classes in general. (Maybe this was the intention the whole time, but at least calling it out now ;-))

Agreed!

[aleclarson]

I wouldn't be surprised if this already exists (or even is done by minifiers, babel, or V8 itself), hoist functions that don't access anything from closure.

Maybe you could ask a V8 team member? :P

[mweststrate]

I wouldn't be surprised if this already exists (or even is done by minifiers, babel, or V8 itself), hoist functions that don't access anything from closure.

Maybe you could ask a V8 team member? :P

Answer: no, static analysis cannot predict whether the functions are mutated in the future, or used in comparisons, so the transformation would be unsafe

[mweststrate]

Ok, how does this sound @aleclarson:

import { cloneable /* a symbol */ } from "immer"

class Person {
   static [cloneable]: true
   age: 3

   constructor(age) { this.age = age }

   older() {
      this.age++
   }
}

const instance1 = new Person(10)
const instancte2 = produce(instance1, p => { p.older() })
console.log(instance1.age) // 3
console.log(instance2.age) // 4

Semantics:

1. If an object has a prototype different from null or Object,
2. And if object[cloneable] === true || object.constructor[cloneable] === true
3. Clone the object and set the correct prototype (don't invoke the constructor).
4. Copy _all own_ properties
5. Freeze the object (if enabled). Helps with point 6.1
6. for cloneable objects the following semantics should be followd:
   
   
   
   1. the objects should be treated as completely immutable
   
   
   2. the constructor should be side-effect free.
   
   
   3. No field should contain bind functions (note: it is possible to return new bound functions from getters, as long as they are not stored that is fine)
   
   
7. Note that in the above example, invoking instance1.older() without producer, would throw an exception! (if auto freeze is enabled)

N.B. Ideally, we would mark the age field as readonly in for example TS, but probably that will result in a compile error for the older function

[be5invis]

@mweststrate memberwiseCloneable might be a better name :)
That would be great for classes representing "structs" (like 3D vectors, etc).

[oriSomething]

Wouldn't be better to expose hooks (Not the React one) for allowing us how to "clone" and state if it's a "draft" or something. But leave us the management how to clone, manage and detect if this special type is supported.
In my mind I think about API, that allow us on one hand to support Set/Map without it being part of the Immer API and on the other hand support efficient immutable data structures for big data (lists and maps as in ClojureScript / Immutable.js).
When I've started to see what's needed to support Maps, It seems this process is the same to whatever data structure that I would like to add. It is a bit messy for ES5+Proxies support, but It seems ES5 API going away anyway.
Moreover, adding static property doesn't seems to me like a good idea, because it might limit in the future

[mweststrate]

Moreover, adding static property doesn't seems to me like a good idea, because it might limit in the future

Note, it doesn't have to be static, the proposal above checks both own, inherited and static properties.

Hooks and plugins have passed this and related threads a few times, to achieve this behavior. But I didn't seen a clean abstraction for them, it remains a bit vague, silver bullet answer at this point. There is no clear idea yet how we could support Map / Set's in the current code base (without writing our entire own Map / Set implementation at least). So if we cannot achieve that nicely within the current setup, it will even be harder to achieve that through plugins, where there is an additional level of indirection. That being said, a PR with a proposal and rough example implementation would obviously be welcome :) Plugins have only value I think if they are fairly easy to write.

[oriSomething]

@mweststrate I would love too, but since I've become father recently I don't have much time to code outside work, And technically immer isn't useful for us now :(

[mweststrate]

I've become father recently

Mzl tov!

isn't useful for us now

is it because of custom classes, or maps? The former is much much easier to achieve (within the limits above), because if the class already represents a conceptually immutable thing (basically a typed record), that is much easier to immerize compared to maps, which are very mutable concepts

[oriSomething]

@mweststrate

Thanks!

The main reason, it's because we use big Maps that get updated very often. In some cases theses Maps become in size of 1000+.
Coping for cloning isn't the best for us, especially because our designer likes very custom UI with a lot of animations / transitions. So, cloning and than re-render with animation become junky. Today, I don't think our "app" performant enough. Surprisingly, I yet to hear complains from users.

Since, we are only 2, and one is a junior, I don't see myself soon get the resource to change how our app + immer works.

[aleclarson]

@mweststrate Looks good!

Though, I think immerable is cooler and clearer than cloneable. 😎

I'll be sending a PR sometime in the next few weeks. 👍

---

It would be nice if there were immutable Map and Set classes in JavaScript. Not even Object.freeze works with them. 😢

You _could_ subclass and mock those classes. Or maybe there's a way for a Proxy to lie about its prototype in order to fool instanceof checks?

If we find a solution, I think we'll want to make immer-map and immer-set libraries, in order to keep immer core on the small side.

[oriSomething]

• Why not @immer/map and @immer/set?
• There was a previous PR of someone who override Map methods to support freeze: https://github.com/mweststrate/immer/pull/149/files#diff-2c36c683dff5798fefed561dbec17803R81
• Personally I think "Freezing" is the least important feature, even un-needed. The most import this library gives is "easy immutability". Freeze, to my opinion, is a nice feature, but might be better as an ESLint rule than a run-time error.

[aleclarson]

For anyone interested, check out #286.

[jineshshah36]

I'd like to throw another idea into the mix, somewhat based on Rust traits / the tc39 protocols proposal:

// in immer
const clone = Symbol("Immer.clone")

export class Immer {
  static clone = clone
}

// you can provide a helper for any class to make it cloneable:
export function immerize(obj: any, cloneFunc: (original: T) => T) {
   obj.prototype[Immer.clone] = cloneFunc
   return obj
}

// somewhere in app
import { Immer } from "immer"

class Test {
  static [Immer.clone](original: Test) {
    return // anything you want
  }
}

// to clone later:
const test = new Test()
const testClone = test.constructor[Immer.clone](test)

// to allow builtins to be cloneable
immerize(Map, (originalMap: Map<any, any>) => new Map(...originalMap))

// after immerizing Map, you can use any Map in a `produce` and it will "just work" because
// internally, immer can call the `Immer.clone` symbol method.
const state = { foo: new Map([["hi", 1], ["hello", 2]]) }
const newState = produce(state, draft => { draft.foo.set("hi", 3) })

// you can also throw and error if a class does not implement `Immer.clone`
const state = { foo: new Set([1,2,3]) }
const newState = produce(state, draft => { draft.foo.add(4) }) // Error: Set does not implement Symbol("Immer.clone")

[aleclarson]

@jineshshah36 For that approach to work, you'll also need a method for detecting changes. Is your proposal an objection to #286? Or are you just looking for a way to optionally support built-in types like Map and Set?

[jineshshah36]

What I meant to propose is a way to define how to clone an object rather than just marking it as DRAFTABLE as #286 does now. #286 does a shallow copy of everything, but it could allow for providing a way to override the default class shallowCopy behavior. In terms of change detection, exported decorators might be a way to allow change detection?

[jineshshah36]

Something along the lines of this:

export function shallowCopy(base, invokeGetters = false) {
    // check if custom copy method is implemented (works with custom and builtin classes)
    if (base[DRAFTABLE]) return base[DRAFTABLE](base)
    if (base.constructor[DRAFTABLE]) return base.constructor[DRAFTABLE](base)
    if (Array.isArray(base)) return base.slice()
    const clone = Object.create(Object.getPrototypeOf(base))
    ownKeys(base).forEach(key => {
        if (key === DRAFT_STATE) {
            return // Never copy over draft state.
        }
        const desc = Object.getOwnPropertyDescriptor(base, key)
        if (desc.get) {
            if (!invokeGetters) {
                throw new Error("Immer drafts cannot have computed properties")
            }
            desc.value = desc.get.call(base)
        }
        if (desc.enumerable) {
            clone[key] = desc.value
        } else {
            Object.defineProperty(clone, key, {
                value: desc.value,
                writable: true,
                configurable: true
            })
        }
    })
    return clone
}

[ghost]

I hope that this new functionality does not noticeably increase the bundle size. I was drawn to immer due to the size -- it's small enough for mobile. Optional add-ons, or new functions that will removed by DCE, would be preferred.

[aleclarson]

Now that #286 is merged and released in v1.11.0, I'm going to close this.

Anyone interested in discussing Map/Set support or extending the functionality of class support should open a new issue with their proposal. Thanks!