UndoManager: Undoing combination of Y.Map#set and deleting that Y.Map from parent Y.Array results in 1 client with invalid Y.Map
Checklist
- Are you reporting a bug? Use github issues for bug reports and feature requests. For general questions, please use https://discuss.yjs.dev/
- Try to report your issue in the correct repository. Yjs consists of many modules. When in doubt, report it to https://github.com/yjs/yjs/issues/
Describe the bug When undoing the combination of map set operation together with an operation that deletes that map from its parent array and then synchronizing, the result is that the other client ends up in a bad and different state than the client that did the undo. That other client ends up with the changed property missing in the YMap instead of being changed back.
So 1 the other client (the one not doing the undo, but receiving the sync result afterwards) ends up in a bad state.
To Reproduce Run the test code below (also on my fork in undo-redo.bug-reproduction.spec.js)
Reproduction Steps Summary:
- Setup 2 clients and an UndoManager
- add a YArray with a YMap with 2 properties
- use stopCapturing on UndoManager in order to group the next 2 operations
- change 1 of the 2 properties of the YMap
- delete the YMap from the YArray
- sync clients
- call undo() on UndoManager
- sync clients Expected: YMap is added back to the YArray and has the old content again on both clients Actual: The other client that is synchronized ends up in a bad state: the value that was changed using set in the YMap is missing there
Workaround Issue can be kind of worked around by splitting up the operations in the undo manager by using stopCapturing unbetween. when the 2 operations are undone separately, the issue is not reproducible.
Code:
import { init } from '../testHelper.js'
import * as Y from 'yjs'
import * as t from 'lib0/testing'
/**
* @param {t.TestCase} tc
*/
export const testUndoMapSetAndDeleteFromArray = tc => {
const { testConnector, array0, array1 } = init(tc, { users: 3 })
const undoManager = new Y.UndoManager(array0)
const mapInArray = new Y.Map()
mapInArray.set('untouchedProp', 'untouched prop value')
const toBeChangedPropKey = 'toBeChangedProp'
mapInArray.set(toBeChangedPropKey, 'before change')
array0.push([mapInArray])
testConnector.syncAll()
// START UNDO BLOCK
undoManager.stopCapturing()
// Change some value in the YMap
mapInArray.set(toBeChangedPropKey, 'changed content')
testConnector.syncAll()
// undoManager.stopCapturing() // Workaround is to split up the undo and then doing undo twice
// Delete the YMap from its parent Array
array0.delete(0, 1)
testConnector.syncAll()
// Undo the set + deletion from parent in 1 undo
undoManager.undo()
// undoManager.undo() // Workaround is to split up the undo and then doing undo twice
testConnector.syncAll()
t.assert(array0.toJSON()[0][toBeChangedPropKey] === 'before change') // OK
t.assert(array1.toJSON()[0][toBeChangedPropKey] === 'before change') // Failing and undefined instead
}
Source: yjs/yjs