fix(editor): guard against apply side effects when undoing/redoing

packages/editor/src/editor/hooks/useSyncValue.ts

@@ -11,7 +11,6 @@ import {validateValue} from '../../utils/validateValue'
 import {toSlateValue, VOID_CHILD_KEY} from '../../utils/values'
 import {isChangingLocally, isChangingRemotely, withRemoteChanges} from '../../utils/withChanges'
 import {withoutPatching} from '../../utils/withoutPatching'
-import {withPreserveKeys} from '../../utils/withPreserveKeys'
 import {withoutSaving} from '../plugins/createWithUndoRedo'
 import {type PortableTextEditor} from '../PortableTextEditor'
 
@@ -184,10 +183,8 @@ export function useSyncValue(
                         currentBlock,
                       )
                     if (validation.valid || validation.resolution?.autoResolve) {
-                      withPreserveKeys(slateEditor, () => {
-                        Transforms.insertNodes(slateEditor, currentBlock, {
-                          at: [currentBlockIndex],
-                        })
+                      Transforms.insertNodes(slateEditor, currentBlock, {
+                        at: [currentBlockIndex],
                       })
                     } else {
                       debug('Invalid', validation)
@@ -267,9 +264,7 @@ function _replaceBlock(
     Transforms.deselect(slateEditor)
   }
   Transforms.removeNodes(slateEditor, {at: [currentBlockIndex]})
-  withPreserveKeys(slateEditor, () => {
-    Transforms.insertNodes(slateEditor, currentBlock, {at: [currentBlockIndex]})
-  })
+  Transforms.insertNodes(slateEditor, currentBlock, {at: [currentBlockIndex]})
   slateEditor.onChange()
   if (selectionFocusOnBlock) {
     Transforms.select(slateEditor, currentSelection)
@@ -350,21 +345,17 @@ function _updateBlock(
           Transforms.removeNodes(slateEditor, {
             at: [currentBlockIndex, currentBlockChildIndex],
           })
-          withPreserveKeys(slateEditor, () => {
-            Transforms.insertNodes(slateEditor, currentBlockChild as Node, {
-              at: [currentBlockIndex, currentBlockChildIndex],
-            })
+          Transforms.insertNodes(slateEditor, currentBlockChild as Node, {
+            at: [currentBlockIndex, currentBlockChildIndex],
           })
           slateEditor.onChange()
           // Insert it if it didn't exist before
         } else if (!oldBlockChild) {
           debug('Inserting new child', currentBlockChild)
-          withPreserveKeys(slateEditor, () => {
-            Transforms.insertNodes(slateEditor, currentBlockChild as Node, {
-              at: [currentBlockIndex, currentBlockChildIndex],
-            })
-            slateEditor.onChange()
+          Transforms.insertNodes(slateEditor, currentBlockChild as Node, {
+            at: [currentBlockIndex, currentBlockChildIndex],
           })
+          slateEditor.onChange()
         }
       }
     })

packages/editor/src/editor/plugins/createWithMaxBlocks.ts

@@ -1,5 +1,6 @@
 import {type PortableTextSlateEditor} from '../../types/editor'
 import {isChangingRemotely} from '../../utils/withChanges'
+import {isRedoing, isUndoing} from '../../utils/withUndoRedo'
 
 /**
  * This plugin makes sure that the PTE maxBlocks prop is respected
@@ -18,6 +19,15 @@ export function createWithMaxBlocks(maxBlocks: number) {
         return
       }
 
+      /**
+       * We don't want to run any side effects when the editor is undoing or
+       * redoing operations.
+       */
+      if (isUndoing(editor) || isRedoing(editor)) {
+        apply(operation)
+        return
+      }
+
       const rows = maxBlocks
       if (rows > 0 && editor.children.length >= rows) {
         if (

packages/editor/src/editor/plugins/createWithObjectKeys.ts

@@ -2,7 +2,7 @@ import {Editor, Element, Node, Transforms} from 'slate'
 
 import {type PortableTextMemberSchemaTypes, type PortableTextSlateEditor} from '../../types/editor'
 import {isChangingRemotely} from '../../utils/withChanges'
-import {isPreservingKeys, PRESERVE_KEYS} from '../../utils/withPreserveKeys'
+import {isRedoing, isUndoing} from '../../utils/withUndoRedo'
 
 /**
  * This plugin makes sure that every new node in the editor get a new _key prop when created
@@ -13,11 +13,8 @@ export function createWithObjectKeys(
   keyGenerator: () => string,
 ) {
   return function withKeys(editor: PortableTextSlateEditor): PortableTextSlateEditor {
-    PRESERVE_KEYS.set(editor, false)
     const {apply, normalizeNode} = editor
 
-    // The apply function can be called with a scope (withPreserveKeys) that will
-    // preserve keys for the produced nodes if they have a _key property set already.
     // The default behavior is to always generate a new key here.
     // For example, when undoing and redoing we want to retain the keys, but
     // when we create a new bold span by splitting a non-bold-span we want the produced node to get a new key.
@@ -31,30 +28,34 @@ export function createWithObjectKeys(
         return
       }
 
-      if (operation.type === 'split_node') {
-        const withNewKey = !isPreservingKeys(editor) || !('_key' in operation.properties)
+      /**
+       * We don't want to run any side effects when the editor is undoing or
+       * redoing operations.
+       */
+      if (isUndoing(editor) || isRedoing(editor)) {
+        apply(operation)
+        return
+      }
 
+      if (operation.type === 'split_node') {
         apply({
           ...operation,
           properties: {
             ...operation.properties,
-            ...(withNewKey ? {_key: keyGenerator()} : {}),
+            _key: keyGenerator(),
           },
         })
 
         return
       }
 
       if (operation.type === 'insert_node') {
-        // Must be given a new key or adding/removing marks while typing gets in trouble (duped keys)!
-        const withNewKey = !isPreservingKeys(editor) || !('_key' in operation.node)
-
         if (!Editor.isEditor(operation.node)) {
           apply({
             ...operation,
             node: {
               ...operation.node,
-              ...(withNewKey ? {_key: keyGenerator()} : {}),
+              _key: keyGenerator(),
             },
           })
 

packages/editor/src/editor/plugins/createWithPatches.ts

@@ -27,7 +27,6 @@ import {fromSlateValue, isEqualToEmptyEditor} from '../../utils/values'
 import {IS_PROCESSING_REMOTE_CHANGES, KEY_TO_VALUE_ELEMENT} from '../../utils/weakMaps'
 import {withRemoteChanges} from '../../utils/withChanges'
 import {isPatching, PATCHING, withoutPatching} from '../../utils/withoutPatching'
-import {withPreserveKeys} from '../../utils/withPreserveKeys'
 import {withoutSaving} from './createWithUndoRedo'
 
 const debug = debugWithName('plugin:withPatches')
@@ -117,11 +116,9 @@ export function createWithPatches({
         Editor.withoutNormalizing(editor, () => {
           withoutPatching(editor, () => {
             withoutSaving(editor, () => {
-              withPreserveKeys(editor, () => {
-                patches.forEach((patch) => {
-                  if (debug.enabled) debug(`Handling remote patch ${JSON.stringify(patch)}`)
-                  changed = applyPatch(editor, patch)
-                })
+              patches.forEach((patch) => {
+                if (debug.enabled) debug(`Handling remote patch ${JSON.stringify(patch)}`)
+                changed = applyPatch(editor, patch)
               })
             })
           })

packages/editor/src/editor/plugins/createWithPlaceholderBlock.ts

@@ -4,6 +4,7 @@ import {type PortableTextSlateEditor} from '../../types/editor'
 import {type SlateTextBlock, type VoidElement} from '../../types/slate'
 import {debugWithName} from '../../utils/debug'
 import {isChangingRemotely} from '../../utils/withChanges'
+import {isRedoing, isUndoing} from '../../utils/withUndoRedo'
 
 const debug = debugWithName('plugin:withPlaceholderBlock')
 
@@ -27,6 +28,15 @@ export function createWithPlaceholderBlock(): (
         return
       }
 
+      /**
+       * We don't want to run any side effects when the editor is undoing or
+       * redoing operations.
+       */
+      if (isUndoing(editor) || isRedoing(editor)) {
+        apply(op)
+        return
+      }
+
       if (op.type === 'remove_node') {
         const node = op.node as SlateTextBlock | VoidElement
         if (op.path[0] === 0 && Editor.isVoid(editor, node)) {

packages/editor/src/editor/plugins/createWithPortableTextMarkModel.ts

@@ -20,6 +20,7 @@ import {debugWithName} from '../../utils/debug'
 import {toPortableTextRange} from '../../utils/ranges'
 import {EMPTY_MARKS} from '../../utils/values'
 import {isChangingRemotely} from '../../utils/withChanges'
+import {isRedoing, isUndoing} from '../../utils/withUndoRedo'
 
 const debug = debugWithName('plugin:withPortableTextMarkModel')
 
@@ -243,6 +244,15 @@ export function createWithPortableTextMarkModel(
         return
       }
 
+      /**
+       * We don't want to run any side effects when the editor is undoing or
+       * redoing operations.
+       */
+      if (isUndoing(editor) || isRedoing(editor)) {
+        apply(op)
+        return
+      }
+
       // Special hook before inserting text at the end of an annotation.
       if (op.type === 'insert_text') {
         const {selection} = editor

packages/editor/src/editor/plugins/createWithUndoRedo.ts

@@ -12,7 +12,7 @@ import {type Descendant, Editor, Operation, Path, type SelectionOperation, Trans
 import {type PatchObservable, type PortableTextSlateEditor} from '../../types/editor'
 import {debugWithName} from '../../utils/debug'
 import {fromSlateValue} from '../../utils/values'
-import {PRESERVE_KEYS, withPreserveKeys} from '../../utils/withPreserveKeys'
+import {setIsRedoing, setIsUndoing, withRedoing, withUndoing} from '../../utils/withUndoRedo'
 
 const debug = debugWithName('plugin:withUndoRedo')
 const debugVerbose = debug.enabled && false
@@ -150,7 +150,7 @@ export function createWithUndoRedo(
 
           try {
             Editor.withoutNormalizing(editor, () => {
-              withPreserveKeys(editor, () => {
+              withUndoing(editor, () => {
                 withoutSaving(editor, () => {
                   reversedOperations.forEach((op) => {
                     editor.apply(op)
@@ -166,7 +166,7 @@ export function createWithUndoRedo(
             Transforms.deselect(editor)
             editor.history = {undos: [], redos: []}
             SAVING.set(editor, true)
-            PRESERVE_KEYS.set(editor, false)
+            setIsUndoing(editor, false)
             editor.onChange()
             return
           }
@@ -196,7 +196,7 @@ export function createWithUndoRedo(
           })
           try {
             Editor.withoutNormalizing(editor, () => {
-              withPreserveKeys(editor, () => {
+              withRedoing(editor, () => {
                 withoutSaving(editor, () => {
                   // eslint-disable-next-line max-nested-callbacks
                   transformedOperations.forEach((op) => {
@@ -213,7 +213,7 @@ export function createWithUndoRedo(
             Transforms.deselect(editor)
             editor.history = {undos: [], redos: []}
             SAVING.set(editor, true)
-            PRESERVE_KEYS.set(editor, false)
+            setIsRedoing(editor, false)
             editor.onChange()
             return
           }

packages/editor/src/utils/withUndoRedo.ts

@@ -0,0 +1,34 @@
+import {type Editor} from 'slate'
+
+const IS_UDOING: WeakMap<Editor, boolean | undefined> = new WeakMap()
+const IS_REDOING: WeakMap<Editor, boolean | undefined> = new WeakMap()
+
+export function withUndoing(editor: Editor, fn: () => void) {
+  const prev = isUndoing(editor)
+  IS_UDOING.set(editor, true)
+  fn()
+  IS_UDOING.set(editor, prev)
+}
+
+export function isUndoing(editor: Editor) {
+  return IS_UDOING.get(editor) ?? false
+}
+
+export function setIsUndoing(editor: Editor, isUndoing: boolean) {
+  IS_UDOING.set(editor, isUndoing)
+}
+
+export function withRedoing(editor: Editor, fn: () => void) {
+  const prev = isRedoing(editor)
+  IS_REDOING.set(editor, true)
+  fn()
+  IS_REDOING.set(editor, prev)
+}
+
+export function isRedoing(editor: Editor) {
+  return IS_REDOING.get(editor) ?? false
+}
+
+export function setIsRedoing(editor: Editor, isRedoing: boolean) {
+  IS_REDOING.set(editor, isRedoing)
+}