From 1b445eb70252bd6febadb83f84e063e6fddf7eb9 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Wed, 28 Oct 2026 19:29:34 -0400 Subject: [PATCH 1/3] [lexical-website][lexical-y] Bug Fix: Write a block reorder as a move of the block Lexical moved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Yjs has no moves, so when a commit reorders blocks, `@lexical/y` keeps some blocks in place and deletes and re-inserts the others. Whatever a collaborator types in a re-inserted block at the same time is lost or stays at its old position. The writer chose the blocks to keep by block count and then by content size, and used Lexical's record of explicit moves (`EditorState#_movedNodes`) only to break exact ties. So a reorder was written as a move of whichever side held less content, not of the block the user moved: - Peer A moves a two-item list above the paragraph `Another line` (`paragraph.insertBefore(list)`, as a block drag does) while peer B types `X` after `Another`. Every client and a fresh reader end with `list("Xitem one", "item two") / paragraph("Another line")`: B's text lands in a block nobody moved. - Without concurrency, B's caret after `Another` jumps to the start of `item one` when A's move arrives. - A table (1x2 or 2x2) or a list with more text than the paragraph, moved up or down past it, leaves B's `X` in a paragraph of its own at the paragraph's old place. A short list, a quote or a paragraph moved the same way only passed because each held less content than the paragraph. With this change, the block Lexical recorded as moved is the one rewritten, which is what `@lexical/yjs` already does (`getStableChildKeys`). `Writer#$writeStructure` splits the matched units into those the commit moved and the rest. A unit counts as moved when its node, or an element between it and the container being written (the list of a list item, the item that holds a nested list), is in the record. The rest keep their relative order, since only a move changes it, so all of them stay in place. A moved unit stays too where it is still in order between them (put back where it was, or indented). Block count and size now only decide where Lexical recorded nothing (an editor state set whole) and among moved units. Core already records every move made with `insertBefore`, `insertAfter`, `append` or `splice`, so it is unchanged. The stored layout does not change. | With a concurrent move of a two-item list | Before | After | | --- | --- | --- | | B types in the paragraph the list passes | lands in `item one` | stays in the paragraph | | B's caret in that paragraph | jumps into `item one` | stays | | B types in a paragraph a moved table passes | lands in a paragraph of its own | stays | | B types in the moved list itself | stays in the list | stays at the list's old place | The last row is the documented Moves limit, which before applied only when the moved block was the smaller one. It covers text typed in the moved block at the same time, a collaborator's caret in it, and what a collaborator typed in it before the mover undoes the move. Two `BlocksAndLines` tests asserted the old trade-off (typing inside a dragged collapsible, and inside a paragraph moved down one slot, survived because the passed block was rewritten). They are now `test.fails`, next to new tests for typing in the block passed. The y.md Moves limit now says which block counts as moved, the fallback, and the undo case. These are left as expected failures in `regressions/Moves.test.ts`: - the moved block itself (above); - a node put back where it was in the same update as a move (an insert next to the sibling it already has, a remove and a re-insert, a move away and back). Lexical's record counts such a node as moved too, so when a real move passes it, the smaller of the two is rewritten. Refining that record would be a core change shared with `@lexical/yjs`. ## Changeset `.changeset/lexical-y-reorder-intent.md`: `[fix]` for `@lexical/y`, with a `Verify:` paragraph for the behavior change. ## Test plan New: `packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts` (132 tests). It covers the S7-2 matrix with its controls, blocks of every shape moved up and down, the result of a recorded move (what the writer deletes), each move API, nested lists, tables and table rows, carets, the fallback with no record, and the remaining limits as `test.fails`, each with a convergence test. Two tests in `BlocksAndLines.test.ts` are updated as described above. `./node_modules/.bin/vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts packages/lexical-y/src/__tests__/unit/BlocksAndLines.test.ts` ### Before On `main` (0cd7448e7): ``` FAIL |unit| packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts > S7-2: a block moved up past the paragraph before it > S7-2 a two-item list moved up: a caret in the paragraph it passed stays there (ids 2,1) AssertionError: expected 'list[bullet](listitem("Xitem one") li…' to be 'list[bullet](listitem("item one") lis…' // Object.is equality Expected: "list[bullet](listitem("item one") listitem("item two")) / paragraph("AnotherX line")" Received: "list[bullet](listitem("Xitem one") listitem("item two")) / paragraph("Another line")" FAIL |unit| packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts > recorded moves of tables and table rows > a table moved up past a paragraph ‖ typing in the paragraph stays there (ids 1,2) AssertionError: expected 'Another line' not to contain 'Another' Test Files 2 failed (2) Tests 72 failed | 108 passed | 8 expected fail (188) ``` Failures by group: S7-2 matrix 8, every shape 12 (long list, 1x2 and 2x2 tables, up and down), what a recorded move rewrites 10, each move API 8, nested lists 6, tables and rows 10, `BlocksAndLines` moves 8. The Moves limit group also counts 10, because those `test.fails` tests pass on `main`, where the passed block was rewritten instead. ### After ``` ✓ |unit| packages/lexical-y/src/__tests__/unit/BlocksAndLines.test.ts (56 tests) 2085ms ✓ |unit| packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts (132 tests) 4828ms Test Files 2 passed (2) Tests 166 passed | 22 expected fail (188) ``` Also run with the change: - `@lexical/y` unit tests: 149 files passed and 2 skipped; 5,027 tests passed, 38 expected fail, 2 skipped (on `main`: 148 files; 4,913 passed, 16 expected fail). - Golden corpus: the fuzz's five lanes (no moves, block ids, gc, moves, block ids with moves), 250 seeds each, compared with `main` for every peer's Yjs state bytes, editor JSON, warnings, fuzz findings, node keys and selection. The three lanes without moves are identical on all 750 seeds. In each lane with moves, 14 of 250 seeds write different bytes (the moved block is rewritten instead of the block it passed), with no other difference. Two of them add a `loss(container)` finding: text typed inside a table while that table moved, which is the Moves limit. - The soak repros: the original S7-2 matrix fails 8 of 36 on `main` and passes 36 of 36; the shapes matrix fails 8 of 20 on `main` and passes 20 of 20. - `pnpm run ci-check` passes. - Not run: browser-mode tests and the playground e2e specs. No collab Y e2e spec moves a block. --- .changeset/lexical-y-reorder-intent.md | 7 + .../lexical-website/docs/collaboration/y.md | 14 +- .../src/__tests__/unit/BlocksAndLines.test.ts | 55 +- .../__tests__/unit/regressions/Moves.test.ts | 766 ++++++++++++++++++ packages/lexical-y/src/internal/Write.ts | 118 ++- 5 files changed, 931 insertions(+), 29 deletions(-) create mode 100644 .changeset/lexical-y-reorder-intent.md create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts diff --git a/.changeset/lexical-y-reorder-intent.md b/.changeset/lexical-y-reorder-intent.md new file mode 100644 index 00000000000..96ab1f9784b --- /dev/null +++ b/.changeset/lexical-y-reorder-intent.md @@ -0,0 +1,7 @@ +--- +'@lexical/y': patch +--- + +[fix] Write a block reorder as a move of the block Lexical moved, so that a collaborator's concurrent typing and caret in a block it passes stay in that block + +Verify: concurrent edits around a dragged block. The blocks it passes keep a collaborator's typing and caret. Text typed in the moved block itself at the same time is lost or lands at its old position, and undoing the move takes back what collaborators typed in the moved block since (the Moves limit in the collaboration docs); this now holds also when the moved block is larger than the blocks it passes. diff --git a/packages/lexical-website/docs/collaboration/y.md b/packages/lexical-website/docs/collaboration/y.md index c79037b55ad..a4ae4dab8ca 100644 --- a/packages/lexical-website/docs/collaboration/y.md +++ b/packages/lexical-website/docs/collaboration/y.md @@ -713,8 +713,18 @@ not merge as a user might expect: - **Moves.** Moving a block is a delete and a re-insert, so text a collaborator types into the moved block at the same time is lost or lands - at its old position, and two collaborators moving the same block at once - can duplicate it. Lexical operations that move content (turning a + at its old position, a collaborator's caret in it stays at that position, + and two collaborators moving the same block at once can duplicate it. + Undoing the move restores the block as it was before the move: what + collaborators typed in it since is lost or, in a moved line, stays where + the moved copy was. The moved block is the node Lexical recorded as moved: + an existing node passed to `insertBefore`, `insertAfter`, `append` or + `splice` in the update (as a block drag does), even one put back where it + was. The blocks it passes that Lexical did not record keep what + collaborators type in them. Between recorded blocks, and in a reorder that + Lexical did not record (an editor state set whole), the binding rewrites + as few blocks as it can, the smaller ones on a tie. + Lexical operations that move content (turning a paragraph into a list inside a container, indenting list items that hold blocks) behave the same way, and so does the first edit that restructures a list stored in an older shape (a nested list kept as a node inside an diff --git a/packages/lexical-y/src/__tests__/unit/BlocksAndLines.test.ts b/packages/lexical-y/src/__tests__/unit/BlocksAndLines.test.ts index 3a49bf48611..49dfdc3b146 100644 --- a/packages/lexical-y/src/__tests__/unit/BlocksAndLines.test.ts +++ b/packages/lexical-y/src/__tests__/unit/BlocksAndLines.test.ts @@ -349,7 +349,7 @@ describe('@lexical/y moves', () => { $create(ColTitleNode).append($createTextNode('t')), $create(ColContentNode).append($p('c1'), $p('c2')), ); - test(`a collapsible dragged past a box ‖ typing inside it keeps the typing (ids ${ids})`, async () => { + test(`a collapsible dragged past a box ‖ typing in the box keeps the typing (ids ${ids})`, async () => { const t = await setup( () => $getRoot().append( @@ -361,27 +361,70 @@ describe('@lexical/y moves', () => { ); t.offline(); t.a.update(() => $block([1]).insertAfter($block([0]))); - t.b.update(() => $selectIn([0, 1, 0], 2).insertText('TYPED')); + t.b.update(() => $selectIn([1, 0], 3).insertText('TYPED')); await t.sync(); await expectConverged(t.peers); - expect(t.a.describe()).toContain('c1TYPED'); + expect(t.a.describe()).toContain('boxTYPED'); }); - test(`moving a paragraph down one slot ‖ typing in it keeps the typing in it (ids ${ids})`, async () => { + // Moves limit (y.md): the dragged block is the one deleted and written + // again, so typing inside it at the same time is lost. + test.fails( + `a collapsible dragged past a box ‖ typing inside it keeps the typing (ids ${ids})`, + async () => { + const t = await setup( + () => + $getRoot().append( + $col(), + $createBoxNode('o').append($p('box')), + $p('end'), + ), + {ids}, + ); + t.offline(); + t.a.update(() => $block([1]).insertAfter($block([0]))); + t.b.update(() => $selectIn([0, 1, 0], 2).insertText('TYPED')); + await t.sync(); + await expectConverged(t.peers); + expect(t.a.describe()).toContain('c1TYPED'); + }, + ); + + test(`moving a paragraph down one slot ‖ typing in the paragraph it passed stays there (ids ${ids})`, async () => { const t = await setup( () => $getRoot().append($p('Revenue grew 4% in Q3.'), $p('Second.')), {ids}, ); t.offline(); - t.a.update(() => $selectIn([0], 13).insertText('by ')); + t.a.update(() => $selectIn([1], 6).insertText(' line')); t.b.update(() => $block([1]).insertAfter($block([0]))); await t.sync(); await expectConverged( t.peers, - 'paragraph("Second.") / paragraph("Revenue grew by 4% in Q3.")', + 'paragraph("Second line.") / paragraph("Revenue grew 4% in Q3.")', ); }); + // Moves limit (y.md): the moved paragraph is the one deleted and written + // again, so typing in it at the same time lands at its old place. + test.fails( + `moving a paragraph down one slot ‖ typing in it keeps the typing in it (ids ${ids})`, + async () => { + const t = await setup( + () => $getRoot().append($p('Revenue grew 4% in Q3.'), $p('Second.')), + {ids}, + ); + t.offline(); + t.a.update(() => $selectIn([0], 13).insertText('by ')); + t.b.update(() => $block([1]).insertAfter($block([0]))); + await t.sync(); + await expectConverged( + t.peers, + 'paragraph("Second.") / paragraph("Revenue grew by 4% in Q3.")', + ); + }, + ); + test(`between equal blocks, the moved one is rewritten: typing in the other is kept (ids ${ids})`, async () => { const t = await setup( () => $getRoot().append($p('one'), $p('two'), $p('three')), diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts new file mode 100644 index 00000000000..06e6847f115 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts @@ -0,0 +1,766 @@ +/** + * 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 + +// How a reorder of blocks is written. Yjs has no moves, so a block that changes +// place is deleted and inserted again, and what a peer types in it meanwhile is +// lost or stays at its old place. The binding rewrites the block that Lexical +// recorded as moved (an existing node passed to insertBefore, insertAfter, +// append or splice, as a block drag does), and every block it passed that +// Lexical did not record keeps its place, with what peers type there. Between +// recorded blocks, and in a reorder with no record (an editor state set whole), +// it rewrites the smaller side. Each case is named after its finding or after +// what it covers. + +import {$createListItemNode, $createListNode} from '@lexical/list'; +import {$createQuoteNode} from '@lexical/rich-text'; +import { + $createTableCellNode, + $createTableNode, + $createTableRowNode, +} from '@lexical/table'; +import * as Y from '@y/y'; +import { + $createTextNode, + $getRoot, + $getSelection, + $isRangeSelection, + type ElementNode, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; + +import { + $block, + $p, + $selectIn, + expectConverged, + type Peer, + type Setup, + setup, +} from '../../utils/harness'; + +const IDS = [ + [1, 2], + [2, 1], +]; + +const $list = (...items: string[]) => + $createListNode('bullet').append( + ...items.map(text => $createListItemNode().append($createTextNode(text))), + ); + +const $table = (...rows: string[]) => + $createTableNode().append( + ...rows.map(text => + $createTableRowNode().append($createTableCellNode().append($p(text))), + ), + ); + +/** The text that `peer`'s own transactions delete while `fn` runs. */ +function deletedBy(peer: Peer, fn: () => void): string { + let deleted = ''; + const onTransaction = (tr: Y.Transaction) => { + if (!tr.local) { + return; + } + Y.iterateStructsByIdSet(tr, tr.deleteSet, s => { + if (s instanceof Y.Item && s.content instanceof Y.ContentString) { + deleted += s.content.str; + } + }); + }; + peer.doc.on('afterTransaction', onTransaction); + try { + fn(); + } finally { + peer.doc.off('afterTransaction', onTransaction); + } + return deleted; +} + +function type(peer: Peer, text: string) { + peer.update(() => { + const selection = $getSelection(); + if ($isRangeSelection(selection)) { + selection.insertText(text); + } + }); +} + +// S7-2: moving a two-item list up past the paragraph before it (as a block +// drag does: `paragraph.insertBefore(list)`) was written as a move of the +// paragraph, the smaller block. A peer's concurrent typing in the paragraph +// landed in the list's first item ("Xitem one") on every client, and a +// peer's caret after "Another" went to the start of "item one". A one-item +// list, a quote and a paragraph (each smaller than the paragraph it passes) +// were controls. +describe('S7-2: a block moved up past the paragraph before it', () => { + const MOVED = { + 'one-item list': () => $list('item one'), + paragraph: () => $p('item one'), + quote: () => $createQuoteNode().append($createTextNode('item one')), + 'two-item list': () => $list('item one', 'item two'), + }; + const SHOWN = { + 'one-item list': 'list[bullet](listitem("item one"))', + paragraph: 'paragraph("item one")', + quote: 'quote("item one")', + 'two-item list': 'list[bullet](listitem("item one") listitem("item two"))', + }; + for (const ids of IDS) { + for (const surrounded of [false, true]) { + for (const kind of Object.keys(MOVED) as (keyof typeof MOVED)[]) { + const at = surrounded ? 1 : 0; + const $init = () => { + if (surrounded) { + $getRoot().append($p('Hello world')); + } + $getRoot().append($p('Another line'), MOVED[kind]()); + if (surrounded) { + $getRoot().append( + $createQuoteNode().append($createTextNode('quote')), + ); + } + }; + const expected = [ + ...(surrounded ? ['paragraph("Hello world")'] : []), + SHOWN[kind], + 'paragraph("AnotherX line")', + ...(surrounded ? ['quote("quote")'] : []), + ].join(' / '); + const where = `${surrounded ? ', between other blocks' : ''} (ids ${ids})`; + + test(`S7-2 a ${kind} moved up ‖ typing in the paragraph it passed stays there${where}`, async () => { + const t = await setup($init, {ids}); + t.offline(); + t.a.update(() => $block([at]).insertBefore($block([at + 1]))); + t.b.update(() => $selectIn([at], 7).insertText('X')); + await t.sync(); + await expectConverged(t.peers, expected); + }); + + test(`S7-2 a ${kind} moved up: a caret in the paragraph it passed stays there${where}`, async () => { + const t = await setup($init, {ids}); + t.b.update(() => $selectIn([at], 7)); + t.a.update(() => $block([at]).insertBefore($block([at + 1]))); + await t.sync(); + type(t.b, 'X'); + await t.sync(); + await expectConverged(t.peers, expected); + }); + } + } + } +}); + +// S7-2, other shapes: which block was rewritten followed content size, so a +// table (1x2 or 2x2) or a list with more text than the paragraph, moved up +// or down past it, put the peer's typing in a paragraph of its own at the +// paragraph's old place; a short list and a quote passed. +describe('S7-2: blocks of every shape moved up or down past a paragraph', () => { + const $cells = (rows: string[][]) => + $createTableNode().append( + ...rows.map(cells => + $createTableRowNode().append( + ...cells.map(text => $createTableCellNode().append($p(text))), + ), + ), + ); + const SHAPES: [string, () => ElementNode][] = [ + ['short list', () => $list('i1', 'i2')], + ['list of 3', () => $list('i1', 'i2', 'i3')], + ['long list', () => $list('the first item', 'the second item')], + ['1x2 table', () => $cells([['c1', 'c2']])], + [ + '2x2 table', + () => + $cells([ + ['r0c0', 'r0c1'], + ['r1c0', 'r1c1'], + ]), + ], + ['quote', () => $createQuoteNode().append($createTextNode('q1'))], + ]; + for (const ids of IDS) { + for (const [shape, $make] of SHAPES) { + for (const direction of ['up', 'down'] as const) { + test(`S7-2 a ${shape} moved ${direction} past a paragraph ‖ typing in the paragraph stays there (ids ${ids})`, async () => { + const t = await setup( + () => + direction === 'up' + ? $getRoot().append($p('Another line'), $make()) + : $getRoot().append($make(), $p('Another line')), + {ids}, + ); + t.offline(); + t.a.update(() => + direction === 'up' + ? $block([0]).insertBefore($block([1])) + : $block([1]).insertAfter($block([0])), + ); + t.b.update(() => + $selectIn([direction === 'up' ? 0 : 1], 7).insertText('X'), + ); + await t.sync(); + await expectConverged(t.peers); + const blocks = t.a.read(() => + $getRoot() + .getChildren() + .map(block => block.getTextContent()), + ); + expect(blocks).toHaveLength(2); + expect(blocks[direction === 'up' ? 1 : 0]).toBe('AnotherX line'); + }); + } + } + } +}); + +describe('a recorded move rewrites the block that moved, not the blocks it passed', () => { + for (const ids of IDS) { + test(`a list moved up past a paragraph: the list is rewritten (ids ${ids})`, async () => { + const t = await setup( + () => + $getRoot().append($p('Another line'), $list('item one', 'item two')), + {ids}, + ); + const deleted = deletedBy(t.a, () => + t.a.update(() => $block([0]).insertBefore($block([1]))), + ); + expect(deleted).toContain('item one'); + expect(deleted).toContain('item two'); + expect(deleted).not.toContain('Another'); + await t.sync(); + await expectConverged( + t.peers, + 'list[bullet](listitem("item one") listitem("item two")) / paragraph("Another line")', + ); + }); + + test(`a list moved down past a paragraph ‖ typing in the paragraph stays there (ids ${ids})`, async () => { + const t = await setup( + () => + $getRoot().append($list('item one', 'item two'), $p('Another line')), + {ids}, + ); + t.offline(); + const deleted = deletedBy(t.a, () => + t.a.update(() => $block([1]).insertAfter($block([0]))), + ); + expect(deleted).not.toContain('Another'); + t.b.update(() => $selectIn([1], 7).insertText('X')); + await t.sync(); + await expectConverged( + t.peers, + 'paragraph("AnotherX line") / list[bullet](listitem("item one") listitem("item two"))', + ); + }); + + test(`a paragraph moved down past a list ‖ typing in the list stays there (ids ${ids})`, async () => { + const t = await setup( + () => + $getRoot().append($p('Another line'), $list('item one', 'item two')), + {ids}, + ); + t.offline(); + t.a.update(() => $block([1]).insertAfter($block([0]))); + t.b.update(() => $selectIn([1, 0], 4).insertText('X')); + await t.sync(); + await expectConverged( + t.peers, + 'list[bullet](listitem("itemX one") listitem("item two")) / paragraph("Another line")', + ); + }); + + for (const direction of ['up', 'down'] as const) { + test(`two long paragraphs moved ${direction} past a short one in one update ‖ typing in the short one stays there (ids ${ids})`, async () => { + const long = ['the first long paragraph', 'the second long paragraph']; + const t = await setup( + () => + direction === 'up' + ? $getRoot().append($p('short'), $p(long[0]), $p(long[1])) + : $getRoot().append($p(long[0]), $p(long[1]), $p('short')), + {ids}, + ); + t.offline(); + const deleted = deletedBy(t.a, () => + t.a.update(() => { + const children = $getRoot().getChildren(); + if (direction === 'up') { + const [short, first, second] = children; + short.insertBefore(first); + short.insertBefore(second); + } else { + const [first, second, short] = children; + short.insertAfter(second); + short.insertAfter(first); + } + }), + ); + expect(deleted).not.toContain('short'); + const shortAt = direction === 'up' ? 0 : 2; + t.b.update(() => $selectIn([shortAt], 5).insertText('X')); + await t.sync(); + const moved = `paragraph("${long[0]}") / paragraph("${long[1]}")`; + await expectConverged( + t.peers, + direction === 'up' + ? `${moved} / paragraph("shortX")` + : `paragraph("shortX") / ${moved}`, + ); + }); + } + + test(`a list and a table moved up past a paragraph in one update ‖ typing in the paragraph stays there (ids ${ids})`, async () => { + const t = await setup( + () => + $getRoot().append( + $p('Another line'), + $list('item one', 'item two'), + $table('cell one', 'cell two'), + ), + {ids}, + ); + t.offline(); + t.a.update(() => { + const [paragraph, list, table] = $getRoot().getChildren(); + paragraph.insertBefore(list); + paragraph.insertBefore(table); + }); + t.b.update(() => $selectIn([0], 7).insertText('X')); + await t.sync(); + await expectConverged(t.peers); + expect(t.a.describe()).toMatch( + /^list.*table.*paragraph\("AnotherX line"\)$/, + ); + }); + } +}); + +// The APIs y.md (Limits, Moves) names as recorded moves: each moves the +// list here, and the paragraph it passes keeps a peer's typing. +describe('every Lexical API that moves a node records the move', () => { + const MOVES: [string, 'up' | 'down', () => void][] = [ + ['insertBefore', 'up', () => $block([0]).insertBefore($block([1]))], + ['insertAfter', 'down', () => $block([1]).insertAfter($block([0]))], + ['append', 'down', () => $getRoot().append($block([0]))], + ['splice', 'up', () => $getRoot().splice(0, 0, [$block([1])])], + ]; + for (const ids of IDS) { + for (const [api, direction, $move] of MOVES) { + test(`${api} moves a list ${direction} past a paragraph ‖ typing in the paragraph stays there (ids ${ids})`, async () => { + const $twoItems = () => $list('item one', 'item two'); + const t = await setup( + () => + direction === 'up' + ? $getRoot().append($p('Another line'), $twoItems()) + : $getRoot().append($twoItems(), $p('Another line')), + {ids}, + ); + t.offline(); + const deleted = deletedBy(t.a, () => t.a.update($move)); + expect(deleted).not.toContain('Another'); + const at = direction === 'up' ? 0 : 1; + t.b.update(() => $selectIn([at], 7).insertText('X')); + await t.sync(); + const shownList = + 'list[bullet](listitem("item one") listitem("item two"))'; + await expectConverged( + t.peers, + direction === 'up' + ? `${shownList} / paragraph("AnotherX line")` + : `paragraph("AnotherX line") / ${shownList}`, + ); + }); + } + + test(`a block detached and inserted back where it was is not rewritten (ids ${ids})`, async () => { + const t = await setup( + () => + $getRoot().append( + $p('Another line'), + $list('item one', 'item two'), + $p('end'), + ), + {ids}, + ); + const deleted = deletedBy(t.a, () => + t.a.update(() => $block([0]).insertAfter($block([1]))), + ); + expect(deleted).toBe(''); + await t.sync(); + await expectConverged( + t.peers, + 'paragraph("Another line") / list[bullet](listitem("item one") listitem("item two")) / paragraph("end")', + ); + }); + } +}); + +describe('recorded moves in nested lists', () => { + // one / two / [n1 / the nested item that moves] + const $nested = () => + $createListNode('bullet').append( + $createListItemNode().append($createTextNode('one')), + $createListItemNode().append($createTextNode('two')), + $createListItemNode().append($list('n1', 'the nested item that moves')), + ); + const $nestedList = () => + ($block([0]).getLastChild() as ElementNode).getFirstChild() as ElementNode; + for (const ids of IDS) { + test(`a nested item moved up past its sibling ‖ typing in the sibling stays there (ids ${ids})`, async () => { + const t = await setup(() => $getRoot().append($nested()), {ids}); + t.offline(); + t.a.update(() => { + const [n1, mover] = $nestedList().getChildren(); + n1.insertBefore(mover); + }); + t.b.update(() => $selectIn([0, 2, 0, 0], 2).insertText('X')); + await t.sync(); + await expectConverged(t.peers); + expect(t.a.describe()).toContain( + 'list[bullet](listitem("the nested item that moves") listitem("n1X"))', + ); + }); + + test(`a nested item moved down past its sibling ‖ typing in the sibling stays there (ids ${ids})`, async () => { + const t = await setup( + () => + $getRoot().append( + $createListNode('bullet').append( + $createListItemNode().append($createTextNode('one')), + $createListItemNode().append( + $list('the nested item that moves', 'n2'), + ), + ), + ), + {ids}, + ); + t.offline(); + t.a.update(() => { + const [mover, n2] = $nestedList().getChildren(); + n2.insertAfter(mover); + }); + t.b.update(() => $selectIn([0, 1, 0, 1], 2).insertText('X')); + await t.sync(); + await expectConverged(t.peers); + expect(t.a.describe()).toContain( + 'list[bullet](listitem("n2X") listitem("the nested item that moves"))', + ); + }); + + test(`a nested list moved up past the outer items ‖ typing in an outer item stays there (ids ${ids})`, async () => { + const t = await setup(() => $getRoot().append($nested()), {ids}); + t.offline(); + t.a.update(() => { + const [one, , level] = $block([0]).getChildren(); + one.insertBefore(level); + }); + t.b.update(() => $selectIn([0, 0], 3).insertText('X')); + await t.sync(); + await expectConverged(t.peers); + expect(t.a.describe()).toContain('listitem("oneX") listitem("two")'); + }); + } +}); + +describe('recorded moves of tables and table rows', () => { + for (const ids of IDS) { + for (const direction of ['up', 'down'] as const) { + test(`a table moved ${direction} past a paragraph ‖ typing in the paragraph stays there (ids ${ids})`, async () => { + const t = await setup( + () => + direction === 'up' + ? $getRoot().append( + $p('Another line'), + $table('cell one', 'cell two'), + ) + : $getRoot().append( + $table('cell one', 'cell two'), + $p('Another line'), + ), + {ids}, + ); + t.offline(); + const deleted = deletedBy(t.a, () => + t.a.update(() => + direction === 'up' + ? $block([0]).insertBefore($block([1])) + : $block([1]).insertAfter($block([0])), + ), + ); + expect(deleted).not.toContain('Another'); + const at = direction === 'up' ? 0 : 1; + t.b.update(() => $selectIn([at], 7).insertText('X')); + await t.sync(); + await expectConverged(t.peers); + expect(t.a.describe()).toMatch( + direction === 'up' + ? /^table\(.*\) \/ paragraph\("AnotherX line"\)$/ + : /^paragraph\("AnotherX line"\) \/ table\(.*\)$/, + ); + }); + + test(`a table row moved ${direction} past another ‖ typing in the other row stays there (ids ${ids})`, async () => { + const long = 'a row with more text than the other'; + const t = await setup( + () => + $getRoot().append( + direction === 'up' + ? $table('short', long) + : $table(long, 'short'), + ), + {ids}, + ); + t.offline(); + const deleted = deletedBy(t.a, () => + t.a.update(() => { + const [first, second] = $block([0]).getChildren(); + if (direction === 'up') { + first.insertBefore(second); + } else { + second.insertAfter(first); + } + }), + ); + expect(deleted).not.toContain('short'); + const shortRow = direction === 'up' ? 0 : 1; + t.b.update(() => $selectIn([0, shortRow, 0, 0], 5).insertText('X')); + await t.sync(); + await expectConverged(t.peers); + const rows = t.a.read(() => + $block([0]) + .getChildren() + .map(row => row.getTextContent()), + ); + expect(rows).toEqual( + direction === 'up' ? [long, 'shortX'] : ['shortX', long], + ); + }); + } + + test(`a table moved up: a caret in the paragraph it passed stays there (ids ${ids})`, async () => { + const t = await setup( + () => + $getRoot().append($p('Another line'), $table('cell one', 'cell two')), + {ids}, + ); + t.b.update(() => $selectIn([0], 7)); + t.a.update(() => $block([0]).insertBefore($block([1]))); + await t.sync(); + type(t.b, 'X'); + await t.sync(); + await expectConverged(t.peers); + expect(t.a.describe()).toMatch( + /^table\(.*\) \/ paragraph\("AnotherX line"\)$/, + ); + }); + } +}); + +// The limits that remain (y.md, Limits, Moves). The moved block is deleted +// and inserted again: what a peer types in it meanwhile is lost (in a block +// node) or stays at its old place, a peer's caret in it goes to the block now +// at that place, and an undo of the move takes the moved copy back, with what +// a peer typed in it since. Every case converges; its expected failure states +// what a native move would keep. +describe('Moves limit: the block that moved', () => { + type Moved = 'list' | 'table'; + const $doc = (moved: Moved) => () => + $getRoot().append( + $p('Another line'), + moved === 'list' + ? $list('item one', 'item two') + : $table('cell one', 'cell two'), + ); + // The path of the moved block's first line, before and after the move. + const FIRST: Record = { + list: [ + [1, 0], + [0, 0], + ], + table: [ + [1, 0, 0, 0], + [0, 0, 0, 0], + ], + }; + const KEPT: Record = { + list: 'list[bullet](listitem("itemX one") listitem("item two"))', + table: + 'table(tablerow(tablecell(paragraph("cellX one"))) tablerow(tablecell(paragraph("cell two"))))', + }; + const $move = () => $block([0]).insertBefore($block([1])); + const CASES: [ + string, + Moved[], + (t: Setup, moved: Moved) => Promise, + (moved: Moved) => string, + ][] = [ + [ + 'typing in the moved block while it moves stays in it', + ['list', 'table'], + async (t, moved) => { + t.offline(); + t.a.update($move); + t.b.update(() => $selectIn(FIRST[moved][0], 4).insertText('X')); + await t.sync(); + }, + moved => `${KEPT[moved]} / paragraph("Another line")`, + ], + [ + 'a caret in the moved block stays in it', + ['list'], + async (t, moved) => { + t.b.update(() => $selectIn(FIRST[moved][0], 4)); + t.a.update($move); + await t.sync(); + type(t.b, 'X'); + await t.sync(); + }, + moved => `${KEPT[moved]} / paragraph("Another line")`, + ], + [ + 'undoing the move keeps what a peer typed in the block since', + ['list', 'table'], + async (t, moved) => { + t.a.update($move); + await t.sync(); + t.b.update(() => $selectIn(FIRST[moved][1], 4).insertText('X')); + await t.sync(); + t.a.undo(); + await t.sync(); + }, + moved => `paragraph("Another line") / ${KEPT[moved]}`, + ], + ]; + for (const ids of IDS) { + for (const [name, kinds, run, kept] of CASES) { + for (const moved of kinds) { + test(`a ${moved} moved up past a paragraph, ${name}: converges (ids ${ids})`, async () => { + const t = await setup($doc(moved), {ids}); + await run(t, moved); + await expectConverged(t.peers); + }); + test.fails( + `a ${moved} moved up past a paragraph, ${name} (ids ${ids})`, + async () => { + const t = await setup($doc(moved), {ids}); + await run(t, moved); + await expectConverged(t.peers, kept(moved)); + }, + ); + } + } + } +}); + +// Lexical records every node detached in the update, also one put back where +// it was in the same update (an insert next to the sibling it already has, a +// remove and a re-insert, a move away and back). Such a node counts as moved, +// so when it is also the block a real move passes, neither is known to have +// stayed, and the smaller one (here the paragraph) is rewritten. +describe('Moves limit: a node put back where it was in the update of a move', () => { + // Each moves the list up past the paragraph, and puts the paragraph back. + const UPDATES: [string, () => void][] = [ + [ + 'inserted next to the sibling it has, before the move', + () => { + const [paragraph, list] = $getRoot().getChildren(); + list.insertBefore(paragraph); + paragraph.insertBefore(list); + }, + ], + [ + 'inserted next to the sibling it has, after the move', + () => { + const [paragraph, list, end] = $getRoot().getChildren(); + paragraph.insertBefore(list); + end.insertBefore(paragraph); + }, + ], + [ + 'removed and inserted again', + () => { + const [paragraph, list] = $getRoot().getChildren(); + paragraph.remove(); + list.insertBefore(paragraph); + paragraph.insertBefore(list); + }, + ], + [ + 'moved away and back', + () => { + const [paragraph, list, end] = $getRoot().getChildren(); + end.insertAfter(paragraph); + list.insertBefore(paragraph); + paragraph.insertBefore(list); + }, + ], + ]; + const $init = () => + $getRoot().append( + $p('Another line'), + $list('item one', 'item two'), + $p('end'), + ); + for (const ids of IDS) { + for (const [how, $update] of UPDATES) { + const run = async (t: Setup) => { + t.offline(); + t.a.update($update); + t.b.update(() => $selectIn([0], 7).insertText('X')); + await t.sync(); + }; + test(`the paragraph ${how}: converges (ids ${ids})`, async () => { + const t = await setup($init, {ids}); + await run(t); + await expectConverged(t.peers); + }); + test.fails( + `the paragraph ${how} ‖ typing in the paragraph stays there (ids ${ids})`, + async () => { + const t = await setup($init, {ids}); + await run(t); + await expectConverged( + t.peers, + 'list[bullet](listitem("item one") listitem("item two")) / paragraph("AnotherX line") / paragraph("end")', + ); + }, + ); + } + } +}); + +// Without a record of moves (an editor state set whole: here a state from +// before the move), the binding cannot tell which block moved: it keeps the +// side that keeps more blocks and content in place, and rewrites the other. +describe('a reorder with no record of moves', () => { + for (const ids of IDS) { + test(`an editor state set whole that swaps two blocks rewrites the smaller one (ids ${ids})`, async () => { + const t = await setup( + () => + $getRoot().append($p('Another line'), $list('item one', 'item two')), + {ids}, + ); + const before = t.a.editor.getEditorState(); + t.a.update(() => $block([0]).insertBefore($block([1]))); + await t.sync(); + t.offline(); + const deleted = deletedBy(t.a, () => t.a.editor.setEditorState(before)); + expect(deleted).toContain('Another line'); + expect(deleted).not.toContain('item'); + t.b.update(() => $selectIn([0, 0], 4).insertText('X')); + await t.sync(); + await expectConverged( + t.peers, + 'paragraph("Another line") / list[bullet](listitem("itemX one") listitem("item two"))', + ); + }); + } +}); diff --git a/packages/lexical-y/src/internal/Write.ts b/packages/lexical-y/src/internal/Write.ts index d5a53d7c97f..dbe730d7257 100644 --- a/packages/lexical-y/src/internal/Write.ts +++ b/packages/lexical-y/src/internal/Write.ts @@ -1957,11 +1957,17 @@ export class Writer { matches.push([di, oi]); } }); - // Keep as many blocks in place as possible (a moved unit is deleted and - // written again, with any concurrent edit inside it). A list counts as - // one block, however many lines it has. Between equal counts, keep what - // costs most to rewrite: a block node (its whole subtree is copied, and - // what peers write inside it meanwhile dropped) outweighs lines. + // Anchors keep their items; every other unit is deleted and written + // again, and what peers write in it meanwhile is lost (in a block node) + // or stays at its old place. So what this update moved (Lexical's record + // of explicit moves, see $movedUnit) is rewritten, not the units it + // passed: those keep their order (only a move changes it), so all of them + // can stay in place. A moved unit stays too where it is still in order + // between them (put back where it was, indented). With no record (an + // editor state set whole), and among moved units, the cheaper rewrite + // wins: as many blocks as possible stay in place (a list counts as one + // block, however many lines it has), then what costs most to rewrite (a + // block node, whose whole subtree is copied, outweighs lines). const groupSize = new Map(); for (const d of desired) { groupSize.set(d.first, (groupSize.get(d.first) ?? 0) + 1); @@ -1980,23 +1986,26 @@ export class Writer { matches.forEach(([di], k) => weightOf.set(di, big / groupSize.get(desired[di].first)! + costs[k]), ); - // Between choices that keep as many blocks and as much content in place - // (an exact tie), what the user moved (Lexical's record of explicit - // moves) is the one rewritten, not the block it passed. Otherwise the - // cheaper rewrite wins: a long move rewrites the moved block rather than - // every block it passed, and in an adjacent swap the smaller block is - // re-created (a container being dragged keeps a peer's typing inside - // it). - const moved = this.plan.moved; - if (moved !== null && moved !== undefined && moved.size > 0) { - matches.forEach(([di]) => { - const d = desired[di]; - if ((d.key !== null && moved.has(d.key)) || moved.has(d.first)) { - weightOf.set(di, weightOf.get(di)! - 0.5); - } - }); + const weight = ([di]: [number, number]) => weightOf.get(di)!; + const moved = this.plan.moved ?? null; + const unmoved: [number, number][] = []; + const movedMatches: [number, number][] = []; + for (const m of matches) { + if ( + moved !== null && + moved.size > 0 && + $movedUnit(desired[m[0]], container, moved) + ) { + movedMatches.push(m); + } else { + unmoved.push(m); + } } - const anchors = longestIncreasing(matches, ([di]) => weightOf.get(di)!); + const anchors = addInOrder( + longestIncreasing(unmoved, weight), + movedMatches, + weight, + ); const n = anchors.length; // Changed line anchors join a neighbouring gap (the following one first) // so that a split or join diffs their characters with it. @@ -4220,6 +4229,73 @@ function unitCost(d: Desired): number { return n; } +/** + * Whether this update moved desired unit `d` of `container`: Lexical + * recorded its node, or an element between it and the container (the list + * of a list item, the item holding a nested list), as detached and inserted + * again (EditorState#_movedNodes). + */ +function $movedUnit( + d: Desired, + container: ElementNode, + moved: ReadonlySet, +): boolean { + for ( + let node: LexicalNode | null = d.node; + node !== null && !node.is(container); + node = node.getParent() + ) { + if (moved.has(node.getKey())) { + return true; + } + } + return false; +} + +/** + * `anchors` (pairs whose both indices increase) and the heaviest + * subsequence of `pairs` (in order of their first index) that fits in order + * between them: each pair added lies, by both indices, in one gap between + * two consecutive anchors. + */ +function addInOrder( + anchors: [number, number][], + pairs: [number, number][], + weight: (pair: [number, number]) => number, +): [number, number][] { + if (pairs.length === 0) { + return anchors; + } + const fits: [number, number][] = []; + let next = 0; + for (const pair of pairs) { + while (next < anchors.length && anchors[next][0] < pair[0]) { + next++; + } + if ( + (next === 0 || anchors[next - 1][1] < pair[1]) && + (next === anchors.length || pair[1] < anchors[next][1]) + ) { + fits.push(pair); + } + } + const kept = longestIncreasing(fits, weight); + const out: [number, number][] = []; + let a = 0; + let k = 0; + while (a < anchors.length || k < kept.length) { + if ( + k === kept.length || + (a < anchors.length && anchors[a][0] < kept[k][0]) + ) { + out.push(anchors[a++]); + } else { + out.push(kept[k++]); + } + } + return out; +} + /** * The heaviest subsequence of `pairs` (in order of their first index) whose * second indices increase: O(n log n) with a Fenwick tree of maxima. From 1a0ad323b7d227dd2c5cd87c8bc3f28f47335ad6 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Wed, 28 Oct 2026 19:29:35 -0400 Subject: [PATCH 2/3] [lexical-website][lexical-y] Documentation Update: Record two concurrent joins into one line as a known race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Take the blocks `[heading, quote, paragraph]`. Peer A deletes from the end of the heading to the start of the paragraph, across the quote. At the same time, peer B joins the paragraph into the quote (Backspace at its start). With one client id order, every client and a fresh reader end with the heading's and the paragraph's text in a quote, although neither edit retyped the first line. The same happens to a code block. A line is stored as its text followed by a marker that holds its type. So a join deletes the earlier line's marker and writes the earlier line's type onto the later line's marker, which now ends the joined line. Here both joins end at the paragraph's marker: A writes the heading's type there and B writes the quote's, and Yjs keeps one of the two writes by client id. The type is one value on that marker, so the two writes cannot both be kept. Keeping the earlier line's type would need each join to record which line it came from, and a follow-up to elect one. This commit records the race as a known limit and does not change what is stored: - `regressions/Joins.test.ts`: S7-22 for a heading and a code block, with a convergence test for both client id orders and a `test.fails` for the order that loses the first line's type. - y.md, Limits, Other races: one sentence for two joins at once into the same line. It is independent of the reorder fix in the previous commit, since a join reorders nothing. The new tests give the same results with and without that commit. ## Changeset None: a test and website docs only. ## Test plan ### Before The soak repro on `main` (0cd7448e7), `./node_modules/.bin/vitest --run --project unit L2-ConcurrentJoinIntoCode.test.ts`: ``` - "final": "heading", - "merged": "heading", + "final": "quote", + "merged": "quote", "mergedShape": "quote(t:\"const t44z = 1; // t46z note t47z\"[\"c1\"] t:\"t49z t50z t51z\")", Tests 2 failed | 2 passed (4) ``` ### After `./node_modules/.bin/vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/Joins.test.ts`, the same on `main` and on this branch (the behavior is unchanged): ``` ✓ |unit| packages/lexical-y/src/__tests__/unit/regressions/Joins.test.ts (8 tests) 388ms Tests 6 passed | 2 expected fail (8) ``` - `@lexical/y` unit tests with both commits: 150 files passed and 2 skipped; 5,033 tests passed, 40 expected fail, 2 skipped. - `pnpm run ci-check` passes. --- .../lexical-website/docs/collaboration/y.md | 5 +- .../__tests__/unit/regressions/Joins.test.ts | 113 ++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Joins.test.ts diff --git a/packages/lexical-website/docs/collaboration/y.md b/packages/lexical-website/docs/collaboration/y.md index a4ae4dab8ca..570da7b3ff7 100644 --- a/packages/lexical-website/docs/collaboration/y.md +++ b/packages/lexical-website/docs/collaboration/y.md @@ -909,7 +909,10 @@ not merge as a user might expect: inside the earlier deleted text after that, where its format changed more than once along it, takes the format that text had at its start, and so does that text where an undo brings it back after another - collaborator's delete there. + collaborator's delete there. Two joins at once into the same line both + write its type, and Yjs keeps one of them by client id: a heading joined + across a quote to the paragraph after it, while a collaborator joins that + paragraph into the quote, can become a quote. ## Changing the layout and migrating diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Joins.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Joins.test.ts new file mode 100644 index 00000000000..6dab3e50b1f --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Joins.test.ts @@ -0,0 +1,113 @@ +/** + * 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 + +// Regression cases for joins. A line is stored as its text followed by its +// marker, which holds its type, so a join deletes the earlier line's marker +// and writes the earlier line's type onto the later line's marker, which now +// ends the joined line. Each case is named after its finding. + +import {$createCodeNode} from '@lexical/code'; +import {$createHeadingNode, $createQuoteNode} from '@lexical/rich-text'; +import { + $createRangeSelection, + $createTextNode, + $getRoot, + $setSelection, + type ElementNode, + type TextNode, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; + +import {$p, expectConverged, type Setup, setup} from '../../utils/harness'; + +/** Delete from the end of block `from` to the start of block `to`. */ +function $joinBlocks(from: number, to: number) { + const blocks = $getRoot().getChildren() as ElementNode[]; + const end = blocks[from].getLastChild() as TextNode; + const start = blocks[to].getFirstChild() as TextNode; + const selection = $createRangeSelection(); + selection.anchor.set(end.getKey(), end.getTextContentSize(), 'text'); + selection.focus.set(start.getKey(), 0, 'text'); + $setSelection(selection); + selection.removeText(); +} + +// S7-22: two joins at once into the same line. A deletes from the end of a +// heading (or code block) to the start of the paragraph after the quote that +// follows it; B joins that paragraph into the quote (Backspace at its +// start). Both write the type of the line that ends at the paragraph's +// marker, A the heading's and B the quote's, and Yjs keeps one of the two +// writes by client id: when it keeps B's, the heading becomes a quote on +// every client although neither edit retyped it. Open: the type is one +// value on that shared marker, so the two writes cannot both be kept; +// agreeing on the earlier line's type would need each join to record which +// line it came from and a follow-up to elect one (y.md, Limits, Other +// races). +describe('S7-22: two joins at once into the same line', () => { + const FIRST: [string, string, () => ElementNode][] = [ + [ + 'heading', + 'heading', + () => $createHeadingNode('h1').append($createTextNode('Title t44z')), + ], + [ + 'code block', + 'code', + () => $createCodeNode().append($createTextNode('const t44z = 1;')), + ], + ]; + for (const [kind, type, $first] of FIRST) { + const run = async (ids: number[]): Promise => { + const t = await setup( + () => + $getRoot().append( + $first(), + $createQuoteNode().append($createTextNode('t48z')), + $p('t49z t50z'), + ), + {ids}, + ); + t.offline(); + t.a.update(() => $joinBlocks(0, 2)); + t.b.update(() => $joinBlocks(1, 2)); + await t.sync(); + return t; + }; + const firstType = (t: Setup) => + t.a.read(() => $getRoot().getFirstChildOrThrow().getType()); + + for (const ids of [ + [1, 2], + [2, 1], + ]) { + test(`S7-22 ${kind}: converges, with the text of both joins (ids ${ids})`, async () => { + const t = await run(ids); + await expectConverged(t.peers); + expect(t.a.read(() => $getRoot().getTextContent())).not.toContain( + 't48z', + ); + expect(t.a.read(() => $getRoot().getChildrenSize())).toBe(1); + }); + } + + test(`S7-22 ${kind}: the joined line keeps the ${kind}'s type when the client that joined the ${kind} has the higher id`, async () => { + const t = await run([2, 1]); + expect(firstType(t)).toBe(type); + }); + + test.fails( + `S7-22 ${kind}: the joined line keeps the ${kind}'s type when the client that joined the quote has the higher id`, + async () => { + const t = await run([1, 2]); + expect(firstType(t)).toBe(type); + }, + ); + } +}); From 5e40824c4a69dbfb9be8aac73d913c59d7e50afa Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Wed, 28 Oct 2026 23:53:22 -0400 Subject: [PATCH 3/3] [lexical-y] Bug Fix: Self-check a container that a move gave new storage as it is stored now MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description With the first commit of this PR, the block an update moved is the one written anew. When the same update first changed the content of a container inside that block (a list given an item, then moved into a box that moves too), the writer writes the box's container in place, and then writes the box anew, with its content, for its move. The document is right and every client converges, but the development self-check (`selfCheck: true`) compared the box's old container, which the move deleted, with the editor, and warned `self-check: container : editor [...] != shared []`. On `main` the smaller list was written anew instead of the box, so this did not happen. The self-check now compares only containers that are still their node's (`state.csByKey`). The box's new container is compared through the write's structural keys, as before. ## Changeset None: this changes only the opt-in development self-check, and `.changeset/lexical-y-reorder-intent.md` already covers this PR. ## Test plan New in `regressions/Moves.test.ts`: `a block written, then moved, in one update`, 4 tests (an empty box and a box holding a line, in both client id orders). They use `expectConverged`, which requires no self-check warning. `./node_modules/.bin/vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts` ### Before On the first two commits of this PR: ``` × a list given an item, moved into an empty box that moves too (ids 1,2) × a list given an item, moved into a box that moves too (ids 1,2) × a list given an item, moved into an empty box that moves too (ids 2,1) × a list given an item, moved into a box that moves too (ids 2,1) Tests 4 failed | 132 skipped (136) ``` Each fails on `expected [ Array(1) ] to deeply equal []`, the warning `self-check: container 13: editor [{"children":[{"children":[{"detail":0,...,"text":"z0",...}]...}]...] != shared []`. On `main` the same edits give no warning. ### After ``` Test Files 1 passed (1) Tests 118 passed | 18 expected fail (136) ``` - `@lexical/y` unit tests: 150 files passed and 2 skipped; 5,037 tests passed, 40 expected fail, 2 skipped. - `pnpm run ci-check` passes. --- .../__tests__/unit/regressions/Moves.test.ts | 36 +++++++++++++++++++ packages/lexical-y/src/internal/Binding.ts | 6 +++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts index 06e6847f115..71f7b26e584 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/Moves.test.ts @@ -37,6 +37,7 @@ import {describe, expect, test} from 'vitest'; import { $block, + $createBoxNode, $p, $selectIn, expectConverged, @@ -343,6 +344,41 @@ describe('a recorded move rewrites the block that moved, not the blocks it passe } }); +// A container written in place (a list given an item, in a box it moves +// into) and then given new storage by the move of its node, in one update: +// the self-check compares the box as stored now. +describe('a block written, then moved, in one update', () => { + for (const ids of IDS) { + for (const inside of [false, true]) { + test(`a list given an item, moved into ${inside ? 'a' : 'an empty'} box that moves too (ids ${ids})`, async () => { + const t = await setup( + () => { + const box = $createBoxNode(); + if (inside) { + box.append($p('inside')); + } + $getRoot().append($list('a', 'b'), box, $p('gap')); + }, + {ids}, + ); + t.a.update(() => { + const [list, box] = [$block([0]), $block([1])]; + list.splice(0, 0, [ + $createListItemNode().append($createTextNode('z')), + ]); + box.append(list); + $block([1]).insertAfter(box); + }); + await t.sync(); + await expectConverged(t.peers); + expect(t.b.read(() => $block([1]).getTextContent())).toBe( + `${inside ? 'inside\n\n' : ''}z\n\na\n\nb`, + ); + }); + } + } +}); + // The APIs y.md (Limits, Moves) names as recorded moves: each moves the // list here, and the paragraph it passes keeps a peer's typing. describe('every Lexical API that moves a node records the move', () => { diff --git a/packages/lexical-y/src/internal/Binding.ts b/packages/lexical-y/src/internal/Binding.ts index f6415d3085f..f523994041d 100644 --- a/packages/lexical-y/src/internal/Binding.ts +++ b/packages/lexical-y/src/internal/Binding.ts @@ -3035,8 +3035,12 @@ export class YBinding { containers.add(cs); } } + // A container whose node this write gave new storage after writing it + // (a block moved after its content changed) is compared as stored now. for (const cs of writer.groupWrites) { - containers.add(cs); + if (state.csByKey.get(cs.key) === cs) { + containers.add(cs); + } } for (const cs of containers) { if (