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

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

Merged
merged 7 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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();
});
});
19 changes: 15 additions & 4 deletions packages/lexical-react/src/__tests__/unit/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -48,7 +50,7 @@ function Editor({
<CollaborationPlugin
id="main"
providerFactory={() => provider}
shouldBootstrap={true}
shouldBootstrap={shouldBootstrapEditor}
awarenessData={awarenessData}
/>
<RichTextPlugin
Expand Down Expand Up @@ -148,7 +150,15 @@ export class Client implements Provider {
this._connected = false;
}

start(rootContainer: Container, awarenessData?: object) {
/**
* @param options
* - shouldBootstrapCollab: Whether to initialize the editor with an empty paragraph
etrepum marked this conversation as resolved.
Show resolved Hide resolved
*/
start(
rootContainer: Container,
awarenessData?: object,
options: {shouldBootstrapEditor?: boolean} = {},
) {
const container = document.createElement('div');
const reactRoot = createRoot(container);
this._container = container;
Expand All @@ -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>,
);
Expand Down
13 changes: 8 additions & 5 deletions packages/lexical-yjs/src/SyncEditorStates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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',
Expand Down
Loading