Slate: Consider replacing immutableJs with immer

[klis87]

Do you want to request a _feature_ or report a _bug_?

infrastructure change, continuing discussion in #2153

What's the current behavior?

Currently immutable.js is used in slate models, which has some serious disadvantages compared to immer:

• ImmutableJS requires learning new API, which is problematic especially for Slate users who didnt use immutableJS in the past
• ImmutableJS is much bigger library, around 15kb compared to Immer 4kb
• ImmutableJS objects introduces new data structures, which need to be converted to native JS objects, which is bad for performance and also forces developers to pay attention whether a given object is Immutable wrapper or not, while with Immer no conversion is necessary - you work with native JS data structures always.

What's the expected behavior?

After switching to Immer, suddenly Slate would be much easier to use for people especially not knowing how to work with ImmutableJS. Also performance should be better as no objects convertion from and to ImmutableJS would be needed anymore.

[ianstormtaylor]

Hey @klis87, thanks for opening this. I have to admit, immer is very interesting. It wasn't around when I first chose the immutable models for Slate, or I may have considered it. I don't know that much about it though, so there are a few concerns I have:

1. Since Class instances aren't supported, I'm not sure how we'd end up modeling all of the Block, Inline, Mark, etc. methods that we currently rely on using our class-based architecture. It sounds like these would need to then become static helper methods instead? Or is there another solution? This seems like a big change, and one that is going to be more confusing than not?

2. Similarly, it sounds like immer can't handle native class instances like Date, Map, Set, etc. Which is fine because we don't use them in Slate's core, but I'm wondering whether people using these things in node.data are in for confusion. I can't tell if it will work seamlessly or not. (Potentially since we only ever allow you to overwrite those values it will be fine.)

I do very much appreciate the "not learning any new APIs" benefit. (And the related improvement to performance for not having to serialize/deserialize from JSON.) I'm just still not clear on how it would work with Slate to be able to evaluate it in the first place.

Cheers!

[ianstormtaylor]

I've commented in the immer repo to hopefully figure out some of those remaining questions, so that we can make an informed decision!

(This is just to feel out what the true pros/cons are. Of course there are other implications since this would be a huge change to the API regardless, so it would require more discussion about its worth.)

[ianstormtaylor]

Another thing to figure out would be how to handle the "memoization" that we currently rely on Immutable.js's architecture for. We just store things on the instance knowing that they'll be obliterated when any changes happens.

[linonetwo]

There may need a wrapper, likes a lightweight @rematch/core + @rematch/immer + @rematch/select without the redux.

In rematch way, you don't create a custom class but pass in an object with methods. Methods change the internal value of that object and return a new object.
But there is no such a package other than mobx.

[ianstormtaylor]

@linonetwo do you have a code sample of what that looks like? Not sure I follow.

[linonetwo]

While using rematch, we can do:

const Point = {
  state: {
    key: null,
    offset: null,
    path: null,
  },
  selectors: {
    isSet() {
      return ({ point }) => point.key != null && point.path != null && point.offset != null
    },

    isInNode(node) {
      ({ point }) => {
        if (!point.isSet) return false
        if (node.object === 'text' && node.key === point.key) return true
        if (node.hasNode(point.key)) return true
        return false
      }
    }
  },
  reducers: {
    setOffset(state, offset) {
      state.offset = offset;
      return state;
    }
  }
}

const store = init({
      models: {
        point: Point
      },
      plugins: [immer],
    })

store.point.setOffset(xxx)
store.point.isSet
store.point.offset

Some parts look good, but it's deeply bound to redux, there could be only a singleton. So it's not useful for the Slate.

To be useful, there should be a new package that allows multiple instance to exist.

@rematch/immer wrap all functions from reducers in a immer's produce:

https://github.com/rematch/rematch/blob/c2677cb5de8dbf2f3e1dd05d8d044e4333670eec/plugins/immer/src/index.ts#L9-L22

function wrapWithImmer(reducers) {
        const reducersWithImmer = {}
    for (const [key, reducerFn] of Object.entries(reducers)) {
        reducersWithImmer[key] = (state, payload) => {
            if (typeof state === 'object') {
                return produce(state, (draft) => {
                    const next = reducerFn(draft, payload)
                    if (typeof next === 'object') {
                        return next
                    }
                })
            } else {
                return reducerFn(state, payload)
            }
        }
    }
        return reducersWithImmer
}

Similar logic can be used to build a Record function:

