ianstormtaylor / slate

A completely customizable framework for building rich text editors. (Currently in beta.)
http://slatejs.org
MIT License
30.02k stars 3.26k forks source link

Support for combining or merging operations #1770

Open majelbstoat opened 6 years ago

majelbstoat commented 6 years ago

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

Feature

Description

Slate's low-level operation primitives are great, but it's a lot of overhead to send them back to the server. I would like to see support for combining sequential operations where possible.

This is a slightly different need, but has some overlap with https://github.com/ianstormtaylor/slate/issues/1730. There, @bryanph describes a higher level abstraction (which I also think is a good idea, but might be application-specific). This is more around merging low-level operations intelligently.

Here's a very simple barebones synchroniser written in TypeScript that performs some minimal merging. It only handles insertText and removeText at the moment, but that already takes care of 80% of the overhead for me. Caveat emptor, no tests, no smart back-off etc. etc.

https://gist.github.com/majelbstoat/a4770728033a82ceb18ad4d37231901f

@ianstormtaylor, would you support adding utility functions offering something similar to the merging portions of this within Slate Core?

ianstormtaylor commented 6 years ago

Absolutely! I think this is a great idea. I believe this is something that is also useful in operational transform, where it is referred to as "composing" operations. I'd be down for a file that does this to live in ./src/operations/compose.js if someone is down to contribute it. I believe it should act on an array/list of operations.

majelbstoat commented 6 years ago

Cool. I've updated my gist a little to get this closer to what it should be. It now handles remove after insert, and removes redundant inserts and removes. It just pushes other kinds of operations. The interface is still working on a class member variable, so it needs tweaking for the general case. If someone else wants to run with this, they are welcome to, otherwise I will attempt to turn it into a vanilla JS implementation with the signature you described in the next week or so:

compose(operations: List<Operation>,  newOperations: List<Operation>): List<Operation>
bryanph commented 6 years ago

I can see basically three granularities of operations that we would need to handle:

  1. The operations needed to make the editor be in-sync
  2. The operations to handle undo/redo's with
  3. The operations to store in the back-end.

For 1. we shouldn't merge. For 2. we can merge some ops (only text operations I think) and for 3. we might want to merge even more operations.

ianstormtaylor commented 6 years ago

That's a good point about history requiring potentially different merging logic. I think for 1. though we'd want to still merge as much as possible, because merges will result in a reduction in over-the-wire bandwidth.

bryanph commented 6 years ago

@ianstormtaylor with 1. i meant the operation that needs to be applied to the editor to make it be in sync (what ops are applied by by the change method) so merging doesn't apply there.

I am actually not convinced anymore we should do this in the editor directly. What I am doing right now in my own code is maintaining my own History model (modified from the original) which I keep in sync by applying operations in onChange. I also hook into the undo and redo handlers to correctly update this buffer in order to make sure that the client and server are applying the same operations.

That said, there is some merging merging missing still that we should implement in the history model. Also, making it pluggable wouldn't hurt I guess.

mib32 commented 5 years ago

For me, the length of the operations is crucial. The Slates' operations compared to Google Docs' are just colossal. For realtime collaboration on the low bandwidth it's a killer. I would very much wish to have them combined.

sunsheeppoplar commented 2 years ago

We're looking to convert from Quill to something like Slate and this has come up as a point of potential concern