yjs / y-prosemirror

ProseMirror editor binding for Yjs
https://demos.yjs.dev/prosemirror/prosemirror.html
MIT License
355 stars 124 forks source link

Some text is duplicated when two users concurrently make changes at the beginning of the same line #161

Closed MacroMackie closed 3 months ago

MacroMackie commented 4 months ago

Checklist

Describe the bug When using y-prosemirror, and two users make a few edits to a line at the exact same line, I sometimes see some text duplicated. (See attached video for more info).

To Reproduce Steps to reproduce the behavior:

You'll notice that one of the characters becomes duplicated.

Expected behavior No duplication - some kind of tie-breaking logic is used to figure out which edits land on the left/right.

Screenshots

I've reproduced the issue on the y-prosemirror demo environment ( https://demos.yjs.dev/prosemirror/prosemirror.html )

https://github.com/user-attachments/assets/4b61e47a-7db0-498d-9493-9e46a832206c

I've also been able to reproduce it in other prosemirror-based environments (e.g. TipTap - https://templates.tiptap.dev )

https://github.com/user-attachments/assets/cb19950b-95f6-47b8-82c1-19ada0ee1eeb

Environment Information I can reproduce in chrome (126.0.6478.127), as well as safari, and firefox.

Locally, I encountered this issue with:

Additional context I've also created a pull-request here with a test case which seems to reproduce the issue - https://github.com/yjs/y-prosemirror/pull/160 ( I'm not too familiar with the y.js internals, so this might not be accurate. )

Let me know if you need any other information!

dmonad commented 3 months ago

I fixed the issue and will make a new release shortly.

The issue is related to an internal cleanup function. The first concurrent edits create two Y.Text elements. The second concurrent edit triggers a cleanup function where both clients want to move the content from the second Y.Text to the first. This leads to duplication.

I fixed the issue by forcing the author of the second Y.Text to move the content directly after it notices a concurrent edit.

This will be good enough for most cases. But for change-tracking, I might need to implement a better fix.

MacroMackie commented 3 months ago

Amazing - thank you!