Consider mobx

[thesunny]

Do you want to request a feature or report a bug?

infrastructure change

What's the current behavior?

Slate uses immutable internally to improve editor performance.

When a render happens, Slate compares nodes for reference equality and skip the re-render if they are the same.

There are two issues with this (1) as the document gets bigger there are more objects that need to be compared on each render so performance can become an issue in a big document even if the change is small (2) there are cases where data outside the Immutable document effects the render. These need to be handled on a property by property basis in shouldComponentUpdate and is a cause for easy to miss bugs in custom Slate code where the outside value changes but the editor isn't re-rendered.

What's the proposed behavior?

So I'm posting this because I've been thinking about this for a while. I understand this would be a huge change and would be hard to do anytime soon if ever. Mostly, I just wanted to start a dialog even if it's short lived so I can move on from thinking about this.

Mobx basically works in reverse. When you update the data structure, mobx is already tied to the right components to do the renders. In other words, it doesn't need to traverse the entire document tree in order to update the right component. This should solve the performance problem as documents get larger.

Furthermore, as long as any other render dependency values are mobx values, we don't have to worry about updating the shouldComponentUpdate logic. This removes that problem case which seems to show up regularly in Issues.

Practically speaking, two things of note:

• According to bundlephobia mobx is a little smaller than ImmutableJS so if there is a switch done at some point the total size should be about the same
• As render dependencies get more complex or the document gets larger, mobx because more and more performant and simpler to manage on a relative basis

[dmitrizzle]

The performance structure is over my head, but may I suggest thinking with Redux instead of Mobx?

