remove document normalize calls

[ianstormtaylor]

There are a handful of bugs with the history (undo/redo) that are from Node.normalize calls in the transform logic.

Previously we used to call .normalize() to ensure that the document never got into "weird" states. This worked fine before we moved to operations, but now that all updates to Slate are performed via undo-able operations, the .normalize() calls are now harmful, since they don't actually add operations to the operation stack, so they aren't able to be undone. This results in strange errors when trying to undo certain behaviors (mostly around removing nodes, splitting nodes, and joining nodes).

Instead of calling .normalize(), the transforms themselves should know about the potential "weird" states a transform can leave the document in, and handle them directly with more operations.

A list of transforms to cover: #354

Also, I think this should make performance better since we won't need to be iterating the whole document with every call to normalize 🎉

[ianstormtaylor]

Alternatively this might be solved by instead using a Schema to normalize internally after all transforms, which might be less error prone. Only reason I could think that this wouldn't work is if it gets strange for user-land transforms that expect the internally schema to be being obeyed.

[tyler-johnson]

@ianstormtaylor we have been having some hang ups with some medium-sized documents (50-100 block nodes). When I do some basic profiling with Chrome, I notice that the majority of time is being spent in normalize(). In my tests, for example, a simple delete command took about 100ms to complete, but the normalize call took over 1 second.

Please let me know if I can help speed this up as it seems most of Slate's performance issue come from normalize.

[tyler-johnson]

I made some changes according to this document on my own branch, specifically for removeTextByKey() and removeNodeByKey(). I had to expose joinNodeOperation() for "Should join text nodes when removing an inline node between them". I hope that is okay.

I noticed that all of the wrapBlock() tests fail due to an extra text node being added when the new block is created. I think this means that moveNodeByKey() needs some love too:

• moveNodeByKey() should remove empty text nodes in the new parent
• moveNodeByKey() should add an empty text node in the old parent if it was the last node

[ianstormtaylor]

Hey @tyler-johnson sorry I just saw your comments here, I don't know how I missed them. I separately arrived at the same conclusion for moveNodeByKey 😄 which is now added to the list in that PR. It's getting pretty close, although the wrap* transforms are the trickiest like you mentioned. If you make any more progress, feel free to comment in that issue and hopefully I can merge anything you branch off of it into there to speed things up! Thanks for the help

[ianstormtaylor]

Done now. Thanks @SamyPesse and @Soreine!!