From 09e5d9a3bd0b77034f48c3469d8206e878da39e0 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 11:41:29 -0400 Subject: [PATCH] [lexical-yjs] Bug Fix: Keep a v1 editor showing what its document holds when collaborators' text lands where no text node is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Three v1 clients share `before [commented] after`, `commented` in a mark. B wraps the `o` in a mark `c1` and types `x` in the rest (`mmented`); C, having seen only B's first change, wraps the first `m` of `mmented` in `c1`, which writes `mmented` again. Once everything is delivered, no editor matches a fresh read of its own document, and the clients disagree (#9583). B's `x` lands in C's document between C's new mark and the deleted old text, where no text node is. `applyChildrenYjsDelta` removes such text from the shared type ("ignore the text"), but the removal was read back as a remote change and applied to collab children that never held the text, so it deleted the child now at that index: C's new mark, which its document kept. On the clients that had `x` inside their text node, C's change then deleted that node's properties map, and the dangling-text heuristic removed the leftover text at an offset into the text node rather than its index in the element, deleting other text (`c` and the map before it). Both places now mark the element instead, and once its deltas are applied `settleOrphanedText` removes the text no text node holds (text not after a text node's properties map) in one transaction with the binding as its origin, so it is not read back, and builds the element's collab children again from the shared type. The text is still removed, as before: a v1 text node needs its properties map, and every client removes the same items. Closes #9583 ## Test plan `packages/lexical-yjs/src/__tests__/unit/NestedMarksConcurrentTyping.test.ts` (new): the issue's steps with three v1 clients wired like the React collaboration plugin, for a mark at depth 1 and 2, with B or C the client that marks and types, in two client id orders: every editor equals a fresh read of its own document, and the clients agree. ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-yjs/src/__tests__/unit/NestedMarksConcurrentTyping.test.ts Tests 8 failed (8) AssertionError: A equals its own document: expected { root: { …(8) } } to deeply equal { root: { …(8) } } ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-yjs/src/__tests__/unit/NestedMarksConcurrentTyping.test.ts Tests 8 passed (8) $ pnpm exec vitest run --project unit packages/lexical-yjs packages/lexical-react Test Files 54 passed (54) Tests 527 passed | 4 expected fail (531) ``` `pnpm run ci-check` passes. --- .changeset/yjs-v1-orphaned-text.md | 9 + packages/lexical-yjs/src/CollabElementNode.ts | 90 +++++- .../unit/NestedMarksConcurrentTyping.test.ts | 259 ++++++++++++++++++ 3 files changed, 351 insertions(+), 7 deletions(-) create mode 100644 .changeset/yjs-v1-orphaned-text.md create mode 100644 packages/lexical-yjs/src/__tests__/unit/NestedMarksConcurrentTyping.test.ts diff --git a/.changeset/yjs-v1-orphaned-text.md b/.changeset/yjs-v1-orphaned-text.md new file mode 100644 index 00000000000..127f5894f86 --- /dev/null +++ b/.changeset/yjs-v1-orphaned-text.md @@ -0,0 +1,9 @@ +--- +'@lexical/yjs': patch +--- + +[fix] Keep a v1 collaboration editor showing what its document holds when text a collaborator typed lands where no text node is, such as inside nested marks marked concurrently + +The v1 binding removes such text from the document, as before, but that removal was read back as a remote change and deleted the node at its index instead (a mark next to the text vanished from the editor while its document kept it), and a text node whose properties were deleted in a merge had its leftover text removed at an offset into the node rather than its index in the element, deleting other text. The text is now removed with the binding as the origin, at its index, and the element's children are read again from the document. + +Verify: with three v1 clients on a paragraph whose middle word is commented, mark a letter of the word and type after it on one client, mark the next letter on another that saw only the first mark, deliver everything, and check that every editor matches a fresh read of its own document. diff --git a/packages/lexical-yjs/src/CollabElementNode.ts b/packages/lexical-yjs/src/CollabElementNode.ts index d0236fc8da1..c4082e69660 100644 --- a/packages/lexical-yjs/src/CollabElementNode.ts +++ b/packages/lexical-yjs/src/CollabElementNode.ts @@ -7,7 +7,7 @@ */ import type {Binding} from '.'; -import type {AbstractType, Map as YMap, XmlElement, XmlText} from 'yjs'; +import type {AbstractType, XmlElement, XmlText} from 'yjs'; import invariant from '@lexical/internal/invariant'; import { @@ -22,6 +22,7 @@ import { type NodeKey, type NodeMap, } from 'lexical'; +import {Map as YMap} from 'yjs'; import {CollabDecoratorNode} from './CollabDecoratorNode'; import {CollabLineBreakNode} from './CollabLineBreakNode'; @@ -218,6 +219,7 @@ export class CollabElementNode { const children = this._children; let currIndex = 0; let pendingSplitText = null; + let orphaned = false; for (let i = 0; i < deltas.length; i++) { const delta = deltas[i]; @@ -261,8 +263,10 @@ export class CollabElementNode { // Merge the text node with previous. prevCollabNode._text += danglingText; } else { - // No previous text node to merge into, just delete the text. - this._xmlText.delete(offset, danglingText.length); + // No previous text node to merge into: the text is removed + // from the shared type once all the deltas are applied (see + // settleOrphanedText). + orphaned = true; } } } else { @@ -291,10 +295,10 @@ export class CollabElementNode { // never can be dangling text. // We have a conflict where there was likely a CollabTextNode and - // an Lexical TextNode too, but they were removed in a merge. So - // let's just ignore the text and trigger a removal for it from our - // shared type. - this._xmlText.delete(offset, insertDelta.length); + // an Lexical TextNode too, but they were removed in a merge: the + // text has no text node to go in. It is removed from the shared + // type once all the deltas are applied (see settleOrphanedText). + orphaned = true; } currIndex += insertDelta.length; @@ -350,6 +354,78 @@ export class CollabElementNode { throw new Error('Unexpected delta format'); } } + if (orphaned) { + this.settleOrphanedText(binding); + } + } + + /** + * Remove the text of this element's shared type that no text node holds + * (no text node's properties map comes before it: that map was deleted in + * a merge, or the text was inserted where no text node is), and build this + * element's collab children again from the shared type. The removal is + * made with the binding as its origin, so it is not read back as a remote + * change: removing it from the shared type at the index it held, and then + * reading that delete against collab children that never held the text, + * deleted another child (a mark next to the text vanished from the editor + * while its document kept it), and removing it at an offset into a text + * node instead of an index into the element deleted other text. + */ + private settleOrphanedText(binding: Binding): void { + const xmlText = this._xmlText; + const orphans: [index: number, length: number][] = []; + let index = 0; + let inText = false; + for (const op of xmlText.toDelta() as {insert: unknown}[]) { + const insert = op.insert; + if (typeof insert === 'string') { + if (!inText) { + orphans.push([index, insert.length]); + } + index += insert.length; + } else { + inText = + insert instanceof YMap && + getNodeTypeFromSharedType(insert as YMap) !== 'linebreak'; + index += 1; + } + } + if (orphans.length > 0) { + binding.doc.transact(() => { + for (let i = orphans.length - 1; i >= 0; i--) { + xmlText.delete(orphans[i][0], orphans[i][1]); + } + }, binding); + } + const children: CollabElementNode['_children'] = []; + let text: CollabTextNode | null = null; + for (const op of xmlText.toDelta() as {insert: unknown}[]) { + const insert = op.insert; + if (typeof insert === 'string') { + if (text !== null) { + text._text += insert; + } + continue; + } + const sharedType = insert as XmlText | YMap | XmlElement; + if (getNodeTypeFromSharedType(sharedType) === undefined) { + text = null; + continue; + } + const child = $getOrInitCollabNodeFromSharedType( + binding, + sharedType, + this, + ); + if (child instanceof CollabTextNode) { + child._text = ''; + text = child; + } else { + text = null; + } + children.push(child); + } + this._children = children; } syncChildrenFromYjs(binding: Binding): void { diff --git a/packages/lexical-yjs/src/__tests__/unit/NestedMarksConcurrentTyping.test.ts b/packages/lexical-yjs/src/__tests__/unit/NestedMarksConcurrentTyping.test.ts new file mode 100644 index 00000000000..18e74648961 --- /dev/null +++ b/packages/lexical-yjs/src/__tests__/unit/NestedMarksConcurrentTyping.test.ts @@ -0,0 +1,259 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +// Three @lexical/yjs v1 clients (wired like the React collaboration plugin) +// share a paragraph "before [commented] after", where "commented" is in a +// MarkNode with id comment-1 (inside a second mark at depth 2). One client +// wraps the "o" of "commented" in a MarkNode c1 and then types "x" in the +// rest ("mmented"); another receives only the first change, and wraps the +// "m" at the start of "mmented" in c1 too, which writes "mmented" again. The +// "x" then lands where no text node is, and the binding removes it from the +// document: that removal was read back as a remote change and deleted the +// collab child at its index (the new c1 mark), so the editor lost text its +// document held (#9583). Once every message is delivered, every editor must +// show what its own Y.Doc holds, and the clients must agree. + +import {buildEditorFromExtensions} from '@lexical/extension'; +import { + $createMarkNode, + $wrapSelectionInMarkNode, + MarkNode, +} from '@lexical/mark'; +import { + createBinding, + type Provider, + syncLexicalUpdateToYjs, + syncYjsChangesToLexical, +} from '@lexical/yjs'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $getSelection, + $isElementNode, + $isRangeSelection, + $isTextNode, + $setSelection, + type LexicalEditor, + type LexicalNode, + type SerializedEditorState, + SKIP_COLLAB_TAG, + type TextNode, +} from 'lexical'; +import {expect, test} from 'vitest'; +import {applyUpdate, Doc, encodeStateAsUpdate, UndoManager} from 'yjs'; + +const REMOTE = 'remote'; +const provider = { + awareness: { + getLocalState: () => null, + getStates: () => new Map(), + off: () => {}, + on: () => {}, + setLocalState: () => {}, + setLocalStateField: () => {}, + }, + connect: () => {}, + disconnect: () => {}, + off: () => {}, + on: () => {}, +} as unknown as Provider; + +type Message = {from: Client; to: Client; update: Uint8Array}; + +class Client { + readonly doc = new Doc({gc: false}); + readonly editor: LexicalEditor; + + constructor( + readonly name: string, + clientID: number, + queue: Message[] | null, + clients: Client[], + ) { + this.doc.clientID = clientID; + const editor = buildEditorFromExtensions({ + $initialEditorState: null, + name: `[v1-marks-${name}]`, + nodes: [MarkNode], + onError: e => { + throw e; + }, + }); + this.editor = editor; + const binding = createBinding( + editor, + provider, + 'main', + this.doc, + new Map([['main', this.doc]]), + ); + binding.root.getSharedType().observeDeep((( + events: never, + transaction: {origin: unknown}, + ) => { + if (transaction.origin !== binding) { + syncYjsChangesToLexical( + binding, + provider, + events, + transaction.origin instanceof UndoManager, + () => {}, + ); + } + }) as never); + editor.registerUpdateListener(p => { + if (!p.tags.has(SKIP_COLLAB_TAG)) { + syncLexicalUpdateToYjs( + binding, + provider, + p.prevEditorState, + p.editorState, + p.dirtyElements, + p.dirtyLeaves, + p.normalizedNodes, + p.tags, + ); + } + }); + if (queue !== null) { + this.doc.on('update', (update: Uint8Array, origin: unknown) => { + if (origin !== REMOTE) { + for (const to of clients) { + if (to !== this) { + queue.push({from: this, to, update}); + } + } + } + }); + } + } + + flush() { + for (let i = 0; i < 20 && this.editor._pendingEditorState !== null; i++) { + this.editor.read(() => {}); + } + } + + update(fn: () => void) { + this.editor.update(fn, {discrete: true}); + this.flush(); + } + + apply(update: Uint8Array) { + applyUpdate(this.doc, update, REMOTE); + this.flush(); + } + + json(): SerializedEditorState { + return this.editor.getEditorState().toJSON(); + } +} + +function $text(content: string): TextNode { + let found: TextNode | null = null; + const walk = (node: LexicalNode) => { + if ( + found === null && + $isTextNode(node) && + node.getTextContent() === content + ) { + found = node; + } + if ($isElementNode(node)) { + node.getChildren().forEach(walk); + } + }; + walk($getRoot()); + return found!; +} + +function $select(content: string, from: number, to: number) { + const node = $text(content); + const s = $createRangeSelection(); + s.anchor.set(node.getKey(), from, 'text'); + s.focus.set(node.getKey(), to, 'text'); + $setSelection(s); +} + +function $mark(content: string, from: number, to: number, id: string) { + $select(content, from, to); + const selection = $getSelection(); + if ($isRangeSelection(selection)) { + $wrapSelectionInMarkNode(selection, false, id); + } +} + +for (const depth of [1, 2]) { + for (const typer of ['B', 'C'] as const) { + for (const ids of [ + [1000, 1001, 1002], + [1002, 1001, 1000], + ]) { + test(`v1 clients agree with their documents after nested marks and typing (depth ${depth}, ${typer} types, ids ${ids})`, () => { + const queue: Message[] = []; + const clients: Client[] = []; + for (const [i, name] of ['A', 'B', 'C'].entries()) { + clients.push(new Client(name, ids[i], queue, clients)); + } + const [a, b, c] = clients; + // The client that marks and types first, and the one that marks + // concurrently having seen only the first mark. + const [first, second] = typer === 'B' ? [b, c] : [c, b]; + const drain = () => { + while (queue.length > 0) { + const m = queue.shift()!; + m.to.apply(m.update); + } + }; + a.update(() => { + let inner: LexicalNode = $createMarkNode(['comment-1']).append( + $createTextNode('commented'), + ); + if (depth === 2) { + inner = $createMarkNode(['outer']).append(inner); + } + $getRoot() + .clear() + .append( + $createParagraphNode().append( + $createTextNode('before '), + inner, + $createTextNode(' after'), + ), + ); + }); + drain(); + first.update(() => $mark('commented', 1, 2, 'c1')); + first.update(() => { + $select('mmented', 6, 6); + const selection = $getSelection(); + if ($isRangeSelection(selection)) { + selection.insertText('x'); + } + }); + const index = queue.findIndex(m => m.from === first && m.to === second); + const [m] = queue.splice(index, 1); + second.apply(m.update); + second.update(() => $mark('mmented', 0, 1, 'c1')); + drain(); + for (const client of clients) { + const fresh = new Client('F', 2000, null, []); + fresh.apply(encodeStateAsUpdate(client.doc)); + expect( + client.json(), + `${client.name} equals its own document`, + ).toEqual(fresh.json()); + } + expect(b.json()).toEqual(a.json()); + expect(c.json()).toEqual(a.json()); + }); + } + } +}