TypeCellOS / BlockNote

A React Rich Text Editor that's block-based (Notion style) and extensible. Built on top of Prosemirror and Tiptap.
https://www.blocknotejs.org/
Mozilla Public License 2.0
6.57k stars 450 forks source link

yjs XmlFragment clone doesn't clone heading level attribute #1123

Open lihebi opened 2 weeks ago

lihebi commented 2 weeks ago

Describe the bug

When used with yjs, the Y.XmlFragment cannot be cloned correctly. The "level" attribute of "heading" is missing.

To Reproduce

Here's the sandbox: https://stackblitz.com/edit/github-wwxgqp?file=App.tsx

The code is a minimal BlockNote setup with yjs.

import * as Y from "yjs";
import { WebrtcProvider } from "y-webrtc";

const ydoc = new Y.Doc();
// clients connected to the same room-name share document updates
const provider = new WebrtcProvider("your-room-name", ydoc, {
  password: "optional-room-password",
});
const yxml = ydoc.getXmlFragment("document-store");

export default function App() {
  // Creates a new editor instance.
  const editor = useCreateBlockNote({
    collaboration: {
      // The Yjs Provider responsible for transporting updates:
      provider,
      // Where to store BlockNote data in the Y.Doc:
      fragment: yxml,
      // Information (name and color) for this user:
      user: {
        name: "My Username",
        color: "#ff0000",
      },
    },
  });

  // Renders the editor instance using a React component.
  return (
    <div
      style={{
        width: "600px",
        height: "400px",
        border: "1px solid black",
      }}
    >
      <BlockNoteView
        editor={editor}
        onChange={() => {
          // clone yxml
          const yxml2 = yxml.clone();
          ydoc.getMap("test").set("1", yxml2);
          // print out yxml and yxml2
          console.log("yxml", yxml.toString());
          console.log("yxml2", yxml2.toString());
        }}
      />
    </div>
  );
}

Type some different headings (level 1,2,3), the output yxml and cloned yxml2 are different, yxml2 doesn't have the level attributes.

yxml

<blockgroup
  ><blockcontainer
    backgroundColor="default"
    id="initialBlockId"
    textColor="default"
    ><heading level="1" textAlignment="left">h1</heading></blockcontainer
  ><blockcontainer
    backgroundColor="default"
    id="ea98008e-ec95-4f19-8bdd-9d17a512f492"
    textColor="default"
    ><heading level="2" textAlignment="left">h2</heading></blockcontainer
  ><blockcontainer
    backgroundColor="default"
    id="ca1a55b2-646e-429f-b18d-6919e03d3e6c"
    textColor="default"
    ><heading level="3" textAlignment="left">h3</heading></blockcontainer
  ><blockcontainer
    backgroundColor="default"
    id="8972b9c9-9458-4ad7-a545-e0cad56305ae"
    textColor="default"
    ><paragraph textAlignment="left"></paragraph></blockcontainer
></blockgroup>

yxml2

<blockgroup
  ><blockcontainer
    backgroundColor="default"
    id="initialBlockId"
    textColor="default"
    ><heading textAlignment="left">h1</heading></blockcontainer
  ><blockcontainer
    backgroundColor="default"
    id="ea98008e-ec95-4f19-8bdd-9d17a512f492"
    textColor="default"
    ><heading textAlignment="left">h2</heading></blockcontainer
  ><blockcontainer
    backgroundColor="default"
    id="ca1a55b2-646e-429f-b18d-6919e03d3e6c"
    textColor="default"
    ><heading textAlignment="left">h3</heading></blockcontainer
  ><blockcontainer
    backgroundColor="default"
    id="8972b9c9-9458-4ad7-a545-e0cad56305ae"
    textColor="default"
    ><paragraph textAlignment="left"></paragraph></blockcontainer
></blockgroup>
lihebi commented 2 weeks ago

It doesn't seem to be a problem of yjs:

import * as Y from "yjs";

function test() {
  const ydoc = new Y.Doc();
  const yxml1 = ydoc.getXmlFragment("my xml fragment");
  // construct: <heading level="1" textAlignment="left">h1</heading>
  const elem = new Y.XmlElement("heading");
  elem.setAttribute("level", "1");
  elem.setAttribute("textAlignment", "left");
  elem.insert(0, [new Y.XmlText("h1")]);
  yxml1.insert(0, [elem]);

  console.log("yxml1", yxml1.toString());

  // clone it
  const yxml2 = yxml1.clone();
  const testMap = ydoc.getMap("test").set("1", yxml2);
  console.log("yxml2", yxml2.toString());
}

Output:

yxml1 <heading level="1" textAlignment="left">h1</heading>
yxml2 <heading level="1" textAlignment="left">h1</heading>
YousefED commented 2 weeks ago

Great find. Seems to be caused here:

image

It seems like Yjs doesn't allow number attributes, and the level prop incorrectly gets set as a number value. I think we should fix this by making sure all node attributes are first converted to strings.

What are you working on btw?

lihebi commented 2 weeks ago

Ah, I see. Thanks for the quick debugging!

I'm building a collaborative IDE, and I'm adding the feature to duplicate/copy/paste an editor. I could use editor.document, but yjs.clone would be simpler since the data is stored in Y.XmlFragment.