y-prosemirror icon indicating copy to clipboard operation
y-prosemirror copied to clipboard

y-prosemirror fires two updates for each change

Open ocavue opened this issue 3 years ago • 2 comments

Please save me some time and use the following template. In 90% of all issues I can't reproduce the problem because I don't know what exactly you are doing, in which environment, or which y-* version is responsible. Just use the following template even if you think the problem is obvious.

Checklist

  • [x] Are you reporting a bug? Use github issues for bug reports and feature requests. For general questions, please use https://discuss.yjs.dev/
  • [x] 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

After updating y-prosemirror from 1.0.20 to 1.1.0, every keypress will trigger the update method for every plugin twice. This causes a lot of issues if a plugin want to compare the current state with the prev state.

To Reproduce

I wrote a minimal reproduction to show this issue. In this example, I have a ProseMirror plugin to show whether or not the cursor is changed in the last operation. The code of this plugin is in prosemirror.ts.

  1. Open https://stackblitz.com/github/ocavue/y-prosemirror-playground/tree/issue-double-update-1.0.20 and https://stackblitz.com/github/ocavue/y-prosemirror-playground/tree/issue-double-update-1.1.2. They have the same code but with different versions of y-prosemirror
  2. Click the Disconnect button. We don't want them to affect each other.
  3. Typing something.
  4. In the 1.0.20 window, you will see Is your cursor changed? true. While in the 1.1.2 window, you will see false.
  5. Open the console. Every keypress will trigger one plugin update in 1.0.20, but two updates in 1.1.2.

Expected behavior

Every keypress will only trigger one plugin update, as what we used to have in y-prosemirror 1.0.20

Screenshots

https://share.cleanshot.com/NJlBDh

Additional context

In commit 2d06ece86f8359b14e9f6b27bb5746e2ca2efc82, pluginState.doc.transact was added inside syncPlugin's update function. Calling pluginState.doc.transact will trigger _typeChanged because it has been observed. tr.replace will be called in _typeChanged, which causes the second update.

ocavue avatar Jun 17 '22 10:06 ocavue

This issue should share the same underlying reason as https://github.com/yjs/y-prosemirror/issues/113

ocavue avatar Jun 17 '22 11:06 ocavue

+1 - It appears now that due to the same commit, NodeSelections are converted to TextSelections anytime the document changes, as the _typeChanged mux function ends up getting called and actually replaces the entire document (like a remote change).

jamesopti avatar Jun 20 '22 04:06 jamesopti

Any updates on this one?

jamesopti avatar Aug 22 '22 03:08 jamesopti

This one seems quite important @dmonad - and I think @ocavue has a PR that fixes it.

maccman avatar Aug 24 '22 02:08 maccman

Fixed and released :+1:

dmonad avatar Aug 24 '22 14:08 dmonad

Amazing! Thank you.

maccman avatar Aug 24 '22 15:08 maccman