Slate: Ctrl+Z to undo on empty editor crashes the editor

Created on 18 Nov 2017  Â·  10Comments  Â·  Source: ianstormtaylor/slate

Creating an empty editor with html.deserialize('<p></p>'), and then pressing Ctrl+Z crashes the editor.

Instead initialize with a raw value:

Value.fromJSON({
  document: {
    nodes: [
      {
        kind: 'block',
        type: 'paragraph',
        nodes: [
          {
            kind: 'text',
            leaves: [{ text: '' }]
          }
        ]
      }
    ]
  }
})
bug ♥ help

Most helpful comment

Hey @conorcussell thanks for the super clear writeup. I'd love a PR that fixes the getSelectionIndexes case.

For the empty nodes case, Slate itself shouldn't add a node, but I'd definitely be open to a PR that threw a more helpful error when that impossible case is run into. (Ideally in a single, or few places, wherever makes the most sense in that code path.)

Thank you!

All 10 comments

Okay nevermind that. It was working for about 2 minutes it seemed, but I'm seeing the issue again. In any event, this is an edge case.

The error is: Uncaught Error: Invalidkeyargument! It must be a key string, but you passed: null in slate/lib/models/node.js on line 2229

Using Slate 0.31.

This also happens when you paste text into an empty editor and then Ctrl-Z

In the fiddle https://jsfiddle.net/mh0an9ss/ paste some text, ctrl-z and it throws Error: Invalid `key` argument! It must be a key string, but you passed: null

Yes I just hit that today as well. @ianstormtaylor sorry for lack of jsfiddle. @julienp thank you for adding one.

This bug been noted in previous issues like https://github.com/ianstormtaylor/slate/issues/1161.

It's pretty awful.

You can't do any transformations in the SlateEditor until you click on it.

I may just be repeating things people already know, but here are my findings from digging into this today.

When you create an empty editor the selection is unset, and an invalid selection is created because text is undefined.

https://github.com/ianstormtaylor/slate/blob/199c32a2b5f776c586e7202cfa3183e551153fe4/packages/slate/src/models/value.js#L117-L120

This also happens with copy and paste because the onPaste plugin invokes Plain.deserialize which in turn invokes Value.fromJSON

In the render method of slate-react's content component getSelectionIndexes is called with a startKey and endKey of null, which calls getFurthestAncestor with null and gives us the error.

https://github.com/ianstormtaylor/slate/blob/199c32a2b5f776c586e7202cfa3183e551153fe4/packages/slate/src/models/node.js#L1410-L1414

Simply returning null in getSelectionIndexes when startKey and endKey are null will fix this case, and copy + pasting but it will not fix the case mentioned in https://github.com/ianstormtaylor/slate/issues/1161. If the editor won't work without any nodes in it, should we ensure there is at least one node by default?

// if we've been passed an invalid range, we can exit early
if (!startKey && !endKey) {
  return null;
}

Hey @conorcussell thanks for the super clear writeup. I'd love a PR that fixes the getSelectionIndexes case.

For the empty nodes case, Slate itself shouldn't add a node, but I'd definitely be open to a PR that threw a more helpful error when that impossible case is run into. (Ideally in a single, or few places, wherever makes the most sense in that code path.)

Thank you!

This isn't an issue anymore (since #1443).

This is an issue again as of 0.33.4

editor.value.history.undos.size seems to have a default size of 1 ~~Why is this? ~~

Edit: Or is this something I have introduced myself? Currently investigating.

Edit2: The undo size of 1 was my fault, but removing this behavior does not correct the undo problem.

Moving discussion to #2260

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bunterWolf picture bunterWolf  Â·  3Comments

gorillatron picture gorillatron  Â·  3Comments

chrpeter picture chrpeter  Â·  3Comments

YurkaninRyan picture YurkaninRyan  Â·  3Comments

ianstormtaylor picture ianstormtaylor  Â·  3Comments