Proper follow redones in nested redos - fixes #317

src/structs/Item.js

@@ -125,12 +125,13 @@ export const splitItem = (transaction, leftItem, diff) => {
  * @param {Transaction} transaction The Yjs instance.
  * @param {Item} item
  * @param {Set<Item>} redoitems
+ * @param {Array<Item>} itemsToDelete
  *
  * @return {Item|null}
  *
  * @private
  */
-export const redoItem = (transaction, item, redoitems) => {
+export const redoItem = (transaction, item, redoitems, itemsToDelete) => {
   const doc = transaction.doc
   const store = doc.store
   const ownClientID = doc.clientID
@@ -170,7 +171,7 @@ export const redoItem = (transaction, item, redoitems) => {
   // make sure that parent is redone
   if (parentItem !== null && parentItem.deleted === true && parentItem.redone === null) {
     // try to undo parent if it will be undone anyway
-    if (!redoitems.has(parentItem) || redoItem(transaction, parentItem, redoitems) === null) {
+    if (!redoitems.has(parentItem) || redoItem(transaction, parentItem, redoitems, itemsToDelete) === null) {
       return null
     }
   }
@@ -209,6 +210,11 @@ export const redoItem = (transaction, item, redoitems) => {
       }
       right = right.right
     }
+    // Iterate right while right is in itemsToDelete
+    // If it is intended to delete right while item is redone, we can expect that item should replace right.
+    while (left !== null && left.right !== null && left.right !== right && itemsToDelete.findIndex(d => d === /** @type {Item} */ (left).right) >= 0) {
+      left = left.right
+    }
   }
   const nextClock = getState(store, ownClientID)
   const nextId = createID(ownClientID, nextClock)

src/types/YMap.js

@@ -36,11 +36,11 @@ export class YMapEvent extends YEvent {
 }
 
 /**
- * @template T number|string|Object|Array|Uint8Array
+ * @template MapType
  * A shared Map implementation.
  *
- * @extends AbstractType<YMapEvent<T>>
- * @implements {Iterable<T>}
+ * @extends AbstractType<YMapEvent<MapType>>
+ * @implements {Iterable<MapType>}
  */
 export class YMap extends AbstractType {
   /**
@@ -85,7 +85,7 @@ export class YMap extends AbstractType {
   }
 
   /**
-   * @return {YMap<T>}
+   * @return {YMap<MapType>}
    */
   clone () {
     const map = new YMap()
@@ -108,11 +108,11 @@ export class YMap extends AbstractType {
   /**
    * Transforms this Shared Type to a JSON object.
    *
-   * @return {Object<string,T>}
+   * @return {Object<string,any>}
    */
   toJSON () {
     /**
-     * @type {Object<string,T>}
+     * @type {Object<string,MapType>}
      */
     const map = {}
     this._map.forEach((item, key) => {
@@ -163,11 +163,11 @@ export class YMap extends AbstractType {
   /**
    * Executes a provided function on once on every key-value pair.
    *
-   * @param {function(T,string,YMap<T>):void} f A function to execute on every element of this YArray.
+   * @param {function(MapType,string,YMap<MapType>):void} f A function to execute on every element of this YArray.
    */
   forEach (f) {
     /**
-     * @type {Object<string,T>}
+     * @type {Object<string,MapType>}
      */
     const map = {}
     this._map.forEach((item, key) => {
@@ -179,7 +179,7 @@ export class YMap extends AbstractType {
   }
 
   /**
-   * @return {IterableIterator<T>}
+   * @return {IterableIterator<MapType>}
    */
   [Symbol.iterator] () {
     return this.entries()
@@ -204,7 +204,7 @@ export class YMap extends AbstractType {
    * Adds or updates an element with a specified key and value.
    *
    * @param {string} key The key of the element to add to this YMap
-   * @param {T} value The value of the element to add
+   * @param {MapType} value The value of the element to add
    */
   set (key, value) {
     if (this.doc !== null) {
@@ -221,7 +221,7 @@ export class YMap extends AbstractType {
    * Returns a specified element from this YMap.
    *
    * @param {string} key
-   * @return {T|undefined}
+   * @return {MapType|undefined}
    */
   get (key) {
     return /** @type {any} */ (typeMapGet(this, key))

src/utils/UndoManager.js

@@ -88,7 +88,7 @@ const popStackItem = (undoManager, stack, eventType) => {
         }
       })
       itemsToRedo.forEach(struct => {
-        performedChange = redoItem(transaction, struct, itemsToRedo) !== null || performedChange
+        performedChange = redoItem(transaction, struct, itemsToRedo, itemsToDelete) !== null || performedChange
       })
       // We want to delete in reverse order so that children are deleted before
       // parents, so we have more information available when items are filtered.

tests/undo-redo.tests.js

@@ -301,3 +301,58 @@ export const testUndoUntilChangePerformed = tc => {
   undoManager.undo()
   t.compareStrings(yMap2.get('key'), 'value')
 }
+
+/**
+ * This issue has been reported in https://github.com/yjs/yjs/issues/317
+ * @param {t.TestCase} tc
+ */
+export const testUndoNestedUndoIssue = tc => {
+  const doc = new Y.Doc({ gc: false })
+  const design = doc.getMap()
+  const undoManager = new Y.UndoManager(design, { captureTimeout: 0 })
+
+  /**
+   * @type {Y.Map<any>}
+   */
+  const text = new Y.Map()
+
+  const blocks1 = new Y.Array()
+  const blocks1block = new Y.Map()
+
+  doc.transact(() => {
+    blocks1block.set('text', 'Type Something')
+    blocks1.push([blocks1block])
+    text.set('blocks', blocks1block)
+    design.set('text', text)
+  })
+
+  const blocks2 = new Y.Array()
+  const blocks2block = new Y.Map()
+  doc.transact(() => {
+    blocks2block.set('text', 'Something')
+    blocks2.push([blocks2block])
+    text.set('blocks', blocks2block)
+  })
+
+  const blocks3 = new Y.Array()
+  const blocks3block = new Y.Map()
+  doc.transact(() => {
+    blocks3block.set('text', 'Something Else')
+    blocks3.push([blocks3block])
+    text.set('blocks', blocks3block)
+  })
+
+  t.compare(design.toJSON(), { text: { blocks: { text: 'Something Else' } } })
+  undoManager.undo()
+  t.compare(design.toJSON(), { text: { blocks: { text: 'Something' } } })
+  undoManager.undo()
+  t.compare(design.toJSON(), { text: { blocks: { text: 'Type Something' } } })
+  undoManager.undo()
+  t.compare(design.toJSON(), { })
+  undoManager.redo()
+  t.compare(design.toJSON(), { text: { blocks: { text: 'Type Something' } } })
+  undoManager.redo()
+  t.compare(design.toJSON(), { text: { blocks: { text: 'Something' } } })
+  undoManager.redo()
+  t.compare(design.toJSON(), { text: { blocks: { text: 'Something Else' } } })
+}