yjs / y-prosemirror

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

yCursorPlugin exception when ySyncPlugin not initialized #156

Closed khrvati closed 1 month ago

khrvati commented 1 month ago

Issue

I'm using ySyncPlugin and yCursorPlugin together, to keep my editor up-to-date and to share cursor information through awareness. Sometimes, when a client is already connected to the same room and sharing their cursor position, trying to connect with a second client causes an exception

Uncaught TypeError TypeError: Cannot read properties of undefined (reading 'nodeSize')
    at relativePositionToAbsolutePosition (/Users/krunohrvatinic/ProjectRootDirName/node_modules/y-prosemirror/src/lib.js:183:63)
    at <anonymous> (/Users/krunohrvatinic/ProjectRootDirName/node_modules/y-prosemirror/src/plugins/cursor-plugin.js:101:20)
    at createDecorations (/Users/krunohrvatinic/ProjectRootDirName/node_modules/y-prosemirror/src/plugins/cursor-plugin.js:85:25)
    at init (/Users/krunohrvatinic/ProjectRootDirName/node_modules/y-prosemirror/src/plugins/cursor-plugin.js:165:16)
    at reconfigure (/Users/krunohrvatinic/ProjectRootDirName/node_modules/prosemirror-state/dist/index.js:868:81)
...

The exception happens because ystate.binding.mapping is an empty map before ySyncPlugin finishes initializing, and relativePositionToAbsolutePosition expects a full and initialized map.

Workaround

I've fixed the issue for myself by patching cursor-plugin.js

export const createDecorations = (state, awareness, awarenessFilter, createCursor, createSelection) => {
   const ystate = ySyncPluginKey.getState(state)
   const y = ystate.doc
   const decorations = []
   if (
     ystate.snapshot != null || ystate.prevSnapshot != null ||
-    ystate.binding === null
+    ystate.binding.mapping.size === 0
   ) {
     // do not render cursors while snapshot is active
     return DecorationSet.create(state.doc, [])
   }

This seems like the right thing to do because the null check will never return true. The null check was originally added in 1.1.3, when ySyncPlugin initialized binding to null. This was changed very recently in 1.2.7 - ySyncPlugin now initializes binding to new ProsemirrorBinding, so binding is never null.

Environment Information

I'm using y-prosemirror through Remirror, but am unaffected by #155 because my app waits until Y.Doc is loaded before loading ySyncPlugin I'm using Liveblocks as the provider.

dmonad commented 1 month ago

Thanks! I added a fix. This will be part of the next release.

khrvati commented 1 month ago

@dmonad Thank you!

If you're touching this part of the code anyway, there is a similar null-check here https://github.com/yjs/y-prosemirror/blob/f41a3e161448fe23e83a5ad69297274e4de7fb09/src/plugins/cursor-plugin.js#L207 If I got the root cause right, the check is a no-op

Leaving it in won't cause problems because absolutePositionToRelativePosition has a built-in way to handle an empty map https://github.com/yjs/y-prosemirror/blob/f41a3e161448fe23e83a5ad69297274e4de7fb09/src/lib.js#L82

...but you may want to remove or change it anyway?

mendesanna commented 1 month ago

Hello @dmonad I'm still having this exact issue even though I have this latest version:

TypeError: Cannot read properties of undefined (reading 'nodeSize')
    at relativePositionToAbsolutePosition (chunk-4AVB3MJ2.js?v=3879bf73:263:37)
    at chunk-4AVB3MJ2.js?v=3879bf73:1171:20
    at Map.forEach (<anonymous>)
    at createDecorations (chunk-4AVB3MJ2.js?v=3879bf73:1157:25)
    at Plugin.init (chunk-4AVB3MJ2.js?v=3879bf73:1215:14)
    at _EditorState.reconfigure (chunk-LUJVU6BP.js?v=3879bf73:2580:75)
    at Editor2.createView (chunk-XN66PWTN.js?v=3879bf73:9440:33)
    at new Editor (chunk-XN66PWTN.js?v=3879bf73:9261:10)
    at new Editor2 (@tiptap_react.js?v=411d6cd6:3312:5)
    at @tiptap_react.js?v=411d6cd6:3620:14

I'm using y-prosemirror through Tiptap and I've checked they are using the lastest version 1.2.9. Is there something else missing here? Can anyone help me?

nyacg commented 3 weeks ago

As above, we're also using y-prosemirror through tiptap and are on 1.2.9 and still experiencing this error

nyacg commented 2 weeks ago

Any update on this?

dmonad commented 2 weeks ago

The fix is released in y-prosemirror@1.2.10. Please make sure to use the latest version and fill out the issue template when you open another ticket.