Added insert_node path checking to match how other operations are applied (#4230)

* Added try/finally to editor.apply and Editor.withoutNormalizing for state cleanup

* Added insert_node path checking to match how other operations are applied

.changeset/bright-pants-grin.md

@@ -0,0 +1,5 @@
+---
+'slate': minor
+---
+
+Applying invalid `insert_node` operations will now throw an exception for all invalid paths, not just invalid parent paths.

.changeset/sixty-glasses-beg.md

@@ -0,0 +1,5 @@
+---
+'slate': patch
+---
+
+Exceptions in `editor.apply()` and `Editor.withoutNormalizing()` will no longer leave the editor in an invalid state

packages/slate/src/interfaces/editor.ts

@@ -1648,8 +1648,11 @@ export const Editor: EditorInterface = {
   withoutNormalizing(editor: Editor, fn: () => void): void {
     const value = Editor.isNormalizing(editor)
     NORMALIZING.set(editor, false)
+    try {
       fn()
+    } finally {
       NORMALIZING.set(editor, value)
+    }
     Editor.normalize(editor)
   },
 }

packages/slate/src/transforms/general.ts

@@ -2,6 +2,7 @@ import { createDraft, finishDraft, isDraft } from 'immer'
 import {
   Node,
   Editor,
+  Selection,
   Range,
   Point,
   Text,
@@ -17,20 +18,19 @@ export interface GeneralTransforms {
   transform: (editor: Editor, op: Operation) => void
 }
 
-export const GeneralTransforms: GeneralTransforms = {
+const applyToDraft = (editor: Editor, selection: Selection, op: Operation) => {
-  /**
-   * Transform the editor by an operation.
-   */
-
-  transform(editor: Editor, op: Operation): void {
-    editor.children = createDraft(editor.children)
-    let selection = editor.selection && createDraft(editor.selection)
-
   switch (op.type) {
     case 'insert_node': {
       const { path, node } = op
       const parent = Node.parent(editor, path)
       const index = path[path.length - 1]
+
+      if (index > parent.children.length) {
+        throw new Error(
+          `Cannot apply an "insert_node" operation at path [${path}] because the destination is past the end of the node.`
+        )
+      }
+
       parent.children.splice(index, 0, node)
 
       if (selection) {
@@ -296,7 +296,21 @@ export const GeneralTransforms: GeneralTransforms = {
       break
     }
   }
+  return selection
+}
 
+export const GeneralTransforms: GeneralTransforms = {
+  /**
+   * Transform the editor by an operation.
+   */
+
+  transform(editor: Editor, op: Operation): void {
+    editor.children = createDraft(editor.children)
+    let selection = editor.selection && createDraft(editor.selection)
+
+    try {
+      selection = applyToDraft(editor, selection, op)
+    } finally {
       editor.children = finishDraft(editor.children)
 
       if (selection) {
@@ -306,5 +320,6 @@ export const GeneralTransforms: GeneralTransforms = {
       } else {
         editor.selection = null
       }
+    }
   },
 }

packages/slate/test/transforms/general/invalid-insert_node.tsx

@@ -0,0 +1,30 @@
+/** @jsx jsx */
+import assert from 'assert'
+import { Transforms } from 'slate'
+import { jsx } from '../..'
+
+export const input = (
+  <editor>
+    <block>
+      word
+      <cursor />
+    </block>
+  </editor>
+)
+export const run = editor => {
+  // position 2 is past the end of the block children
+  assert.throws(() => {
+    Transforms.insertNodes(editor, <text>another</text>, { at: [0, 2] })
+  }, 'Inserting a node after the end of a block should fail')
+  // 1 is _at_ the end, so it's still valid
+  Transforms.insertNodes(editor, <text>another</text>, { at: [0, 1] })
+}
+export const output = (
+  <editor>
+    <block>
+      word
+      <cursor />
+      another
+    </block>
+  </editor>
+)