yjs / y-prosemirror

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

yCollabPlugin does not work when the input is a yXmlFragment nested inside a shared type #150

Closed harrisjose closed 8 months ago

harrisjose commented 8 months ago

Describe the bug The ySyncPlugin plugin does not seem to work when the yXmlFragment is not at the root of the yDoc. I was trying to nest the xmlFragment within a yMap.

To Reproduce Steps to reproduce the behavior:

  1. In the y-prosemirror demo, open prosemirror.js
  2. Create a yMap and a yXmlFragment within it
    const pages = ydoc.getMap('pages');
    if (!pages.get('prosemirror')) {
    pages.set('prosemirror', new Y.XmlFragment());
    }
  3. Change the input to ySyncPlugin
    ySyncPlugin(pages.get('prosemirror', Y.XmlFragment))

    Expected behavior The plugin should be able to sync updates even if the yXmlFragment is nested within other shared types. (https://discuss.yjs.dev/t/y-prosemirror-usage/1357)

Environment Information

dmonad commented 8 months ago

It looks like you are overwriting the "prosemirror" property on every pageload.

  const ydoc = new Y.Doc()
  const provider = new WebsocketProvider(ydoc, ...)
  The provider automatically syncs, but this will take some time...
  const pages = ydoc.getMap('pages');
  // pages will always be empty, so we always set a new YXmlFragment
  if (!pages.get('prosemirror')) {
    pages.set('prosemirror', new Y.XmlFragment());
  }
  // you are now listening to the current YXmlType
  ySyncPlugin(pages.get('prosemirror', /** not necessary: Y.XmlFragment */))
  const currentPage = pages.get('prosemirror')

  // eventually.. some time later.. the provider syncs with the other clients. 
  // Now there are two conflicting Y.XmlFragments under the key "prosemirror". 
  // The other client will use the overwritten type (which won't sync anymore).
  currentPage !== pages.get('prosemirror') // depending on which client "wins"

I recommend using top-level types when appropriate. If you have a directory of documents, each page should only be created once (no if condition checking if the document already exists).

harrisjose commented 8 months ago

Thanks for the detailed explanation and for your work on this plugin 🙏🏽