Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

yCursorPlugin exception when ySyncPlugin not initialized #156

Closed
2 tasks
khrvati opened this issue Jun 24, 2024 · 6 comments
Closed
2 tasks

yCursorPlugin exception when ySyncPlugin not initialized #156

khrvati opened this issue Jun 24, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@khrvati
Copy link

khrvati commented Jun 24, 2024

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

  • MacOS, happens in Safari, Firefox and Chrome
  • yjs@13.6.14
  • y-prosemirror@1.2.8

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.

  • I'm a sponsor 💖
  • This issue is a blocker for my project.
@khrvati khrvati added the bug Something isn't working label Jun 24, 2024
@dmonad dmonad closed this as completed in f41a3e1 Jun 25, 2024
@dmonad
Copy link
Member

dmonad commented Jun 25, 2024

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

@khrvati
Copy link
Author

khrvati commented Jun 25, 2024

@dmonad Thank you!

If you're touching this part of the code anyway, there is a similar null-check here

if (ystate.binding == null) {

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

const pNodeSize = /** @type {any} */ (mapping.get(n) || { nodeSize: 0 }).nodeSize

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

@mendesanna
Copy link

mendesanna commented Jul 23, 2024

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
Copy link

nyacg commented Jul 28, 2024

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

@nyacg
Copy link

nyacg commented Aug 2, 2024

Any update on this?

@dmonad
Copy link
Member

dmonad commented Aug 5, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants