[lexical-yjs] Bug Fix: Properly sync when emptying document via undo (#…

…6523) Co-authored-by: Ivaylo Pavlov <[email protected]> Co-authored-by: Bob Ippolito <[email protected]>
facebook · Aug 22, 2024 · 25f543e · 25f543e
1 parent fb82331
commit 25f543e
Show file tree

Hide file tree

Showing 3 changed files with 121 additions and 10 deletions.
diff --git a/packages/lexical-react/src/__tests__/unit/Collaboration.test.ts b/packages/lexical-react/src/__tests__/unit/Collaboration.test.ts
@@ -6,7 +6,14 @@
  *
  */
 
-import {$createTextNode, $getRoot, ParagraphNode, TextNode} from 'lexical';
+import {
+  $createParagraphNode,
+  $createTextNode,
+  $getRoot,
+  ParagraphNode,
+  TextNode,
+  UNDO_COMMAND,
+} from 'lexical';
 
 import {Client, createTestConnection, waitForReact} from './utils';
 
@@ -312,4 +319,94 @@ describe('Collaboration', () => {
     client1.stop();
     client2.stop();
   });
+
+  /**
+   * When a document is not bootstrapped (via `shouldBootstrap`), the document only initializes the initial paragraph
+   * node upon the first user interaction. Then, both a new paragraph as well as the user character are inserted as a
+   * single Yjs change. However, when the user undos this initial change, the document now has no initial paragraph
+   * node. syncYjsChangesToLexical addresses this by doing a check: `$getRoot().getChildrenSize() === 0)` and if true,
+   * inserts the paragraph node. However, this insertion was previously being done in an editor.update block that had
+   * either the tag 'collaboration' or 'historic'. Then, when `syncLexicalUpdateToYjs` was called, because one of these
+   * tags were present, the function would early-return, and this change would not be synced to other clients, causing
+   * permanent desync and corruption of the doc for both users. Not only was the change not syncing to other clients,
+   * but even the initiating client was not notified via the proper callbacks, and the change would fall through from
+   * persistence, causing permanent desync. The fix was to move the insertion of the paragraph node outside of the
+   * editor.update block that included the 'collaboration' or 'historic' tag, and instead insert it in a separate
+   * editor.update block that did not have these tags.
+   */
+  it('Should sync to other clients when inserting a new paragraph node when document is emptied via undo', async () => {
+    const connector = createTestConnection();
+
+    const client1 = connector.createClient('1');
+    const client2 = connector.createClient('2');
+
+    client1.start(container!, undefined, {shouldBootstrapEditor: false});
+    client2.start(container!, undefined, {shouldBootstrapEditor: false});
+
+    expect(client1.getHTML()).toEqual('');
+    expect(client1.getHTML()).toEqual(client2.getHTML());
+
+    // Wait for clients to render the initial content
+    await Promise.resolve().then();
+
+    expect(client1.getHTML()).toEqual('');
+    expect(client1.getHTML()).toEqual(client2.getHTML());
+
+    await waitForReact(() => {
+      client1.update(() => {
+        const root = $getRoot();
+
+        // Since bootstrap is false, we create our own paragraph node
+        const paragraph = $createParagraphNode();
+        const text = $createTextNode('Hello');
+        paragraph.append(text);
+
+        root.append(paragraph);
+      });
+    });
+
+    expect(client1.getHTML()).toEqual(
+      '<p dir="ltr"><span data-lexical-text="true">Hello</span></p>',
+    );
+    expect(client1.getHTML()).toEqual(client2.getHTML());
+    expect(client1.getDocJSON()).toEqual({
+      root: '[object Object]Hello',
+    });
+    expect(client1.getDocJSON()).toEqual(client2.getDocJSON());
+
+    await waitForReact(() => {
+      // Undo the insertion of the initial paragraph and text node
+      client1.getEditor().dispatchCommand(UNDO_COMMAND, undefined);
+    });
+
+    // We expect the safety check in syncYjsChangesToLexical to
+    // insert a new paragraph node and prevent the document from being empty
+    expect(client1.getHTML()).toEqual('<p><br></p>');
+    expect(client1.getHTML()).toEqual(client2.getHTML());
+    expect(client1.getDocJSON()).toEqual(client2.getDocJSON());
+
+    await waitForReact(() => {
+      client1.update(() => {
+        const root = $getRoot();
+
+        const paragraph = $createParagraphNode();
+        const text = $createTextNode('Hello world');
+        paragraph.append(text);
+
+        root.append(paragraph);
+      });
+    });
+
+    expect(client1.getHTML()).toEqual(
+      '<p><br></p><p dir="ltr"><span data-lexical-text="true">Hello world</span></p>',
+    );
+    expect(client1.getHTML()).toEqual(client2.getHTML());
+    expect(client1.getDocJSON()).toEqual({
+      root: '[object Object]Hello world',
+    });
+    expect(client1.getDocJSON()).toEqual(client2.getDocJSON());
+
+    client1.stop();
+    client2.stop();
+  });
 });
diff --git a/packages/lexical-react/src/__tests__/unit/utils.tsx b/packages/lexical-react/src/__tests__/unit/utils.tsx
@@ -27,11 +27,13 @@ function Editor({
   provider,
   setEditor,
   awarenessData,
+  shouldBootstrapEditor = true,
 }: {
   doc: Y.Doc;
   provider: Provider;
   setEditor: (editor: LexicalEditor) => void;
   awarenessData?: object | undefined;
+  shouldBootstrapEditor?: boolean;
 }) {
   const context = useCollaborationContext();
 
@@ -48,7 +50,7 @@ function Editor({
       <CollaborationPlugin
         id="main"
         providerFactory={() => provider}
-        shouldBootstrap={true}
+        shouldBootstrap={shouldBootstrapEditor}
         awarenessData={awarenessData}
       />
       <RichTextPlugin
@@ -148,7 +150,15 @@ export class Client implements Provider {
     this._connected = false;
   }
 
-  start(rootContainer: Container, awarenessData?: object) {
+  /**
+   * @param options
+   *  - shouldBootstrapEditor: Whether to initialize the editor with an empty paragraph
+   */
+  start(
+    rootContainer: Container,
+    awarenessData?: object,
+    options: {shouldBootstrapEditor?: boolean} = {},
+  ) {
     const container = document.createElement('div');
     const reactRoot = createRoot(container);
     this._container = container;
@@ -162,15 +172,16 @@ export class Client implements Provider {
           initialConfig={{
             editorState: null,
             namespace: '',
-            onError: () => {
-              throw Error();
+            onError: (e) => {
+              throw e;
             },
           }}>
           <Editor
             provider={this}
             doc={this._doc}
             setEditor={(editor) => (this._editor = editor)}
             awarenessData={awarenessData}
+            shouldBootstrapEditor={options.shouldBootstrapEditor}
           />
         </LexicalComposer>,
       );

diff --git a/packages/lexical-yjs/src/SyncEditorStates.ts b/packages/lexical-yjs/src/SyncEditorStates.ts
@@ -100,11 +100,6 @@ export function syncYjsChangesToLexical(
         const event = events[i];
         $syncEvent(binding, event);
       }
-      // If there was a collision on the top level paragraph
-      // we need to re-add a paragraph
-      if ($getRoot().getChildrenSize() === 0) {
-        $getRoot().append($createParagraphNode());
-      }
 
       const selection = $getSelection();
 
@@ -135,6 +130,14 @@ export function syncYjsChangesToLexical(
     {
       onUpdate: () => {
         syncCursorPositions(binding, provider);
+        // If there was a collision on the top level paragraph
+        // we need to re-add a paragraph. To ensure this insertion properly syncs with other clients,
+        // it must be placed outside of the update block above that has tags 'collaboration' or 'historic'.
+        editor.update(() => {
+          if ($getRoot().getChildrenSize() === 0) {
+            $getRoot().append($createParagraphNode());
+          }
+        });
       },
       skipTransforms: true,
       tag: isFromUndoManger ? 'historic' : 'collaboration',