CodeMirror combined with Y.UndoManager results in duplicate input

[hesselbom]

Describe the bug When deleting a character in CodeMirror editor, then undo-ing with Y.UndoManager, yjs reports that undo as two inputs. So even though the CodeMirror editor just shows one update, yjs "update" event gets called twice with two inputs making the resulting document value be incorrect.

My theory is something with Y.UndoManager reporting the undo, updating the Y.Text document and then updating CodeMirror, which recognizes an insert and reports that as a new insert operation. Or something similar, still investigating.

To Reproduce Steps to reproduce the behavior:

1. Run the project (files included under "Additional context")
2. Put cursor in the CodeMirror textarea so it gets focus
3. Write the letter "a", then hit backspace to delete it, then hit CMD-Z/CTRL-Z to undo the change
4. See console.log results, it will show:

   update: a
   update: 
   update: a
   update: aa

Expected behavior When undoing a delete, yjs should only insert the deleted character once, not twice. So the result in the above example should just read:

update: a
update: 
update: a

Screenshots

Environment Information

• Browser: Chrome
• yjs@13.4.1, y-codemirror@2.0.9, codemirror@5.58.1

Additional context Files used to replicate problem:

package.json

{
  "scripts": {
    "start": "parcel index.html"
  },
  "dependencies": {
    "codemirror": "5.58.1",
    "parcel-bundler": "1.12.4",
    "y-codemirror": "2.0.9",
    "yjs": "13.4.1"
  }
}

index.html

<div id="editor"></div>
<script src="./index.js"></script>

index.js

import CodeMirror from 'codemirror'
import 'codemirror/lib/codemirror.css'
import * as Y from 'yjs'
import { CodeMirrorBinding } from 'y-codemirror'

const editor = CodeMirror(document.getElementById('editor'), {})
const doc = new Y.Doc()
const yText = doc.getText('text')
const binding = new CodeMirrorBinding(yText, editor)
const undoManager = new Y.UndoManager(yText, { captureTimeout: 0, trackedOrigins: new Set([binding]) })

CodeMirror.commands.undo = () => undoManager.undo()
CodeMirror.commands.redo = () => undoManager.redo()

doc.on('update', () => console.log('update:', yText.toString()))

[hesselbom]

I've tried to understand why, when y-indexeddb loads stored data, and updates CodeMirror, we don't get the same problem with a CodeMirror update resulting in two inserts. It appears that inside y-codemirror we have a typeObserver (with updates from our YText) and targetObserver (with updates from our CodeMirror) and because they're using the same mutex (from lib0/mutex) and get the updates at the same time only the typeObserver update is called. Type updates first, then target.

So the question is now why, when undoing a change, we don't get the same effect, that both type and target are updated simultaneously and thus targetObserver being ignored thanks to mutex.

It looks like when undoing we get first a typeObserver update and then a targetObserver update but both are running so maybe there is something happening async here, making the first typeObserver update not mutex block the targetObserver update.

Will continue investigating.

[hesselbom]

I believe I've found a solution, even if I'm a little bit unsure exactly why it works. Looking at CodeMirror source, if there is a curOp object (which there isn't is when y-indexeddb does its load), the operation() method appears to buffer operations and waits to call endOperation(), probably until no more operations are added. endOperation() flushes changes to the DOM.

So tried adding cm.endOperation() inside y-codemirror on line 69 like this:

    // if possible, bundle the changes using cm.operation
    if (cm) {
      cm.operation(performChange)
      cm.endOperation() // <-- new line
    } else {
      performChange()
    }

And doing that for the reproducable demo I shared in my original post seems to solve the problem. I think this is a good solution but because I'm note 100% sure of all the side-effects of doing this I'll probably wait a bit for a PR until I've tested it more.