[lexical-playground][lexical-website][lexical-y] Bug Fix: Hold back remote changes to the line being composed until the composition ends

.changeset/lexical-y-composition-holdback.md

@@ -0,0 +1,7 @@
+---
+'@lexical/y': patch
+---
+
+[fix] Keep the line being composed with an IME unchanged while collaborators add to it, and show every held change exactly once after composition ends, also after Enter or a paste made while composing
+
+Verify: IME composition (Chinese, Japanese or Korean input) in a paragraph a collaborator edits at the same time: the composition is not interrupted, and what they typed, formatted, split or pasted there shows when it ends. Check that a held insert followed by a multi-paragraph paste appears exactly once after composition ends, and that held collaborator inline formatting is visible when composition ends and Enter is pressed in the same update. With BlockIdExtension enabled, pasted lists and their items must agree with the block IDs stored in the document.

packages/lexical-playground/__tests__/e2e/CollabYNodes.spec.mjs

@@ -218,6 +218,30 @@ test.describe('Collab Y: IME', () => {
     expect(blocks).toEqual(['paragraph:s1z', 'paragraph:kb1z  s2z s3z 日本']);
   });
 
+  test('compose while a peer splits the line after the composition', async ({
+    browser,
+  }) => {
+    const {clients, session} = await openClients(browser);
+    const [a, b] = clients;
+    await seed(clients, ['s1z s2z s3z s4z']);
+    await a.caret('s2z');
+    await b.caret('s3z');
+    await session.delay();
+    await Promise.all([
+      a.ime(NIHON, '日本', 80),
+      (async () => {
+        await b.page.waitForTimeout(120);
+        await b.press('Enter');
+      })(),
+    ]);
+    const {blocks} = await finish(clients, {
+      present: ['s1z', 's2z', 's3z', 's4z', '日本'],
+    });
+    // The node being composed keeps its DOM text node: the browser goes on
+    // composing in it, and commits only what it composed.
+    expect(blocks).toEqual(['paragraph:s1z s2z日本 s3z', 'paragraph: s4z']);
+  });
+
   test('compose in a line while a peer joins it into the previous line', async ({
     browser,
   }) => {

packages/lexical-website/docs/collaboration/y.md

@@ -744,6 +744,15 @@ not merge as a user might expect:
   stays outside, and a collaborator's concurrent delete of a character there
   does not apply to the moved text. Store comments as NodeState instead,
   with [`MarkStateExtension`](./comments.md).
+- **Input method composition.** While the editor composes with an input
+  method (IME) in a line, what collaborators add to that line (text,
+  formats and comments, a split, pasted lines, a change of its type) waits
+  until the composition ends, and the line keeps its nodes, so the browser's
+  composition is not disturbed. Then all of it shows where it is, also after
+  Enter or a paste made while composing. What removes something from the
+  line (a deletion, a join, a `MarkNode` comment, which moves the text into
+  it) shows at once: the text node being composed keeps its key, unless its
+  class changes (a code block made a heading).
 - **Set-valued NodeState** (`arraySetValue`, which `MarkStateExtension` uses)
   merges per member on text, line breaks and nodes stored as their own Yjs
   node (such as decorators), so two collaborators commenting on the same

packages/lexical-y/src/__tests__/unit/Composition.test.ts

@@ -195,18 +195,22 @@ describe('@lexical/y IME composition with concurrent remote edits', () => {
       );
     });
 
-    test(`a structural edit of the composing line is read at once (ids ${ids})`, async () => {
+    test(`a split of the composing line waits for the composition to end (ids ${ids})`, async () => {
       const t = await setup(() => $getRoot().append($p('one two')), {ids});
       compositionStart(t.a, [0], 7);
       compositionStep(t.a, [0], 7, '', 'n');
       await t.sync();
+      const key = t.a.read(() => $text([0]).getKey());
       t.b.update(() => $selectIn([0], 3).insertParagraph());
       await t.sync();
-      expect(t.a.read(() => $getRoot().getChildrenSize())).toBe(2);
+      // The composing line keeps its nodes while composing.
+      expect(t.a.read(() => $getRoot().getChildrenSize())).toBe(1);
+      expect(t.a.read(() => $text([0]).getKey())).toBe(key);
+      expect(lineText(t.a, [0])).toBe('one twon');
       t.a.update(() => {
-        const node = $text([1]);
-        node.setTextContent(' twoに');
-        node.select(6, 6);
+        const node = $text([0]);
+        node.setTextContent('one twoに');
+        node.select(8, 8);
         $setCompositionKey(null);
       });
       await t.sync();
@@ -304,10 +308,10 @@ describe('@lexical/y IME composition with concurrent remote edits', () => {
   ]) {
     test(`a peer joins the lines after typing into the composing line (ids ${ids})`, async () => {
       // The peer's insert into the composing line is held back (not shown
-      // until the composition ends); the join then reads the line again, so
-      // it shows everything: what the editor shows and what it holds back
-      // must stay complementary (the self-check compares them on the next
-      // local write).
+      // until the composition ends); the join, which cannot be held (Yjs
+      // does not keep the deleted line marker), is read at once without it:
+      // what the editor shows and what it holds back stay complementary (the
+      // self-check compares them on the next local write).
       const t = await setup(() => $getRoot().append($p('j1z'), $p('j2z')), {
         ids,
       });
@@ -325,7 +329,7 @@ describe('@lexical/y IME composition with concurrent remote edits', () => {
         second.remove();
       });
       await t.sync();
-      expect(lineText(t.a, [0])).toBe('j1zにj2zZ');
+      expect(lineText(t.a, [0])).toBe('j1zにj2z');
       t.a.update(() => {
         const node = $getNodeByKey(key) as TextNode;
         node.setTextContent(node.getTextContent().replace('に', '日本'));

packages/lexical-y/src/__tests__/unit/CompositionHoldBack.test.ts

@@ -0,0 +1,55 @@
+/**
+ * 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.
+ *
+ */
+
+// @vitest-environment node
+
+// The promise of the composition hold-back (see Hidden), checked over line
+// shapes, collaborators' changes, and the ways a composition ends:
+//
+// - While the local editor composes (IME) in a line, a collaborator's change
+//   to that line that Yjs keeps whole (an insert, a format, a comment, a
+//   split, a change of the line's type) does not change the line's nodes:
+//   the browser composes into their DOM. A change that removes something
+//   (a deletion, a join, a comment wrapped around text, which moves it) is
+//   read at once, and the node being composed keeps its key and its line,
+//   unless its class changes (a code line made a heading).
+// - When the composition ends (completed, cancelled, cut short by a blur),
+//   or the local editor changes the line's structure while composing (Enter,
+//   Shift+Enter, Backspace or Delete across lines, a paste, a change of the
+//   line's type, after which the composition goes on), every held change
+//   shows where it is: the editor equals a fresh read of its document, as
+//   every peer's does, and nothing a collaborator did is lost.
+//
+// The matrix is in ../utils/compositionHoldBack.ts; part of it runs with
+// BlockIdExtension in CompositionHoldBackBlockIds.test.ts.
+
+import {describe, test} from 'vitest';
+
+import {ENDS, REMOTE, runCase, SHAPES} from '../utils/compositionHoldBack';
+
+describe('@lexical/y composition hold-back', () => {
+  for (const [shapeName, shape] of Object.entries(SHAPES)) {
+    for (const [remoteName, remote] of Object.entries(REMOTE)) {
+      for (const end of ENDS) {
+        for (const ids of [
+          [1, 2],
+          [2, 1],
+        ]) {
+          const limited =
+            remote.limit !== undefined && remote.limit(shape, end);
+          (limited ? test.skip : test)(
+            `${shapeName} line, remote ${remoteName}, ${end} (ids ${ids})`,
+            async () => {
+              await runCase(shape, remote, end, ids);
+            },
+          );
+        }
+      }
+    }
+  }
+});

packages/lexical-y/src/__tests__/unit/CompositionHoldBackBlockIds.test.ts

@@ -0,0 +1,93 @@
+/**
+ * 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.
+ *
+ */
+
+// @vitest-environment node
+
+// Part of the composition hold-back matrix (see CompositionHoldBack.test.ts)
+// with BlockIdExtension: the reads a held change makes keep the line's id
+// and node too; and finding R-5 of the review of the hold-back.
+import '../utils/blockIdsEnv';
+
+import {blockIdState} from '@lexical/extension';
+import {
+  $getNodeByKey,
+  $getRoot,
+  $getState,
+  $setCompositionKey,
+  $setState,
+  type TextNode,
+} from 'lexical';
+import {describe, expect, test} from 'vitest';
+
+import {REMOTE, runCase, SHAPES} from '../utils/compositionHoldBack';
+import {
+  $block,
+  $p,
+  expectConverged,
+  setup,
+  WITH_BLOCK_IDS,
+} from '../utils/harness';
+
+delete process.env.LEXICAL_Y_BLOCK_ID;
+
+const ENDINGS = ['completed', 'Enter', 'a change of the line type'] as const;
+
+describe('@lexical/y composition hold-back with block ids', () => {
+  for (const shapeName of ['list item', 'plain']) {
+    for (const [remoteName, remote] of Object.entries(REMOTE)) {
+      for (const end of ENDINGS) {
+        for (const ids of [
+          [1, 2],
+          [2, 1],
+        ]) {
+          const shape = SHAPES[shapeName];
+          const limited =
+            remote.limit !== undefined && remote.limit(shape, end);
+          (limited ? test.skip : test)(
+            `${shapeName} line, remote ${remoteName}, ${end} (ids ${ids})`,
+            async () => {
+              expect(WITH_BLOCK_IDS).toBe(true);
+              await runCase(shape, remote, end, ids);
+            },
+          );
+        }
+      }
+    }
+  }
+});
+
+describe('R-5: a peer changes the block id of the line A composes in', () => {
+  test('the line shows its old id until the composition ends', async () => {
+    expect(WITH_BLOCK_IDS).toBe(true);
+    const t = await setup(() => $getRoot().append($p('composed'), $p('next')));
+    const id = () => t.a.read(() => $getState($block([0]), blockIdState));
+    const was = id();
+    let key = '';
+    t.a.update(() => {
+      const node = $block([0]).getFirstChildOrThrow() as TextNode;
+      node.select(8, 8);
+      $setCompositionKey(node.getKey());
+      key = node.getKey();
+    });
+    t.b.update(() => $setState($block([0]), blockIdState, 'remote-id'));
+    await t.sync();
+    expect(id()).toBe(was);
+    // A composition step: an update that settles the ids it touched.
+    t.a.update(() => {
+      const node = $getNodeByKey(key) as TextNode;
+      node.setTextContent('composedに');
+      node.select(9, 9);
+    });
+    await t.sync();
+    expect(id()).toBe(was);
+    t.a.update(() => $setCompositionKey(null));
+    await t.sync();
+    await expectConverged(t.peers);
+    expect(id()).toBe('remote-id');
+  });
+});