yjs / y-prosemirror

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

TypeError: Cannot read property 'mapping' of null #42

Closed ivan-topp closed 2 years ago

ivan-topp commented 3 years ago

Hello, this is the first issue that I do, first of all I would like to thank the amazing and brilliant work that has been done for the development of this library. It is a very powerful library and it has somehow saved me.

Description of the bug

I am currently using the prosemirror editor together with Yjs in an application made in React. In this application I have developed my own component that creates a tiptap editor (uses prosemirror, I was guided by the examples recommended in the documentation), this since I need to be able to add and remove editors dynamically. The error occurs on line 127 of the file "cursors-plugin.js" of the library "y-prosemirror" located in "[...]/node_modules/y-prosemirror/src/plugins/cursor-plugin.js: 127". And it happens when a user adds a new editor (input) while another user is editing in another editor (input).

To reproduce Due to the context, it is a case that is perhaps difficult to reproduce, however broadly the steps to reproduce the behavior are:

  1. Create a project in React.
  2. Create a context that stores the y-document and provider.
  3. Create a component that instantiates a new TipTap editor in the document.
  4. Generate for each element of an array (it can be an array with numbers) a component (the one that was created previously).
  5. Create a button to add a new element to the array.
  6. Open two tabs in the browser.
  7. In a tab, leave the cursor inside one of the editors already created according to the array.
  8. In the other tab, press the button that adds a new element to the array.
  9. See the error in the console.

Expected behavior The desired behavior is that if a user is editing a field and another user adds a new element to an array (thus a new editor is generated), the application does not break and that user who was editing a field can continue editing it.

Screenshots

image

Environment Information

Proposed Solution The solution that worked for me has been to change line 127 of the file "[...]/node_modules/y-prosemirror/src/plugins/cursor-plugin.js" which was like this:

} else if (current.cursor != null && relativePositionToAbsolutePosition (ystate.doc, ystate.type, Y.createRelativePositionFromJSON (current.cursor.anchor), ystate.binding.mapping) !== null) {

Adding the validation "ystate.binding!= Null":

} else if (current.cursor != null && ystate.binding != null && relativePositionToAbsolutePosition (ystate.doc, ystate.type, Y.createRelativePositionFromJSON (current.cursor.anchor), ystate.binding.mapping) !== null) {

However, I would like to have an answer or opinion as to whether what I just did is okay or not. Thank you very much in advance for your time.

martinpengellyphillips commented 3 years ago

Not sure if helpful, but I've also experienced this error under slightly different conditions. I have multiple (tiptap2) editors in a view, backed by different fragments of the same ydoc document and the same connection provider.

The same error (caught TypeError: Cannot read property 'mapping' of null) occurs if I use the autofocus: true option on one of the editors. If instead I run my own focus command with a longer delay (15 ms) then the error does not occur.

BrianHung commented 3 years ago

If I had to guess, it might be with this asynchronous code in the sync-plugin:

      const binding = new ProsemirrorBinding(yXmlFragment, view)
      // Make sure this is called in a separate context
      setTimeout(() => {
        binding._forceRerender()
        view.dispatch(view.state.tr.setMeta(ySyncPluginKey, { binding }))
      }, 0)

I've noticed previously that there's some finicky behavior with plugin views asynchronously dispatching transactions in the initialization method (I encountered previously loading syntax highlighting languages). It is legal to do it though: hence why this is binded by default in dispatchTransaction.

nc commented 2 years ago

Has anyone got a fix for this? We're encountering it in an app using multiple prosemirrors.

dmonad commented 2 years ago

Thank you @IvanTopp for your suggestion!

I don't expect updateCursorInfo to be called after the binding has been destroyed. But it seems, that it is - as Brian suggested - because of the timeout. I simply return now when the binding doesn't exist which should fix the issue. If somebody can still reproduce this, pls reopen this ticket.

I'll publish a new release in a bit.