function Record(model) {
  return () => { ...model, ...someProxyToAutoPassStateToReducer(
     wrapWithImmer(model.reducers),
  );
}

const PointFactory = Record(Point);

const point1 = PointFactory();
const point2 = PointFactory();

const point3 = point1.setOffset(xxx)
point3.isSet
point3.offset

Just some pseudocode. I think use immer this way is similar to use mobx.

[klis87]

@ianstormtaylor Thanks for quick answer and contacting Immer author, do you think that solving https://github.com/mweststrate/immer/issues/202 will be enough for Slate?

Similarly, it sounds like immer can't handle native class instances like Date, Map, Set, etc. Which is fine because we don't use them in Slate's core, but I'm wondering whether people using these things in node.data are in for confusion. I can't tell if it will work seamlessly or not. (Potentially since we only ever allow you to overwrite those values it will be fine.)

I think this won't be an issue at all, as people updating node.data should be responsible themselves for not mutating anything, like generally in React world. Whats nice about Immer is that Slate consumers wouldn't need to use Immer at all, or even be aware of its existence (maybe apart from installing it as peer dependency :))

[ianstormtaylor]

@klis87 that's one of the bigger questions, but I'm not sure it's the only thing. Off the top of my head, the things that would need to be considered/solved are:

1. How to handle the current instance methods/getters?
2. How to handle the current memoization technique? (eg. for key lookups, or normalization)
3. How to handle Set use cases if we can't use the native Set objects? (eg. for marks)

[klis87]

@ianstormtaylor I guess that answer to question 1 depends on immer class support issue. Regarding question 3, I believe we still can use Set, see https://github.com/mweststrate/immer#pitfalls 9

[steida]

As for performance

immutable.js is optimized for write because of Trie. I remember, Lee Byron noted it's important for huge models. For example, editor model. But I am not sure how important it is with current JS engines.

https://hackernoon.com/how-immutable-data-structures-e-g-immutable-js-are-optimized-using-structural-sharing-e4424a866d56

As for API

• Getters are anti-pattern I am pretty sure. They hide potentially costly operations behind innocent .foo properties.
• Instead of classes, functional programmers use algebraic types https://gist.github.com/hallettj/281e4080c7f4eaf98317.
• Instead of instance methods, functional programmers use simple functional typed mappers.

This functional typed API thing is probably a little bit confusing for you. I can see because of how Slate is written. Don't get me wrong, Slate is excellent. But it's old school OOP. I would love to have a pure and typed functional Slate API. No instance methods, no classes. Just objects, types, and pure edit helpers. Much easier to write, maintain, learn, and use.

Immer can help a lot. The only potential issues I see is its write performance. But huge documents should be rendered with some kind of pagination or windowing anyway, so maybe it's not a real issue anyway.

So, typed Immer objects with pure typed functions is the way Slate should go.

[ianstormtaylor]

@steida thanks for the input.

This functional typed API thing is probably a little bit confusing for you. I can see because of how Slate is written. Don't get me wrong, Slate is excellent. But it's old school OOP. I would love to have a pure and typed functional Slate API. No instance methods, no classes. Just objects, types, and pure edit helpers. Much easier to write, maintain, learn, and use.

I understand functional APIs, sorry for the confusion. I was originally questioning whether moving to Immer would not involve switching to a functional API, but it sounds like it mostly would, and that would be where the big gains are anyways. So now this issue for me has changed from "could we switch to Immer" to "would a functional API work for us".

When I say that those parts of the API are still a "question" I'm more concerned about the UX of the potential functional API. The "much easier to write, maintain, learn and use" is just your opinion. It depends on what the API is for—there are definitely APIs that are "functional" and "pure" that end up harder to use because of it.

I'm more curious about what the functional API would end up looking like in end-user code.

If people are using the instance methods and getters on the current Slate models heavily, I can see it being a lot more cumbersome to use a functional version of the API. The "pipeline" operator would help things here, but it's not something we can use easily yet. If we only ever used TypeScript that would help too, but that's not something I want to mandate.

That said, the potential benefit of using native JavaScript data structures, and not having any serialize/deserialize steps is hugely appealing, which is why I've left this open, so that others can prove out this idea if they'd like to see it in core.

If anyone wants to see this happen, please provide code snippets for API design/architecture ideas that make it easier to visualize how these pieces will fit together. And then from there we can decide whether the tradeoffs are worth it.

[ianstormtaylor]

Hey everyone, I'm going to move this to https://github.com/ianstormtaylor/slate/issues/2345. Thank you very, very much to @klis87 for opening this in the first place! And to @linonetwo and @steida for your thoughts. I'd love if you could add anything else you think of to that new issue. Thanks!