Redux is a much smaller package (though it's more likely to require external libs for support) and it's also a lot more popular component in React apps, which means no additional weight what so ever for some users.

[lxcid]

This will be a breaking change for me though. We use immutable.js and referential equality heavily and Slate was a perfect fit for my use case.

I think is there a way to expose an API that capture custom immutable.js collection that user can use to effect rerendering?

I can see the power of mobx but if the value was not managed by mobx, isn’t it the same issue as well? Although mobx handle computed value magically.

Is there a way to mix as well?

[thesunny]

@dmitrizzle

Redux helps manage complex state. State management is simple in Slate. We are passing one Value object into the editor which means that Redux won't add value to this use case but will add overhead. I suggest mobx not for state management but for render management (there may be a better word for this).

If we change the value of one Slate Node represented in mobx then it will trigger a render on the one Component that needs to be rendered. The Slate Document tree isn't traversed to find out which components need to be rendered. In Immutable Slate, as the Document gets bigger, the slower the editor gets.

@lxcid

It would be a breaking change; however, all of the custom stuff you are doing wouldn't be required any more. All you'd have to do is put whatever custom state you were rendering from into a mobx value (basically an Object) and use that. mobx handles the wiring for you so you could get rid of all your referential equality code. That's the main benefit. I've run into the problem myself where I add a value that didn't have a custom reference equality check and Slate doesn't render the update. With mobx, that won't happen.

It's not the same issue as managing the state somewhere else for two reasons:

1. mobx handles all the dependency checks on when a component needs to re-render for you which removes all the bugs related to forgetting to update the shouldComponentUpdate methods everywhere in Slate code. This makes it easier to add custom rendering values and even have them sit outside Slate's Value object.

2. When you change a mobx value, mobx knows which components to update. This means that we don't make the reference equality comparisons and we don't traverse the Document tree which improves performance.

In 2, imagine adding a single letter to the DOM due to a keyDown event in Slate the way it is now. Slate loops through each of the top level nodes and does ref-equality checks. The one node that is different, we drill into that until we get to the node with the change and we re-render that. If the top level tree has 100 blocks, we made 100 reference equality checks.

Contrast that to the mobx Slate node being updated. The node knows it needs to update one specific Component. There is no node-traversal.

We end up with three benefits:

1. Performance

2. Simplicity

3. Less bugs because less code related to knowing when something should be updated

So it's a big change. Yucky. Messy. It will have long-term benefits but non-trivial pain in the short-term...

[freeplant]

I suggest a simple improvement on the performance in the short term.

A large document typically has a long list of top level nodes. In content.js, the render function first render the list of the children:

    const children = document.nodes.toArray().map((child, i) => {
      const isSelected = !!indexes && indexes.start <= i && i < indexes.end

      return this.renderNode(child, isSelected, childrenDecorations[i])
    })

If the children is a long list with 10K elements, there will be 10K shouldComponentUpdate call.

To improve the performance, we can past the change object to the Editor, then to content.js. The render function checks the operations in the change object. If it is a simple operation, only re-render the affected nodes.

[lxcid]

@thesunny I understand your point of view.

The auto-subscription and dependency graph of mobx really help with common programming errors.

Technically, I will have to re-evaluate if Slate change to mobx, I don't depend on Slate data model since many versions ago so it won't be a painful switch but I depend on Redux a lot and its something much more set to stone than Immutable.js as I uses redux-saga a lot.

my understanding is that mobx and redux is 2 very different system. One focus on automatic managed subscription for mutation while the other focus on snapshot based event driven system (which basically oversubscribe at every events).

My concern is that is there a escape hatch that allow us to propagate the final changes from slate (in mobx) to redux and bridge between the 2 world?

Edit:

Actually, we'll likely expose onChange callback on <Editor />. That would be sufficient for me I think.

But I not sure how that will affect controlled component and how we repopulate it with initiate set of data first.

Also, how do remote changes coming outside of mobx system get updated. Do we have to track 2 different state tree, one redux one mobx (with mobx-state-tree)?

[thesunny]

@lxcid

I didn't think about that. My gut says to use a regular prop passed into Slate and do shouldComponentUpdate as an escape hatch from that prop. In your case, it's the redux value but it is unopinionated about the type of prop (could also be a plain Object).

[thesunny]

@freeplant

Sorry I missed your post.

I'm trying to think of how that might work.

You could have shouldComponentUpdate return false on content so it doesn't re-render. Then find the ref to the Block that contains the update then call forceUpdate on it.

Things that may trip it up are if two change objects come in the same tick. Also, you'd have to build a map since if you looped through the components to find it, that's not much different than calling shouldComponentUpdate while looping through the blocks. I worry about the edge cases like it not being compatible with data that sits outside Slate.

edit: Not sure how you'd get the new props into the Block without rendering content first.

[newtack]

Another option is to use immer rather than immutablejs. Immer is from the same author as Mobx. It would preserve ability to have immutability but with a more modern library that has a friendlier API.

[ericedem]

I've read through this a few times now and I have to admit I'm having trouble grokking what tangible benefit MobX will provide for both performance and preventing bugs.

@thesunny Did you have a specific example in mind where performance would be improved? You also mentioned some bugs that would have been prevented with MobX, do you have specific examples in mind?

One other consideration: what effect would this change have on schema validation and normalization? We still need to do tree traversal there.

[thesunny]

@newtack immer may be a substitute for immutable.js but it wouldn't address the issues that mobx would solve like performance and preventing bugs.

@ericdem breaking your questions down:

Performance:

Let's say you have 2 identical arrays of 1000 JavaScript objects which we'll call arrayA and arrayB. The objects look something like {value: 8} (doesn't really matter but just so we have something in mind). We always want array B to be equivalent to array A. Now in this hypothetical, it is really slow to make any updates to set B so we don't want to make them often.

Now let's say we update the object at index 555 in arrayA so that it changes {value: 1} to {value: 2}. The code would be arrayA[555].value = 2

One way to find out how to update arrayB is to loop through all 1000 values and compare the objects to each other and see if they are equivalent. If they are not, then we do a set arrayB[555].value = 2. We'll call this the "full comparison method". So to be clear, this method requires us to make 1000 comparisons.

Another way to keep the two arrays in sync is to keep a reference in each object in arrayA to the equivalent item in arrayB. In JavaScript, you can program a setter to have side effects. So if we do arrayA[555].value = 2 it can compare then update arrayB[555].value = 2 because it has a reference to arrayB[555]. We've now done 1 comparison and skipped 999 comparisons. We'll call this the "MobX method".

It's more nuanced in that MobX can actually target the change down to the individual text node so if you typed one letter, instead of comparing the entirety of Slate document against the entirety of the old version of the Slate document, you are only updating the single span that the text you typed is in.

shouldComponentUpdate bugs:

In order to improve performance, React looks for a method in a React Component called shouldComponentUpdate. By default, React generates the entire virtual DOM (i.e. it runs all those statements in the render method) and compares the newly created virtual DOM against the existing virtual DOM. But sometimes you know ahead of time if the new virtual DOM will equal the old virtual DOM by just looking at the old props and the new props that were passed in and that comparison is faster than generating a new virtual DOM and comparing them.

Slate does this to improve performance.

But it only compares the values Slate expects will result in a change to the render.

The bugs appear because sometimes you need to re-render based on a value Slate doesn't expect will change the render. This can happen, for example, if say you want to change the heading block's background color and that color is held in a variable outside Slate. If you change the headingBgcolor variable (and it is outside Slate), the editor won't re-render because Slate's shouldComponentUpdate does not check for it.

This is a simple example, but there are many reasons to change the way Slate renders its document based on outside state.

Schema Validation and Normalization:

MobX won't affect the way validation and normalization work.

[thesunny]

Performance graphically illustrated:

Below is the way Slate works now. We changed one of the values from 1 to 2 and each | represents a comparison and the o represents no change and x represents a change.

1111111111111111111111111111111111111211
||||||||||||||||||||||||||||||||||||||||
oooooooooooooooooooooooooooooooooooooxoo
1111111111111111111111111111111111111111

With MobX we make one comparison

1111111111111111111111111111111111111211
                                     |
                                     x
1111111111111111111111111111111111111111

[klis87]

I would like to add one potential benefit of immer compared to both mobx and immutable - with immer you just use JS objects, you don't need to do any convertion back and forth, like for example immutable and mobx .toJS method. Not only this requirement is bugging especially for devs not using mobx or immutable, but also it is really intensive operation, sometimes probably killing any gains of using immutablejs or mobx in the 1st place (depending on use-case)

So looking at it, immutable-js vs immer, immer is no-brainer for me, immer is the clear winner:

• Slate users can use just JS object, without learning additional library API like immutable
• no costs of convertion from and to native JS objects
• immer has only 4kbs size

Regarding Mobx vs Immer, I dont wanna giving any arguments about performance and bugs, as I am not Mobx expert (I used it only in 1 project), but by using Mobx data structures you are again open to immutable disadvantages (conversions and learning new API). But I cannot make a fair comparison here. I am Redux fan not Mobx, as imho Mobx introduces too much magic and it has really serious debugging issues (in comparison to Redux)

[ericedem]

@thesunny Thanks, thats a great explanation!

On performance: One thing I do wonder about is how often we are going to run into a flat data structure like this in slate. It doesn't seem likely that you would have 1000 blocks at the top level. I'd expect comparison performance to be closer to a O(log(N)) due to the tree structure nature of the document.

I also wonder a bit about the performance hit of having to continually attach / detatch the two way binding in MobX. I've seen performance problems around two way binding when working with Angular 1 with a large amount of variables in the app. The performance impact of pure React seems a bit easier to understand.

@klis87 It might be good to open a separate discussion issue for replacing Immer with Immutable.js. It seems like a separate discussion which might be promising on its own! 😄

[thesunny]

@ericedem

I think the current code is O(n). As a document gets bigger, we add more top level blocks (e.g. paragraphs) of similar size rather than having bigger paragraphs. I believe MobX would make it O(1) although the first render would still be O(n).

With respect to binding being a performance issue, and from what I've read, mobx in real life scenarios outperforms custom shouldComponentUpdate because the bindings are granular and precise. In the example where we change the heading background color MobX would only do comparisons for the heading blocks. So far I haven't read that in real-life scenarios MobX has made things slower.

I think the O(1) nature is what I like about MobX because, generally, if Slate performs well in a small document, it should perform similarly in a big one.

@klis87

I'd like to ask about "it has really serious debugging issues (in comparison to redux)". Can you be specific?

[klis87]

@thesunny I remember that sometimes nothing was shown in console in case of an error, or it wasnt as easy to find based on stacktrace what part of code was problematic (I used Mobx both as separate classes and inside React class components). Also, I had some issues when I used observer component inside inline render pattern (like React-Router render). Additionally, for me Mobx looks like improved version of Angular, much better and consise syntax, but it really looks like 2-way data binding, and in more complex scenarios it is really hard to understand why some state is updated. Whats related to this, I feel that Mobx more fits OOP pattern, which imho is not as scallable as FPP when speaking about really complex state management.

[PierBover]

Personally I would vote for MobX too, but (if possible) the ideal solution is that Slate should be agnostic. With a modular approach one should be able to use Mobx, Immer, or whatever new thing that comes in the future.

[ianstormtaylor]

Hey folks, I'm going to close this in favor of #2345 now. I don't think we'd end up using MobX, as React seems to be moving away from monolithic state management in general (which I'm in favor of). But the idea of being able to use plain JavaScript constructs is incredibly appealing for other reasons. I'd love if you could add your thoughts there. Thanks!