From e366d2b23993babc949a69715636a8fedb8e58f7 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Thu, 29 Oct 2026 18:26:49 -0400 Subject: [PATCH 1/6] [lexical-table][lexical-y] Bug Fix: Pad a table's rows again when an edit removes cells from some of them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description A rich paste over a range selection across table cells that hold lists (from the middle of one cell to the middle of a cell in the next row) removes the cells between the two ends, so some rows end up with fewer cells than the others: `2,2,3` cells per row instead of `3,3,3`. The table's transform pads ragged rows (`$padTableRows`), but Lexical runs a transform only on a node marked dirty itself, and removing cells marks their rows dirty, not the table. The grid came back only when something else happened to mark the table dirty in the same update: `NodeStructureExtension`'s normalization does when it repairs a list in a cell, which is why the issue's editor without collaboration kept its grid while the same editor bound with `@lexical/y` (where that update gives the normalization nothing to repair) did not. An editor without `NodeStructureExtension` loses the cells too. The row transform now marks its table dirty, so the table's own transform pads its rows after any change to a row's cells (it already appends the padding cells at the end of the short rows and changes nothing on a full grid). Remote changes applied by `@lexical/y` skip transforms as before; the author's editor pads, and the padding is written like any other edit. Closes #9520 ## Changeset `.changeset/table-pad-rows-after-row-change.md` (`@lexical/table`, `[fix]`). ## Test plan `PasteAcrossCellsPadsRows.test.ts` (`@lexical/table`, with and without `NodeStructureExtension`) and `TablePasteAcrossCells.test.ts` (`@lexical/y`, the issue's repro: a bound editor ends with the same editor state as one without collaboration, and a fresh reader of the document shows the padding cells). ``` pnpm exec vitest run --project unit --reporter=verbose packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts ``` ### Before ``` × |unit| packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts > a paste over a selection across cells > from r0c1 to r1c1 keeps every row as wide as the table × |unit| packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts > a paste over a selection across cells > from r0c2 to r1c1 keeps every row as wide as the table ✓ |unit| packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts > a paste over a selection across cells with NodeStructureExtension > from r0c1 to r1c1 keeps every row as wide as the table ✓ |unit| packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts > a paste over a selection across cells with NodeStructureExtension > from r0c2 to r1c1 keeps every row as wide as the table × |unit| packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts > a rich paste over a selection from r0c1 to r1c1 across table cells holding lists gives the same document with and without collaboration × |unit| packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts > a rich paste over a selection from r0c2 to r1c1 across table cells holding lists gives the same document with and without collaboration ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 4 ⎯⎯⎯⎯⎯⎯⎯ AssertionError: expected '2,2,3' to be '3,3,3' // Object.is equality AssertionError: expected '3,2,3' to be '3,3,3' // Object.is equality AssertionError: expected '2,2,3' to be '3,3,3' // Object.is equality AssertionError: expected '3,2,3' to be '3,3,3' // Object.is equality Test Files 2 failed (2) Tests 4 failed | 2 passed (6) ``` ### After ``` ✓ |unit| packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts > a paste over a selection across cells > from r0c1 to r1c1 keeps every row as wide as the table ✓ |unit| packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts > a paste over a selection across cells > from r0c2 to r1c1 keeps every row as wide as the table ✓ |unit| packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts > a paste over a selection across cells with NodeStructureExtension > from r0c1 to r1c1 keeps every row as wide as the table ✓ |unit| packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts > a paste over a selection across cells with NodeStructureExtension > from r0c2 to r1c1 keeps every row as wide as the table ✓ |unit| packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts > a rich paste over a selection from r0c1 to r1c1 across table cells holding lists gives the same document with and without collaboration ✓ |unit| packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts > a rich paste over a selection from r0c2 to r1c1 across table cells holding lists gives the same document with and without collaboration Test Files 2 passed (2) Tests 6 passed (6) ``` Full unit suite (`pnpm exec vitest run --project unit`): 568 files, 14,657 passed, 46 expected fail, 0 failed. `@lexical/y` fuzz with more seeds than the suite runs (`FUZZ_SEED_START=1000 FUZZ_SEEDS=2000 FUZZ_GC_SEEDS=500 SHAPE_FUZZ_SEEDS=400 FOLLOWUP_FUZZ_SEEDS=600`, `Fuzz`, `NestedFuzz`, `ConcurrentShapesFuzz`, `FollowUpFuzz`, `FuzzReplay`): 767 passed, 1 failed, seed 2277 (`dup`), which fails the same way on `main` without this change. `pnpm run ci-check` passes. --- .changeset/table-pad-rows-after-row-change.md | 7 + .../src/LexicalTablePluginHelpers.ts | 8 +- .../unit/PasteAcrossCellsPadsRows.test.ts | 121 ++++++++++++++ .../unit/TablePasteAcrossCells.test.ts | 152 ++++++++++++++++++ 4 files changed, 287 insertions(+), 1 deletion(-) create mode 100644 .changeset/table-pad-rows-after-row-change.md create mode 100644 packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts diff --git a/.changeset/table-pad-rows-after-row-change.md b/.changeset/table-pad-rows-after-row-change.md new file mode 100644 index 00000000000..7f651dfe897 --- /dev/null +++ b/.changeset/table-pad-rows-after-row-change.md @@ -0,0 +1,7 @@ +--- +'@lexical/table': patch +--- + +[fix] Pad the rows of a table again when an edit removes cells from some of its rows, such as a rich paste over a selection across cells that hold lists, so the table keeps a full grid + +Verify: select from the middle of one cell to the middle of a cell in the next row, in a table whose cells hold lists, paste HTML that holds a list, and check that every row keeps as many cells as the table has columns (the padding cells are appended at the end of the rows that lost cells, as before). diff --git a/packages/lexical-table/src/LexicalTablePluginHelpers.ts b/packages/lexical-table/src/LexicalTablePluginHelpers.ts index ece64935bf8..96d587a0a2b 100644 --- a/packages/lexical-table/src/LexicalTablePluginHelpers.ts +++ b/packages/lexical-table/src/LexicalTablePluginHelpers.ts @@ -118,12 +118,18 @@ function $tableCellTransform(node: TableCellNode) { } function $tableRowTransform(node: TableRowNode) { - if (!$isTableNode(node.getParent())) { + const table = node.getParent(); + if (!$isTableNode(table)) { // TableRowNode must be a child of TableNode. // TODO: Future support of tbody/thead/tfoot may change this node.remove(); } else { $unwrapAndFilterDescendants(node, $isTableCellNode); + // A row whose cells changed (a delete across cells removes some) can + // leave the table ragged. The table's transform pads its rows, and + // Lexical runs it only on a table marked dirty itself, which a change + // to one of its rows does not do. + table.markDirty(); } } diff --git a/packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts b/packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts new file mode 100644 index 00000000000..4b5ff85b114 --- /dev/null +++ b/packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts @@ -0,0 +1,121 @@ +/** + * 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. + * + */ + +import {$insertGeneratedNodes} from '@lexical/clipboard'; +import { + buildEditorFromExtensions, + NodeStructureExtension, +} from '@lexical/extension'; +import {$generateNodesFromDOM} from '@lexical/html'; +import { + $createListItemNode, + $createListNode, + ListExtension, +} from '@lexical/list'; +import {RichTextExtension} from '@lexical/rich-text'; +import { + $createTableCellNode, + $createTableNode, + $createTableRowNode, + $isTableNode, + $isTableRowNode, + TableExtension, +} from '@lexical/table'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getEditor, + $getRoot, + $setSelection, + configExtension, + defineExtension, +} from 'lexical'; +import {assert, describe, expect, test} from 'vitest'; + +// A 3x3 table whose cells each hold a bullet list with one item "rNcM text". +function $createListTable() { + const table = $createTableNode(); + for (let r = 0; r < 3; r++) { + const row = $createTableRowNode(); + for (let c = 0; c < 3; c++) { + row.append( + $createTableCellNode(0).append( + $createListNode('bullet').append( + $createListItemNode().append($createTextNode(`r${r}c${c} text`)), + ), + ), + ); + } + table.append(row); + } + $getRoot().clear().append(table, $createParagraphNode()); +} + +// Select from offset 2 of the text starting with `from` to offset 2 of the +// text starting with `to`, and paste rich HTML holding a list over it. +function $pasteAcross(from: string, to: string) { + const texts = $getRoot().getAllTextNodes(); + const find = (prefix: string) => { + const text = texts.find(t => t.getTextContent().startsWith(prefix)); + assert(text !== undefined, `no text starting with ${prefix}`); + return text; + }; + const selection = $createRangeSelection(); + selection.anchor.set(find(from).getKey(), 2, 'text'); + selection.focus.set(find(to).getKey(), 2, 'text'); + $setSelection(selection); + const html = '

bold italic

'; + $insertGeneratedNodes( + $getEditor(), + $generateNodesFromDOM( + $getEditor(), + new DOMParser().parseFromString(html, 'text/html'), + ), + selection, + ); +} + +function $cellsPerRow() { + const table = $getRoot().getFirstChild(); + assert($isTableNode(table), 'expected a TableNode at the root'); + return table + .getChildren() + .filter($isTableRowNode) + .map(row => row.getChildrenSize()) + .join(','); +} + +describe.each([ + ['', []], + [' with NodeStructureExtension', [NodeStructureExtension]], +] as const)('a paste over a selection across cells%s', (_name, extra) => { + test.each([ + ['r0c1', 'r1c1'], + ['r0c2', 'r1c1'], + ])('from %s to %s keeps every row as wide as the table', (from, to) => { + using editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + dependencies: [ + ...extra, + RichTextExtension, + ListExtension, + configExtension(TableExtension, {hasHorizontalScroll: false}), + ], + name: 'paste-across-cells', + onError: e => { + throw e; + }, + }), + ); + editor.update($createListTable, {discrete: true}); + editor.update(() => $pasteAcross(from, to), {discrete: true}); + expect(editor.read($cellsPerRow)).toBe('3,3,3'); + }); +}); diff --git a/packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts b/packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts new file mode 100644 index 00000000000..1e771606638 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts @@ -0,0 +1,152 @@ +/** + * 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. + * + */ + +import {$insertGeneratedNodes} from '@lexical/clipboard'; +import { + buildEditorFromExtensions, + NodeStructureExtension, +} from '@lexical/extension'; +import {$generateNodesFromDOM} from '@lexical/html'; +import { + $createListItemNode, + $createListNode, + ListExtension, +} from '@lexical/list'; +import {RichTextExtension} from '@lexical/rich-text'; +import { + $createTableCellNode, + $createTableNode, + $createTableRowNode, + $isTableNode, + $isTableRowNode, + TableExtension, +} from '@lexical/table'; +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getEditor, + $getRoot, + $setSelection, + configExtension, + defineExtension, + type TextNode, +} from 'lexical'; +import {assert, expect, test} from 'vitest'; + +import {createYDoc, YCollabExtension, type YDoc} from '../..'; +import {readYState} from '../../headless'; + +const EXTENSIONS = [ + NodeStructureExtension, + RichTextExtension, + ListExtension, + configExtension(TableExtension, {hasHorizontalScroll: false}), +]; + +function buildEditor(doc: YDoc | null) { + return buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + dependencies: [ + ...EXTENSIONS, + ...(doc + ? [ + configExtension(YCollabExtension, { + doc, + schemaVersion: 1, + synced: true, + }), + ] + : []), + ], + name: 'table-paste-across-cells', + onError: e => { + throw e; + }, + }), + ); +} + +// A 3x3 table; each cell holds a bullet list with one item "rNcM text". +function $init() { + const table = $createTableNode(); + for (let r = 0; r < 3; r++) { + const row = $createTableRowNode(); + for (let c = 0; c < 3; c++) { + row.append( + $createTableCellNode(0).append( + $createListNode('bullet').append( + $createListItemNode().append($createTextNode(`r${r}c${c} text`)), + ), + ), + ); + } + table.append(row); + } + $getRoot().clear().append(table, $createParagraphNode()); +} + +// Select from offset 2 of `from` to offset 2 of `to` and paste rich HTML. +const $paste = (from: string, to: string) => () => { + const texts = $getRoot().getAllTextNodes(); + const find = (prefix: string) => + texts.find((t: TextNode) => t.getTextContent().startsWith(prefix))!; + const selection = $createRangeSelection(); + selection.anchor.set(find(from).getKey(), 2, 'text'); + selection.focus.set(find(to).getKey(), 2, 'text'); + $setSelection(selection); + const html = '

bold italic

'; + $insertGeneratedNodes( + $getEditor(), + $generateNodesFromDOM( + $getEditor(), + new DOMParser().parseFromString(html, 'text/html'), + ), + selection, + ); +}; + +function $cellsPerRow() { + const table = $getRoot().getFirstChild(); + assert($isTableNode(table), 'expected a TableNode at the root'); + return table + .getChildren() + .filter($isTableRowNode) + .map(row => row.getChildrenSize()) + .join(','); +} + +test.each([ + ['r0c1', 'r1c1'], + ['r0c2', 'r1c1'], +])( + 'a rich paste over a selection from %s to %s across table cells holding lists gives the same document with and without collaboration', + (from, to) => { + const doc = createYDoc(); + const [alone, bound] = [null, doc].map(d => { + const editor = buildEditor(d); + editor.update($init, {discrete: true}); + editor.update($paste(from, to), {discrete: true}); + return editor; + }); + expect(bound.read($cellsPerRow)).toBe('3,3,3'); + expect(bound.getEditorState().toJSON()).toEqual( + alone.getEditorState().toJSON(), + ); + // The padding cells are stored: a fresh reader of the document shows them + expect( + readYState( + Y.encodeStateAsUpdate(doc), + {extensions: EXTENSIONS, schemaVersion: 1}, + () => $getEditor().getEditorState().toJSON(), + ), + ).toEqual(bound.getEditorState().toJSON()); + }, +); From 72ecb3caf688d4412e7df9a72a40448227f2e3ad Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 09:06:52 -0400 Subject: [PATCH 2/6] [lexical-table][lexical-y][lexical-yjs] Bug Fix: Pad only the rows whose cells changed, so peers that pad their own rows after remote changes stop padding each other's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Follow-up to this PR's row transform. Marking the table dirty from the row transform ran the table's transform, which pads every short row, after any change to one row. With collaboration, a table can be ragged by edits made at the same time: a column insert concurrent with two row inserts leaves the two new rows short, one per author. An app that pads, after remote changes, the short rows each client created (so that no two clients pad the same row; reported downstream with the v2 `@lexical/yjs` binding) then padded one row and, through the table's transform, every other short row too. Two clients padded each other's rows at the same time, the duplicated cells made the other rows short, and the next repair padded again: with three clients, 30 of 40 delivery orders ended with 4 to 29 columns, and in the downstream harness some never stopped sending updates. Two clients, or no repair, were not enough to show it. The row transform now records the rows whose cells changed (per update) and marks the table dirty, as before. The table's transform pads every row when the update changed the table's rows (a row added, removed or moved, compared with the editor state before the update, also where a later transform pass did it), as on main; when the table's rows are the ones it had, it pads only the recorded rows (`$padTableRows` takes an optional `$shouldPad` to limit it, in its TypeScript and Flow types). A delete across cells that removes some from a row still gets that row padded in the same update, which is this PR's fix, and short rows the edit did not touch stay as they are, for their authors to pad. The table map is computed once per update, as before: a column insert in an 800-row table takes about 60 ms, as on main (padding each row from its own transform took 1.9 s). ## Test plan - `packages/lexical-yjs/src/__tests__/unit/TableRowRepairCollab.test.ts`: three v2 peers that pad the rows they created after remote changes; a column insert concurrent with two row inserts, 40 delivery orders, each must end quiet with 3 cells in every row; and the two-peer case. - `packages/lexical-y/src/__tests__/unit/TableRowRepairCollab.test.ts`: the same with `@lexical/y` peers (each pads the rows it inserted after each remote update). - `PasteAcrossCellsPadsRows.test.ts`: a change to one row of a ragged table pads that row only; an update that removes a cell from one row and adds a row, in the update or in a later row transform, pads every short row. ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-yjs/src/__tests__/unit/TableRowRepairCollab.test.ts packages/lexical-y/src/__tests__/unit/TableRowRepairCollab.test.ts packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts lexical-yjs TableRowRepairCollab: Tests 30 failed | 50 passed (80) lexical-y TableRowRepairCollab: Tests 30 failed | 10 passed (40) PasteAcrossCellsPadsRows: Tests 1 failed | 4 passed (5) AssertionError: expected { left: +0, …(1) } to deeply equal { left: +0, …(1) } AssertionError: expected '3,3,3' to be '3,2,3' // Object.is equality ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-yjs/src/__tests__/unit/TableRowRepairCollab.test.ts packages/lexical-y/src/__tests__/unit/TableRowRepairCollab.test.ts packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts Test Files 4 passed (4) Tests 133 passed (133) $ pnpm exec vitest run --project unit packages/lexical-table packages/lexical-yjs packages/lexical-y packages/lexical-react packages/lexical-playground Test Files 266 passed | 2 skipped (268) Tests 8511 passed | 46 expected fail | 4 skipped (8561) ``` On main the v2 case passes (each owner pads its own row once). A matrix of concurrent table edits with owner-gated repairs over `@lexical/y`, v1 and v2 bindings (726 cases) passes here; on this PR's previous head 22 of its owner-repair cases failed. Its case where every peer marks each table dirty after every remote change still grows the table, exactly as on main: there the table changes itself, and such an update pads every row, as before. `pnpm run ci-check` passes. --- .changeset/table-pad-rows-after-row-change.md | 2 +- .../lexical-table/flow/LexicalTable.js.flow | 5 +- .../src/LexicalTablePluginHelpers.ts | 69 +++- .../lexical-table/src/LexicalTableUtils.ts | 11 +- .../unit/PasteAcrossCellsPadsRows.test.ts | 110 +++++++ .../unit/TableRowRepairCollab.test.ts | 225 +++++++++++++ .../unit/TableRowRepairCollab.test.ts | 306 ++++++++++++++++++ 7 files changed, 719 insertions(+), 9 deletions(-) create mode 100644 packages/lexical-y/src/__tests__/unit/TableRowRepairCollab.test.ts create mode 100644 packages/lexical-yjs/src/__tests__/unit/TableRowRepairCollab.test.ts diff --git a/.changeset/table-pad-rows-after-row-change.md b/.changeset/table-pad-rows-after-row-change.md index 7f651dfe897..c61aa331a24 100644 --- a/.changeset/table-pad-rows-after-row-change.md +++ b/.changeset/table-pad-rows-after-row-change.md @@ -2,6 +2,6 @@ '@lexical/table': patch --- -[fix] Pad the rows of a table again when an edit removes cells from some of its rows, such as a rich paste over a selection across cells that hold lists, so the table keeps a full grid +[fix] Pad the rows of a table again when an edit removes cells from some of its rows, such as a rich paste over a selection across cells that hold lists, so the table keeps a full grid. Only the rows whose cells changed are padded: short rows the edit did not touch (a table ragged by collaborators' concurrent row and column inserts) are left to their authors Verify: select from the middle of one cell to the middle of a cell in the next row, in a table whose cells hold lists, paste HTML that holds a list, and check that every row keeps as many cells as the table has columns (the padding cells are appended at the end of the rows that lost cells, as before). diff --git a/packages/lexical-table/flow/LexicalTable.js.flow b/packages/lexical-table/flow/LexicalTable.js.flow index 83cd2a9d3c4..a9009dbf644 100644 --- a/packages/lexical-table/flow/LexicalTable.js.flow +++ b/packages/lexical-table/flow/LexicalTable.js.flow @@ -488,4 +488,7 @@ declare export function $moveTableRow( declare export function $mergeCells(cellNodes: TableCellNode[]): TableCellNode | null; /** @experimental */ -declare export function $padTableRows(tableNode: TableNode): boolean; +declare export function $padTableRows( + tableNode: TableNode, + $shouldPad?: (row: TableRowNode) => boolean, +): boolean; diff --git a/packages/lexical-table/src/LexicalTablePluginHelpers.ts b/packages/lexical-table/src/LexicalTablePluginHelpers.ts index 96d587a0a2b..73258335bc0 100644 --- a/packages/lexical-table/src/LexicalTablePluginHelpers.ts +++ b/packages/lexical-table/src/LexicalTablePluginHelpers.ts @@ -19,6 +19,7 @@ import { import { $createParagraphNode, $findMatchingParent, + $getEditor, $getNearestNodeFromDOMNode, $getPreviousSelection, $getRoot, @@ -33,6 +34,7 @@ import { COMMAND_PRIORITY_HIGH, COMMAND_PRIORITY_LOW, type CommandPayloadType, + type EditorState, type ElementNode, isDOMNode, type LexicalEditor, @@ -117,6 +119,13 @@ function $tableCellTransform(node: TableCellNode) { } } +/** + * Per update (its pending editor state), by table, the rows whose cells the + * update changed (see $tableRowTransform): what the table's transform pads + * when the update did not change the table's rows (see $tableTransform). + */ +const rowsChanged = new WeakMap>>(); + function $tableRowTransform(node: TableRowNode) { const table = node.getParent(); if (!$isTableNode(table)) { @@ -125,20 +134,70 @@ function $tableRowTransform(node: TableRowNode) { node.remove(); } else { $unwrapAndFilterDescendants(node, $isTableCellNode); - // A row whose cells changed (a delete across cells removes some) can - // leave the table ragged. The table's transform pads its rows, and - // Lexical runs it only on a table marked dirty itself, which a change - // to one of its rows does not do. + // A row whose cells changed (a delete across cells removes some) can be + // shorter than the table. The table's transform pads it, and Lexical + // runs that only on a table marked dirty itself, which a change to one + // of its rows does not do. + const pending = $getEditor()._pendingEditorState; + if (pending !== null) { + let tables = rowsChanged.get(pending); + if (tables === undefined) { + tables = new Map(); + rowsChanged.set(pending, tables); + } + let rows = tables.get(table.getKey()); + if (rows === undefined) { + rows = new Set(); + tables.set(table.getKey(), rows); + } + rows.add(node.getKey()); + } table.markDirty(); } } +/** + * Whether the running update left the rows of `table` as they were (the + * same rows in the same order, in a table that was there before it). + */ +function $sameRows(table: TableNode): boolean { + const before = $getEditor()._editorState._nodeMap; + const previous = before.get(table.getKey()); + if (!$isTableNode(previous)) { + return false; + } + const keys = table.getChildrenKeys(); + let key = previous.__first; + for (const current of keys) { + if (key !== current) { + return false; + } + const row = before.get(key); + key = row === undefined ? null : row.__next; + } + return key === null; +} + function $tableTransform(node: TableNode) { // TableRowNode is the only valid child for TableNode // TODO: Future support of tbody/thead/tfoot/caption may change this $unwrapAndFilterDescendants(node, $isTableRowNode); - $padTableRows(node); + // A table whose rows the update changed (a row added, removed or moved) + // gets all its rows padded. One whose rows are the ones it had gets the + // rows whose cells changed padded, and no other: the other short rows (a + // table ragged by collaborators' concurrent row and column inserts) are + // their authors' to pad, and a client padding them at the same time as + // their authors duplicates cells, which makes the other rows short in + // turn. + const pending = $getEditor()._pendingEditorState; + const changed = pending === null ? undefined : rowsChanged.get(pending); + const rows = changed === undefined ? undefined : changed.get(node.getKey()); + if (rows !== undefined && $sameRows(node)) { + $padTableRows(node, row => rows.has(row.getKey())); + } else { + $padTableRows(node); + } const colWidths = node.getColWidths(); const columnCount = node.getColumnCount(); if (colWidths && colWidths.length !== columnCount) { diff --git a/packages/lexical-table/src/LexicalTableUtils.ts b/packages/lexical-table/src/LexicalTableUtils.ts index 147118ef341..5c3c2543092 100644 --- a/packages/lexical-table/src/LexicalTableUtils.ts +++ b/packages/lexical-table/src/LexicalTableUtils.ts @@ -1182,9 +1182,16 @@ export function $computeTableMapSkipCellCheck( * a column) are merged, since remote updates are applied with transforms * skipped. * + * `$shouldPad` limits the padding to the rows it accepts (all by default): + * the table transform pads only the rows an update changed when nothing + * else about the table changed. + * * @experimental */ -export function $padTableRows(tableNode: TableNode): boolean { +export function $padTableRows( + tableNode: TableNode, + $shouldPad: (row: TableRowNode) => boolean = () => true, +): boolean { const [gridMap] = $computeTableMapSkipCellCheck(tableNode, null, null); const maxRowLength = gridMap.reduce( (curLength, row) => Math.max(curLength, row.length), @@ -1194,7 +1201,7 @@ export function $padTableRows(tableNode: TableNode): boolean { let changed = false; for (let i = 0; i < gridMap.length; ++i) { const rowNode = rowNodes[i]; - if (!$isTableRowNode(rowNode)) { + if (!$isTableRowNode(rowNode) || !$shouldPad(rowNode)) { continue; } // Count the grid positions this row covers, including those covered by a diff --git a/packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts b/packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts index 4b5ff85b114..80dddf448b4 100644 --- a/packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts +++ b/packages/lexical-table/src/__tests__/unit/PasteAcrossCellsPadsRows.test.ts @@ -25,6 +25,7 @@ import { $isTableNode, $isTableRowNode, TableExtension, + TableRowNode, } from '@lexical/table'; import { $createParagraphNode, @@ -119,3 +120,112 @@ describe.each([ expect(editor.read($cellsPerRow)).toBe('3,3,3'); }); }); + +test('a change to one row pads that row only, not the other short rows', () => { + // Rows ragged before the update (a concurrent row and column insert from + // collaborators, merged without transforms) belong to whoever made them: + // padding them here would race with their authors' own padding. + using editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + dependencies: [ + RichTextExtension, + configExtension(TableExtension, {hasHorizontalScroll: false}), + ], + name: 'pad-changed-row', + onError: e => { + throw e; + }, + }), + ); + editor.update( + () => { + const table = $createTableNode(); + for (const width of [3, 2, 3]) { + const row = $createTableRowNode(); + for (let c = 0; c < width; c++) { + row.append($createTableCellNode(0).append($createParagraphNode())); + } + table.append(row); + } + $getRoot().clear().append(table, $createParagraphNode()); + }, + {discrete: true, skipTransforms: true}, + ); + expect(editor.read($cellsPerRow)).toBe('3,2,3'); + editor.update( + () => { + const table = $getRoot().getFirstChild(); + assert($isTableNode(table), 'expected a TableNode at the root'); + const last = table.getLastChild(); + assert($isTableRowNode(last), 'expected a row'); + last.getLastChildOrThrow().remove(); + }, + {discrete: true}, + ); + expect(editor.read($cellsPerRow)).toBe('3,2,3'); +}); + +test.each([false, true])( + 'an update that adds a row pads every short row, also when a row transform adds it after rows changed (in a transform: %s)', + inTransform => { + using editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + dependencies: [ + RichTextExtension, + configExtension(TableExtension, {hasHorizontalScroll: false}), + ], + name: 'pad-after-row-added', + onError: e => { + throw e; + }, + }), + ); + const $row = (width: number) => { + const row = $createTableRowNode(); + for (let c = 0; c < width; c++) { + row.append($createTableCellNode(0).append($createParagraphNode())); + } + return row; + }; + let first = ''; + let armed = false; + // Registered before the table exists, so that registering it does not + // run the transforms on the ragged table. + const remove = editor.registerNodeTransform(TableRowNode, row => { + if (armed && row.getKey() === first) { + armed = false; + row.insertBefore($row(3)); + } + }); + editor.update( + () => { + const table = $createTableNode(); + for (const width of [3, 3, 2]) { + table.append($row(width)); + } + $getRoot().clear().append(table, $createParagraphNode()); + first = table.getFirstChildOrThrow().getKey(); + }, + {discrete: true, skipTransforms: true}, + ); + expect(editor.read($cellsPerRow)).toBe('3,3,2'); + armed = inTransform; + editor.update( + () => { + const table = $getRoot().getFirstChild(); + assert($isTableNode(table), 'expected a TableNode at the root'); + const row = table.getFirstChild(); + assert($isTableRowNode(row), 'expected a row'); + row.getLastChildOrThrow().remove(); + if (!inTransform) { + row.insertBefore($row(3)); + } + }, + {discrete: true}, + ); + remove(); + expect(editor.read($cellsPerRow)).toBe('3,3,3,3'); + }, +); diff --git a/packages/lexical-y/src/__tests__/unit/TableRowRepairCollab.test.ts b/packages/lexical-y/src/__tests__/unit/TableRowRepairCollab.test.ts new file mode 100644 index 00000000000..f846f5333e7 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/TableRowRepairCollab.test.ts @@ -0,0 +1,225 @@ +/** + * 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 + +// A column insert concurrent with two row inserts leaves two short rows, one +// per author. Each peer pads the short rows it created once it has the +// others' changes (an app's repair after remote changes, owned so that no +// two peers pad the same row). Its update runs the table's transforms: they +// must pad only the row that changed. Padding every short row, as marking +// the table dirty did, made peers pad each other's rows at the same time, +// and the duplicated cells made the other rows short again (reported +// downstream with the v2 binding; see lexical-yjs TableRowRepairCollab). + +import { + buildEditorFromExtensions, + type LexicalEditorWithDispose, +} from '@lexical/extension'; +import {RichTextExtension} from '@lexical/rich-text'; +import { + $createTableCellNode, + $createTableNodeWithDimensions, + $insertTableColumnAtSelection, + $insertTableRowAtSelection, + $isTableCellNode, + $isTableNode, + $isTableRowNode, + TableExtension, +} from '@lexical/table'; +import * as Y from '@y/y'; +import { + $createParagraphNode, + $getNodeByKey, + $getRoot, + $isElementNode, + configExtension, + defineExtension, + type NodeKey, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; + +import {createYDoc, YCollabExtension, type YDoc} from '../..'; + +type Message = {from: Peer; to: Peer; update: Uint8Array}; + +const REMOTE = 'remote'; + +class Peer { + readonly doc: YDoc = createYDoc(); + readonly editor: LexicalEditorWithDispose; + /** The rows this peer inserted. */ + readonly own = new Set(); + + constructor( + name: string, + readonly network: Message[], + readonly peers: Peer[], + ) { + this.doc.clientID = 1000 + name.charCodeAt(0); + this.editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + dependencies: [ + RichTextExtension, + configExtension(TableExtension, {hasHorizontalScroll: false}), + configExtension(YCollabExtension, { + doc: this.doc, + schemaVersion: 1, + synced: true, + }), + ], + name: `[table-${name}]`, + onError: e => { + throw e; + }, + }), + ); + this.doc.on('update', (update: Uint8Array, origin: unknown) => { + if (origin === REMOTE) { + return; + } + for (const peer of this.peers) { + if (peer !== this) { + this.network.push({from: this, to: peer, update}); + } + } + }); + } + + update(fn: () => void) { + this.editor.update(fn, {discrete: true}); + } + + /** Apply a remote update, then pad the short rows this peer inserted. */ + apply(update: Uint8Array) { + Y.applyUpdate(this.doc, update, REMOTE); + this.editor.read(() => {}); + this.update(() => { + const table = $getRoot().getChildren().find($isTableNode); + if (table === undefined) { + return; + } + const rows = table.getChildren(); + const width = Math.max( + ...rows.map(row => ($isElementNode(row) ? row.getChildrenSize() : 0)), + ); + for (const key of this.own) { + const row = $getNodeByKey(key); + if ($isTableRowNode(row) && row.isAttached()) { + for (let i = row.getChildrenSize(); i < width; i++) { + row.append($createTableCellNode().append($createParagraphNode())); + } + } + } + }); + } + + cellsPerRow(): string { + return this.editor.read(() => { + const table = $getRoot().getChildren().find($isTableNode); + return table === undefined + ? '' + : table + .getChildren() + .map(row => + $isElementNode(row) + ? row.getChildren().filter($isTableCellNode).length + : 0, + ) + .join(','); + }); + } +} + +function $selectCell(row: number, col: number) { + const table = $getRoot().getChildren().find($isTableNode)!; + const r = table.getChildAtIndex(row); + const cell = $isElementNode(r) ? r.getChildAtIndex(col) : null; + if (!$isElementNode(cell)) { + throw new Error('no cell'); + } + cell.selectStart(); +} + +function mulberry32(seed: number) { + let a = seed; + return () => { + a |= 0; + a = (a + 0x6d2b79f5) | 0; + let t = Math.imul(a ^ (a >>> 15), 1 | a); + t ^= t + Math.imul(t ^ (t >>> 7), 61 | t); + return ((t ^ (t >>> 14)) >>> 0) / 4294967296; + }; +} + +function run(interleaving: number) { + const network: Message[] = []; + const peers: Peer[] = []; + const all = ['A', 'B', 'C'].map(name => new Peer(name, network, peers)); + peers.push(...all); + const [a, b, c] = all; + a.update(() => { + $getRoot() + .clear() + .append($createTableNodeWithDimensions(2, 2, false)); + }); + for (const peer of [b, c]) { + Y.applyUpdate(peer.doc, Y.encodeStateAsUpdate(a.doc), REMOTE); + peer.editor.read(() => {}); + } + network.length = 0; + a.update(() => { + $selectCell(0, 1); + $insertTableColumnAtSelection(true); + }); + b.update(() => { + $selectCell(1, 0); + const row = $insertTableRowAtSelection(true); + if (row !== null) { + b.own.add(row.getKey()); + } + }); + c.update(() => { + $selectCell(0, 0); + const row = $insertTableRowAtSelection(true); + if (row !== null) { + c.own.add(row.getKey()); + } + }); + const random = mulberry32(interleaving); + let delivered = 0; + while (network.length > 0 && delivered < 500) { + const pick = + network[interleaving === 0 ? 0 : Math.floor(random() * network.length)]; + const i = network.findIndex(m => m.to === pick.to && m.from === pick.from); + const [{to, update}] = network.splice(i, 1); + to.apply(update); + delivered++; + } + const result = { + left: network.length, + shapes: peers.map(p => p.cellsPerRow()), + }; + for (const peer of all) { + peer.editor.dispose(); + } + return result; +} + +describe('@lexical/y peers that pad the table rows they created after remote changes', () => { + test.each(Array.from({length: 40}, (_, i) => i))( + 'a column insert concurrent with two row inserts settles with 3 cells in every row (interleaving %i)', + interleaving => { + expect(run(interleaving)).toEqual({ + left: 0, + shapes: ['3,3,3,3', '3,3,3,3', '3,3,3,3'], + }); + }, + ); +}); diff --git a/packages/lexical-yjs/src/__tests__/unit/TableRowRepairCollab.test.ts b/packages/lexical-yjs/src/__tests__/unit/TableRowRepairCollab.test.ts new file mode 100644 index 00000000000..d7fb7cacdc6 --- /dev/null +++ b/packages/lexical-yjs/src/__tests__/unit/TableRowRepairCollab.test.ts @@ -0,0 +1,306 @@ +/** + * 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 + +// A column insert concurrent with two row inserts leaves two short rows, one +// per author. Each peer pads the short rows it created once it has the +// others' changes (a repair after remote changes, owned so that no two peers +// pad the same row; reported downstream). The table's row transform pads +// only the row that changed: padding every short row, as marking the table +// dirty did, made peers pad each other's rows at the same time, and the +// duplicated cells made the other rows short again, without end. + +import {buildEditorFromExtensions} from '@lexical/extension'; +import {RichTextExtension} from '@lexical/rich-text'; +import { + $createTableCellNode, + $createTableNodeWithDimensions, + $insertTableColumnAtSelection, + $insertTableRowAtSelection, + $isTableCellNode, + $isTableNode, + $isTableRowNode, + TableExtension, +} from '@lexical/table'; +import { + type BindingV2, + createBindingV2__EXPERIMENTAL, + type Provider, + syncLexicalUpdateToYjsV2__EXPERIMENTAL, + syncYjsChangesToLexicalV2__EXPERIMENTAL, +} from '@lexical/yjs'; +import { + $createParagraphNode, + $getRoot, + $isElementNode, + COLLABORATION_TAG, + configExtension, + type LexicalEditor, + SKIP_COLLAB_TAG, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; +import { + applyUpdate, + Doc, + encodeStateAsUpdate, + UndoManager, + type XmlElement, +} 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: Peer; to: Peer; update: Uint8Array}; + +class Peer { + readonly doc = new Doc({gc: false}); + readonly editor: LexicalEditor; + readonly binding: BindingV2; + + constructor( + name: string, + readonly network: Message[], + readonly peers: Peer[], + ) { + this.doc.clientID = 1000 + name.charCodeAt(0); + const editor = buildEditorFromExtensions({ + $initialEditorState: null, + dependencies: [ + RichTextExtension, + configExtension(TableExtension, {hasHorizontalScroll: false}), + ], + name: `[table-${name}]`, + onError: e => { + throw e; + }, + }); + this.editor = editor; + const binding = createBindingV2__EXPERIMENTAL( + editor, + 'main', + this.doc, + new Map([['main', this.doc]]), + ); + this.binding = binding; + (binding.root as XmlElement).observeDeep((( + events: never, + transaction: {origin: unknown}, + ) => { + if (transaction.origin !== binding) { + syncYjsChangesToLexicalV2__EXPERIMENTAL( + binding, + provider, + events, + transaction as never, + transaction.origin instanceof UndoManager, + ); + } + }) as never); + editor.registerUpdateListener( + ({ + prevEditorState, + editorState, + dirtyElements, + dirtyLeaves, + normalizedNodes, + tags, + }) => { + if (!tags.has(SKIP_COLLAB_TAG)) { + syncLexicalUpdateToYjsV2__EXPERIMENTAL( + binding, + provider, + prevEditorState, + editorState, + dirtyElements, + dirtyLeaves, + normalizedNodes, + tags, + ); + } + }, + ); + // After a remote change, pad the short rows this peer created. + editor.registerUpdateListener(({tags}) => { + if (!tags.has(COLLABORATION_TAG)) { + return; + } + editor.update( + () => { + for (const table of $getRoot().getChildren().filter($isTableNode)) { + const rows = table.getChildren(); + const width = Math.max( + ...rows.map(row => + $isElementNode(row) ? row.getChildrenSize() : 0, + ), + ); + for (const row of rows) { + if (!$isTableRowNode(row) || row.getChildrenSize() >= width) { + continue; + } + const item = binding.mapping.getSharedType(row)?._item; + if (!item || item.id.client !== this.doc.clientID) { + continue; + } + for (let i = row.getChildrenSize(); i < width; i++) { + row.append( + $createTableCellNode().append($createParagraphNode()), + ); + } + } + } + }, + {discrete: true}, + ); + }); + this.doc.on('update', (update: Uint8Array, origin: unknown) => { + if (origin === REMOTE) { + return; + } + for (const peer of this.peers) { + if (peer !== this) { + this.network.push({from: this, to: peer, update}); + } + } + }); + } + + update(fn: () => void) { + this.editor.update(fn, {discrete: true}); + } + + apply(update: Uint8Array) { + applyUpdate(this.doc, update, REMOTE); + this.editor.read(() => {}); + } + + cellsPerRow(): string { + return this.editor.read(() => { + const table = $getRoot().getChildren().find($isTableNode); + return table === undefined + ? '' + : table + .getChildren() + .map(row => + $isElementNode(row) + ? row.getChildren().filter($isTableCellNode).length + : 0, + ) + .join(','); + }); + } +} + +function $selectCell(row: number, col: number) { + const table = $getRoot().getChildren().find($isTableNode)!; + const r = table.getChildAtIndex(row); + const cell = $isElementNode(r) ? r.getChildAtIndex(col) : null; + if (!$isElementNode(cell)) { + throw new Error('no cell'); + } + cell.selectStart(); +} + +function mulberry32(seed: number) { + let a = seed; + return () => { + a |= 0; + a = (a + 0x6d2b79f5) | 0; + let t = Math.imul(a ^ (a >>> 15), 1 | a); + t ^= t + Math.imul(t ^ (t >>> 7), 61 | t); + return ((t ^ (t >>> 14)) >>> 0) / 4294967296; + }; +} + +/** + * A inserts a column while B (and C) insert a row; the messages are then + * delivered per channel in order, the channels in an order `interleaving` + * draws, until none is left (at most 500). + */ +function run(interleaving: number, count: 2 | 3) { + const network: Message[] = []; + const peers: Peer[] = []; + const all = ['A', 'B', 'C'] + .slice(0, count) + .map(name => new Peer(name, network, peers)); + peers.push(...all); + const [a, b, c] = all; + a.update(() => { + $getRoot() + .clear() + .append($createTableNodeWithDimensions(2, 2, false)); + }); + const seed = encodeStateAsUpdate(a.doc); + for (const peer of all.slice(1)) { + peer.apply(seed); + } + network.length = 0; + a.update(() => { + $selectCell(0, 1); + $insertTableColumnAtSelection(true); + }); + b.update(() => { + $selectCell(1, 0); + $insertTableRowAtSelection(true); + }); + if (c !== undefined) { + c.update(() => { + $selectCell(0, 0); + $insertTableRowAtSelection(true); + }); + } + const random = mulberry32(interleaving); + let delivered = 0; + while (network.length > 0 && delivered < 500) { + const pick = + network[interleaving === 0 ? 0 : Math.floor(random() * network.length)]; + const i = network.findIndex(m => m.to === pick.to && m.from === pick.from); + const [{to, update}] = network.splice(i, 1); + to.apply(update); + delivered++; + } + return {left: network.length, shapes: peers.map(p => p.cellsPerRow())}; +} + +const INTERLEAVINGS = Array.from({length: 40}, (_, i) => i); + +describe('v2 peers that pad the table rows they created after remote changes', () => { + test.each(INTERLEAVINGS)( + 'a column insert concurrent with two row inserts settles with 3 cells in every row (interleaving %i)', + interleaving => { + expect(run(interleaving, 3)).toEqual({ + left: 0, + shapes: ['3,3,3,3', '3,3,3,3', '3,3,3,3'], + }); + }, + ); + + test.each(INTERLEAVINGS)( + 'two peers: a column insert concurrent with a row insert settles with 3 cells in every row (interleaving %i)', + interleaving => { + expect(run(interleaving, 2)).toEqual({ + left: 0, + shapes: ['3,3,3', '3,3,3'], + }); + }, + ); +}); From fae62ccbedb77dbce0930507a0975b0fd70dc2d2 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 11:25:12 -0400 Subject: [PATCH 3/6] [lexical-table] Bug Fix: Pad again only the rows an edit removed cells from, so an app's own row repair does not ping-pong MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Follow-up to the previous commit (pad a table's rows again when an edit removes cells from some of them). That commit marked the table dirty from the row transform so the table's transform would pad, and padded only the rows recorded as changed when the table's rows were the same. Two clients that run an owner-gated row repair (the app pads, in an update after each remote change, the short rows the client itself created, counting cells) still kept re-padding each other: the rows the app's repair grew were recorded as changed and padded up to the table's grid width (spans counted), so the other client's app saw its row short again, and so on (17 messages still queued after 400 deliveries, on v1 and on v2 `@lexical/yjs`, with "One or more update listeners are endlessly enqueueing more updates"). The row transform now pads its row itself, and only a row that lost cells in the update (fewer children than in the previous editor state): a row that gained cells (a column insert, an app's own padding, which may count spans another way) is left as its author made it, and a row the update added belongs to a table it changed, which the table's transform pads as before. The table is no longer marked dirty from the row transform, and `$tableTransform` and `$padTableRows` are as on `main` again (the `$shouldPad` parameter of the previous commit is gone). The table's grid width and each row's covered cells are computed once per update and table (`$padTableRow`), kept for rows it pads and computed again when the table's rows change, when a row's child count differs from what was computed, or after `$padTableRows` pads: a column insert over 800 rows stays about 50 ms. ## Test plan `packages/lexical-yjs/src/__tests__/unit/TableRepairSettles.test.ts` (new): three v1 and three v2 `@lexical/yjs` clients with an owner-gated row repair, 12 core edits (list type and start, a paragraph, table row and column inserts and deletes, a deleted character, bold) with a few messages delivered in between, then every message delivered, up to 400: the clients go quiet; controls without the row repair. ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-yjs/src/__tests__/unit/TableRepairSettles.test.ts × yjs-v1: clients with an owner-gated row repair go quiet after concurrent edits ✓ yjs-v1, control: the same edits without the row repair go quiet × yjs-v2: clients with an owner-gated row repair go quiet after concurrent edits ✓ yjs-v2, control: the same edits without the row repair go quiet - "left": 0, + "Error: One or more update listeners are endlessly enqueueing more updates", + "left": 17, ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-yjs/src/__tests__/unit/TableRepairSettles.test.ts Tests 4 passed (4) $ pnpm exec vitest run --project unit packages/lexical-table packages/lexical-yjs packages/lexical-react packages/lexical-y/src/__tests__/unit/TableRowRepairCollab.test.ts packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts Test Files 82 passed (82) Tests 1601 passed | 4 expected fail (1605) ``` The paste-across-cells cases of the previous commit (changed rows only, added row, both orders) pass. The randomized catalog's table cells with an owner-gated row repair (200 seeds per binding) fail on the same three seeds as this PR's base, and no others. `pnpm run ci-check` and `pnpm run tsc-test` pass. --- .changeset/table-pad-rows-after-row-change.md | 2 +- .../lexical-table/flow/LexicalTable.js.flow | 5 +- .../src/LexicalTablePluginHelpers.ts | 73 +-- .../lexical-table/src/LexicalTableUtils.ts | 124 ++++- .../__tests__/unit/TableRepairSettles.test.ts | 523 ++++++++++++++++++ 5 files changed, 649 insertions(+), 78 deletions(-) create mode 100644 packages/lexical-yjs/src/__tests__/unit/TableRepairSettles.test.ts diff --git a/.changeset/table-pad-rows-after-row-change.md b/.changeset/table-pad-rows-after-row-change.md index c61aa331a24..313a8b617e2 100644 --- a/.changeset/table-pad-rows-after-row-change.md +++ b/.changeset/table-pad-rows-after-row-change.md @@ -2,6 +2,6 @@ '@lexical/table': patch --- -[fix] Pad the rows of a table again when an edit removes cells from some of its rows, such as a rich paste over a selection across cells that hold lists, so the table keeps a full grid. Only the rows whose cells changed are padded: short rows the edit did not touch (a table ragged by collaborators' concurrent row and column inserts) are left to their authors +[fix] Pad the rows of a table again when an edit removes cells from some of its rows, such as a rich paste over a selection across cells that hold lists, so the table keeps a full grid. Only the rows that lost cells in the edit are padded: rows that gained cells (a column insert, an app's own row padding) and short rows the edit did not touch (a table ragged by collaborators' concurrent row and column inserts) are left to their authors Verify: select from the middle of one cell to the middle of a cell in the next row, in a table whose cells hold lists, paste HTML that holds a list, and check that every row keeps as many cells as the table has columns (the padding cells are appended at the end of the rows that lost cells, as before). diff --git a/packages/lexical-table/flow/LexicalTable.js.flow b/packages/lexical-table/flow/LexicalTable.js.flow index a9009dbf644..83cd2a9d3c4 100644 --- a/packages/lexical-table/flow/LexicalTable.js.flow +++ b/packages/lexical-table/flow/LexicalTable.js.flow @@ -488,7 +488,4 @@ declare export function $moveTableRow( declare export function $mergeCells(cellNodes: TableCellNode[]): TableCellNode | null; /** @experimental */ -declare export function $padTableRows( - tableNode: TableNode, - $shouldPad?: (row: TableRowNode) => boolean, -): boolean; +declare export function $padTableRows(tableNode: TableNode): boolean; diff --git a/packages/lexical-table/src/LexicalTablePluginHelpers.ts b/packages/lexical-table/src/LexicalTablePluginHelpers.ts index 73258335bc0..99d19807648 100644 --- a/packages/lexical-table/src/LexicalTablePluginHelpers.ts +++ b/packages/lexical-table/src/LexicalTablePluginHelpers.ts @@ -19,7 +19,6 @@ import { import { $createParagraphNode, $findMatchingParent, - $getEditor, $getNearestNodeFromDOMNode, $getPreviousSelection, $getRoot, @@ -34,7 +33,6 @@ import { COMMAND_PRIORITY_HIGH, COMMAND_PRIORITY_LOW, type CommandPayloadType, - type EditorState, type ElementNode, isDOMNode, type LexicalEditor, @@ -77,6 +75,7 @@ import { $createTableNodeWithDimensions, $getNodeTriplet, $insertTableIntoGrid, + $padTableRow, $padTableRows, } from './LexicalTableUtils'; @@ -119,13 +118,6 @@ function $tableCellTransform(node: TableCellNode) { } } -/** - * Per update (its pending editor state), by table, the rows whose cells the - * update changed (see $tableRowTransform): what the table's transform pads - * when the update did not change the table's rows (see $tableTransform). - */ -const rowsChanged = new WeakMap>>(); - function $tableRowTransform(node: TableRowNode) { const table = node.getParent(); if (!$isTableNode(table)) { @@ -135,47 +127,14 @@ function $tableRowTransform(node: TableRowNode) { } else { $unwrapAndFilterDescendants(node, $isTableCellNode); // A row whose cells changed (a delete across cells removes some) can be - // shorter than the table. The table's transform pads it, and Lexical - // runs that only on a table marked dirty itself, which a change to one - // of its rows does not do. - const pending = $getEditor()._pendingEditorState; - if (pending !== null) { - let tables = rowsChanged.get(pending); - if (tables === undefined) { - tables = new Map(); - rowsChanged.set(pending, tables); - } - let rows = tables.get(table.getKey()); - if (rows === undefined) { - rows = new Set(); - tables.set(table.getKey(), rows); - } - rows.add(node.getKey()); - } - table.markDirty(); - } -} - -/** - * Whether the running update left the rows of `table` as they were (the - * same rows in the same order, in a table that was there before it). - */ -function $sameRows(table: TableNode): boolean { - const before = $getEditor()._editorState._nodeMap; - const previous = before.get(table.getKey()); - if (!$isTableNode(previous)) { - return false; + // shorter than the table: pad it here. Lexical runs the table's own + // transform, which pads every short row, only on a table marked dirty + // itself, which a change to one of its rows does not do. Marking the + // table instead padded rows the update did not change (a table ragged by + // collaborators' concurrent row and column inserts, which each author + // pads), and ran the update's transforms again. + $padTableRow(table, node); } - const keys = table.getChildrenKeys(); - let key = previous.__first; - for (const current of keys) { - if (key !== current) { - return false; - } - const row = before.get(key); - key = row === undefined ? null : row.__next; - } - return key === null; } function $tableTransform(node: TableNode) { @@ -183,21 +142,7 @@ function $tableTransform(node: TableNode) { // TODO: Future support of tbody/thead/tfoot/caption may change this $unwrapAndFilterDescendants(node, $isTableRowNode); - // A table whose rows the update changed (a row added, removed or moved) - // gets all its rows padded. One whose rows are the ones it had gets the - // rows whose cells changed padded, and no other: the other short rows (a - // table ragged by collaborators' concurrent row and column inserts) are - // their authors' to pad, and a client padding them at the same time as - // their authors duplicates cells, which makes the other rows short in - // turn. - const pending = $getEditor()._pendingEditorState; - const changed = pending === null ? undefined : rowsChanged.get(pending); - const rows = changed === undefined ? undefined : changed.get(node.getKey()); - if (rows !== undefined && $sameRows(node)) { - $padTableRows(node, row => rows.has(row.getKey())); - } else { - $padTableRows(node); - } + $padTableRows(node); const colWidths = node.getColWidths(); const columnCount = node.getColumnCount(); if (colWidths && colWidths.length !== columnCount) { diff --git a/packages/lexical-table/src/LexicalTableUtils.ts b/packages/lexical-table/src/LexicalTableUtils.ts index 5c3c2543092..2c743ecb02d 100644 --- a/packages/lexical-table/src/LexicalTableUtils.ts +++ b/packages/lexical-table/src/LexicalTableUtils.ts @@ -15,10 +15,12 @@ import { $createParagraphNode, $createTextNode, $findMatchingParent, + $getEditor, $getSelection, $isParagraphNode, $isRangeSelection, $isTextNode, + type EditorState, type ElementNode, type LexicalNode, type NodeKey, @@ -1182,16 +1184,112 @@ export function $computeTableMapSkipCellCheck( * a column) are merged, since remote updates are applied with transforms * skipped. * - * `$shouldPad` limits the padding to the rows it accepts (all by default): - * the table transform pads only the rows an update changed when nothing - * else about the table changed. - * * @experimental */ -export function $padTableRows( - tableNode: TableNode, - $shouldPad: (row: TableRowNode) => boolean = () => true, -): boolean { +/** + * Per update (its pending editor state), by table, the grid width and the + * grid positions each row covers, computed once (see $padTableRow): a + * column insert changes every row, and computing the map for each would be + * quadratic. Rows $padTableRow pads are updated in place; a change to the + * table's rows (their number, first or last) computes it again. + */ +const rowWidths = new WeakMap< + EditorState, + Map< + NodeKey, + { + size: number; + first: NodeKey | null; + last: NodeKey | null; + width: number; + lengths: Map; + } + > +>(); + +/** + * Pad `row`, a row of `tableNode`, to the table's width (that of its widest + * row, spans included), as {@link $padTableRows} pads each row, and leave + * the other rows as they are. For the row transform: a row whose cells + * changed is padded again where that left it short. Returns true if a cell + * was added. + * + * @internal + */ +export function $padTableRow(tableNode: TableNode, row: TableRowNode): boolean { + const editor = $getEditor(); + // Only a row that lost cells in this update (a delete across cells): one + // that gained some was given them by its author (a column insert, an + // app's own padding, which may count spans another way), and one the + // update added belongs to a table it changed, which the table's transform + // pads. + const previous = editor._editorState._nodeMap.get(row.getKey()); + if (!$isTableRowNode(previous) || row.getChildrenSize() >= previous.__size) { + return false; + } + const pending = editor._pendingEditorState; + let tables = pending === null ? undefined : rowWidths.get(pending); + if (tables === undefined) { + tables = new Map(); + if (pending !== null) { + rowWidths.set(pending, tables); + } + } + const table = tableNode.getLatest(); + let entry = tables.get(table.__key); + const known = + entry === undefined ? undefined : entry.lengths.get(row.getKey()); + if ( + entry === undefined || + entry.size !== table.__size || + entry.first !== table.__first || + entry.last !== table.__last || + known === undefined || + known[1] !== row.getChildrenSize() + ) { + const [gridMap] = $computeTableMapSkipCellCheck(tableNode, null, null); + const lengths = new Map(); + const rowNodes = tableNode.getChildren(); + let width = 0; + for (let i = 0; i < gridMap.length; i++) { + width = Math.max(width, gridMap[i].length); + const rowNode = rowNodes[i]; + if ($isTableRowNode(rowNode)) { + lengths.set(rowNode.getKey(), [ + gridMap[i].reduce((acc, cell) => (cell ? 1 + acc : acc), 0), + rowNode.getChildrenSize(), + ]); + } + } + entry = { + first: table.__first, + last: table.__last, + lengths, + size: table.__size, + width, + }; + tables.set(table.__key, entry); + } + const cached = entry.lengths.get(row.getKey()); + if (cached === undefined || cached[0] >= entry.width) { + return false; + } + const length = cached[0]; + // The same header bit as $padTableRows gives its padding cells. + const lastCell = row.getLastChild(); + const headerState = $isTableCellNode(lastCell) + ? lastCell.getHeaderStyles() & TableCellHeaderStates.ROW + : TableCellHeaderStates.NO_STATUS; + for (let j = length; j < entry.width; ++j) { + row.append( + $createTableCellNode(headerState).append($createParagraphNode()), + ); + } + entry.lengths.set(row.getKey(), [entry.width, row.getChildrenSize()]); + return true; +} + +export function $padTableRows(tableNode: TableNode): boolean { const [gridMap] = $computeTableMapSkipCellCheck(tableNode, null, null); const maxRowLength = gridMap.reduce( (curLength, row) => Math.max(curLength, row.length), @@ -1201,7 +1299,7 @@ export function $padTableRows( let changed = false; for (let i = 0; i < gridMap.length; ++i) { const rowNode = rowNodes[i]; - if (!$isTableRowNode(rowNode) || !$shouldPad(rowNode)) { + if (!$isTableRowNode(rowNode)) { continue; } // Count the grid positions this row covers, including those covered by a @@ -1229,6 +1327,14 @@ export function $padTableRows( } changed = true; } + if (changed) { + // The rows' lengths $padTableRow knows are stale. + const pending = $getEditor()._pendingEditorState; + const tables = pending === null ? undefined : rowWidths.get(pending); + if (tables !== undefined) { + tables.delete(tableNode.getKey()); + } + } return changed; } diff --git a/packages/lexical-yjs/src/__tests__/unit/TableRepairSettles.test.ts b/packages/lexical-yjs/src/__tests__/unit/TableRepairSettles.test.ts new file mode 100644 index 00000000000..420e7d76dfd --- /dev/null +++ b/packages/lexical-yjs/src/__tests__/unit/TableRepairSettles.test.ts @@ -0,0 +1,523 @@ +/** + * 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 clients (v1 and v2, wired like the React collaboration +// plugin) whose app pads the short table rows the client itself created +// after every remote change (an owner-gated row repair; the owner of a row +// is the client that created its shared type). They make 12 edits from the +// core packages (list type and start, a paragraph, table row and column +// inserts and deletes, a deleted character, bold) with a few messages +// delivered in between, then every message is delivered, oldest first, up +// to 400 deliveries. The clients should go quiet. When a change to one row +// padded the whole table (or that row up to the table's grid width, spans +// counted, where the app counts cells), each client padded again the row +// the other's repair had just padded, and they kept sending each other row +// repairs (17 messages still queued after 400 deliveries, on v1 and on v2); +// the same script without the row repair goes quiet. + +import {buildEditorFromExtensions} from '@lexical/extension'; +import {$createLinkNode, LinkNode} from '@lexical/link'; +import { + $createListItemNode, + $createListNode, + $isListNode, + ListExtension, + type ListNode, + type ListType, +} from '@lexical/list'; +import {$createMarkNode, MarkNode} from '@lexical/mark'; +import { + $createHeadingNode, + $createQuoteNode, + HeadingNode, + QuoteNode, +} from '@lexical/rich-text'; +import { + $createTableCellNode, + $createTableNode, + $createTableRowNode, + $deleteTableColumnAtSelection, + $deleteTableRowAtSelection, + $insertTableColumnAtSelection, + $insertTableRowAtSelection, + $isTableCellNode, + $isTableNode, + $isTableRowNode, + TableCellHeaderStates, + type TableCellNode, + TableExtension, +} from '@lexical/table'; +import { + createBinding, + createBindingV2__EXPERIMENTAL, + type Provider, + syncLexicalUpdateToYjs, + syncLexicalUpdateToYjsV2__EXPERIMENTAL, + syncYjsChangesToLexical, + syncYjsChangesToLexicalV2__EXPERIMENTAL, +} from '@lexical/yjs'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $getSelection, + $isElementNode, + $isRangeSelection, + $isTextNode, + $setSelection, + COLLABORATION_TAG, + type LexicalEditor, + type LexicalNode, + SKIP_COLLAB_TAG, + type TextNode, +} from 'lexical'; +import {expect, test} from 'vitest'; +import {applyUpdate, Doc, type Item, UndoManager, type XmlElement} from 'yjs'; + +type Kind = 'yjs-v1' | 'yjs-v2'; +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; + readonly errors: string[] = []; + /** The shared type of a row, to tell which rows this client created. */ + private readonly sharedTypeOf: (row: LexicalNode) => Item | null; + + constructor( + readonly name: string, + kind: Kind, + clientID: number, + queue: Message[], + clients: Client[], + repair: boolean, + ) { + this.doc.clientID = clientID; + const editor = buildEditorFromExtensions({ + $initialEditorState: null, + dependencies: [ListExtension, TableExtension], + name: `[repair-${name}]`, + nodes: [HeadingNode, QuoteNode, LinkNode, MarkNode], + onError: e => { + throw e; + }, + }); + this.editor = editor; + const docMap = new Map([['main', this.doc]]); + if (kind === 'yjs-v1') { + const binding = createBinding(editor, provider, 'main', this.doc, docMap); + 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, + ); + } + }); + this.sharedTypeOf = row => + ( + binding.collabNodeMap.get(row.getKey()) as unknown as + | {_xmlText?: {_item: Item | null}} + | undefined + )?._xmlText?._item ?? null; + } else { + const binding = createBindingV2__EXPERIMENTAL( + editor, + 'main', + this.doc, + docMap, + ); + (binding.root as XmlElement).observeDeep((( + events: never, + transaction: {origin: unknown}, + ) => { + if (transaction.origin !== binding) { + syncYjsChangesToLexicalV2__EXPERIMENTAL( + binding, + provider, + events, + transaction as never, + transaction.origin instanceof UndoManager, + ); + } + }) as never); + editor.registerUpdateListener(p => { + if (!p.tags.has(SKIP_COLLAB_TAG)) { + syncLexicalUpdateToYjsV2__EXPERIMENTAL( + binding, + provider, + p.prevEditorState, + p.editorState, + p.dirtyElements, + p.dirtyLeaves, + p.normalizedNodes, + p.tags, + ); + } + }); + editor.read(() => {}); + this.sharedTypeOf = row => + ( + binding.mapping.getSharedType(row as never) as unknown as + | {_item: Item | null} + | undefined + )?._item ?? null; + } + if (repair) { + // After every remote change, pad the short rows this client created. + editor.registerUpdateListener(({tags}) => { + if (tags.has(COLLABORATION_TAG)) { + editor.update(() => this.$padOwnRows(), {discrete: true}); + } + }); + } + 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}); + } + } + } + }); + } + + private $padOwnRows() { + const walk = (node: LexicalNode) => { + if ($isTableNode(node)) { + const rows = node.getChildren().filter($isTableRowNode); + const width = Math.max(0, ...rows.map(r => r.getChildrenSize())); + for (const row of rows) { + const item = this.sharedTypeOf(row); + if (item !== null && item.id.client === this.doc.clientID) { + for (let j = row.getChildrenSize(); j < width; j++) { + row.append($createTableCellNode().append($createParagraphNode())); + } + } + } + } + if ($isElementNode(node)) { + node.getChildren().forEach(walk); + } + }; + walk($getRoot()); + } + + flush() { + for (let i = 0; i < 20 && this.editor._pendingEditorState !== null; i++) { + this.editor.read(() => {}); + } + } + + private guard(fn: () => void) { + try { + fn(); + this.flush(); + } catch (error) { + this.errors.push(String(error).split('.')[0]); + } + } + + update(fn: () => void) { + this.guard(() => this.editor.update(fn, {discrete: true})); + } + + apply(update: Uint8Array) { + this.guard(() => applyUpdate(this.doc, update, REMOTE)); + } +} + +const $t = (text: string) => $createTextNode(text); +const $p = (...children: LexicalNode[]) => + $createParagraphNode().append(...children); + +function $list(type: ListType, start = 1, nested = false): ListNode { + const item = (text: string) => + $createListItemNode( + type === 'check' ? text.length % 2 === 0 : undefined, + ).append($t(text)); + const list = $createListNode(type, start).append( + item(`first ${type}`), + item(`second ${type}`), + item(`third ${type}`), + ); + if (nested) { + list.splice(2, 0, [ + $createListItemNode().append( + $createListNode(type, 1).append(item('inner one'), item('inner two')), + ), + ]); + } + return list; +} + +function $cell(text: string, header = false, colSpan = 1) { + return $createTableCellNode( + header ? TableCellHeaderStates.ROW : TableCellHeaderStates.NO_STATUS, + colSpan, + ).append($p($t(text))); +} + +/** Two tables (one with merged cells), lists, and a few text blocks. */ +function $build() { + const span = $cell('spans two rows'); + span.setRowSpan(2); + $getRoot() + .clear() + .append( + $createTableNode().append( + $createTableRowNode().append($cell('h1', true), $cell('h2', true)), + $createTableRowNode().append( + $cell('a'), + $createTableCellNode().append($list('number', 3), $p($t('after'))), + ), + ), + $createTableNode().append( + $createTableRowNode().append( + $cell('wide header', true, 2), + $cell('h3'), + ), + $createTableRowNode().append(span, $cell('b'), $cell('c')), + $createTableRowNode().append( + $cell('d'), + $createTableCellNode().append($list('bullet')), + ), + ), + $list('number', 5), + $list('check', 1, true), + $p( + $t('bold start').setFormat('bold').setStyle('color: red'), + $t(' plain'), + ), + $p($t('before plain after')), + $p( + $t('before '), + $createMarkNode(['comment-1']).append($t('commented')), + $t(' after'), + ), + $p( + $t('before '), + $createLinkNode('https://example.com').append($t('a link')), + $t(' after'), + ), + $createQuoteNode().append($t('quoted')), + $createHeadingNode('h2').append($t('Heading')), + ); +} + +/** Every node in document order that passes `test`. */ +function $all(keep: (n: LexicalNode) => boolean): T[] { + const out: T[] = []; + const walk = (node: LexicalNode) => { + if (keep(node)) { + out.push(node as T); + } + if ($isElementNode(node)) { + node.getChildren().forEach(walk); + } + }; + walk($getRoot()); + return out; +} + +const $texts = () => + $all(n => $isTextNode(n) && (n as TextNode).isSimpleText()); + +/** The `index`th simple text node; checks it holds `content`. */ +function $text(index: number, content: string): TextNode { + const node = $texts()[index]; + expect(node.getTextContent()).toBe(content); + return node; +} + +function $select(node: TextNode, from: number, to = from) { + const s = $createRangeSelection(); + s.anchor.set(node.getKey(), from, 'text'); + s.focus.set(node.getKey(), to, 'text'); + $setSelection(s); +} + +/** Selects the start of the `index`th table cell, as a click there would. */ +function $selectCell(index: number) { + const cell = $all($isTableCellNode)[index]; + const text = cell.getAllTextNodes().find(t => t.isSimpleText()); + if (text !== undefined) { + $select(text, 0); + } else { + cell.selectStart(); + } +} + +type Step = + | {deliver: [string, string]} + | ({peer: string} & ( + | {op: 'listType'; list: number; type: ListType} + | {op: 'listStart'; list: number; start: number} + | {op: 'insertParagraph'; text: number; content: string} + | {op: 'deleteChar'; text: number; content: string; at: number} + | {op: 'bold'; text: number; content: string; from: number; to: number} + | { + op: 'tableInsertRow' | 'tableInsertColumn'; + cell: number; + after: boolean; + } + | {op: 'tableDeleteRow' | 'tableDeleteColumn'; cell: number} + )); + +const SCRIPT: Step[] = [ + {list: 2, op: 'listType', peer: 'B', type: 'check'}, + {content: 'c', op: 'insertParagraph', peer: 'B', text: 11}, + {after: false, cell: 1, op: 'tableInsertRow', peer: 'A'}, + {after: false, cell: 10, op: 'tableInsertRow', peer: 'B'}, + {list: 0, op: 'listStart', peer: 'C', start: 2}, + {deliver: ['B', 'C']}, + {deliver: ['B', 'A']}, + {at: 1, content: 'quoted', op: 'deleteChar', peer: 'B', text: 34}, + {content: 'first number', from: 10, op: 'bold', peer: 'B', text: 17, to: 11}, + {cell: 11, op: 'tableDeleteColumn', peer: 'B'}, + {after: false, cell: 6, op: 'tableInsertColumn', peer: 'C'}, + {cell: 3, op: 'tableDeleteColumn', peer: 'A'}, + {deliver: ['B', 'A']}, + {deliver: ['A', 'B']}, + {after: false, cell: 8, op: 'tableInsertColumn', peer: 'A'}, + {deliver: ['C', 'B']}, + {at: 0, content: 'second bullet', op: 'deleteChar', peer: 'B', text: 13}, + {deliver: ['C', 'A']}, + {deliver: ['B', 'C']}, +]; + +function run(kind: Kind, repair: boolean) { + const queue: Message[] = []; + const clients: Client[] = []; + for (const [i, name] of ['A', 'B', 'C'].entries()) { + clients.push(new Client(name, kind, 1000 + i, queue, clients, repair)); + } + const byName = (name: string) => clients.find(c => c.name === name)!; + const deliverOldest = (from: string, to: string) => { + const i = queue.findIndex(m => m.from.name === from && m.to.name === to); + const [m] = queue.splice(i, 1); + m.to.apply(m.update); + }; + const drain = (cap: number) => { + let delivered = 0; + while (queue.length > 0 && delivered < cap) { + const m = queue.shift()!; + m.to.apply(m.update); + delivered++; + } + return delivered; + }; + byName('A').update($build); + drain(400); + expect(queue.length).toBe(0); + for (const step of SCRIPT) { + if ('deliver' in step) { + deliverOldest(...step.deliver); + continue; + } + byName(step.peer).update(() => { + switch (step.op) { + case 'listType': + $all($isListNode)[step.list].setListType(step.type); + break; + case 'listStart': + $all($isListNode)[step.list].setStart(step.start); + break; + case 'insertParagraph': + $text(step.text, step.content) + .getTopLevelElementOrThrow() + .insertAfter($p($t('new line'))); + break; + case 'deleteChar': + $text(step.text, step.content).spliceText(step.at, 1, ''); + break; + case 'bold': { + $select($text(step.text, step.content), step.from, step.to); + const selection = $getSelection(); + if ($isRangeSelection(selection)) { + selection.formatText('bold'); + } + break; + } + case 'tableInsertRow': + $selectCell(step.cell); + $insertTableRowAtSelection(step.after); + break; + case 'tableInsertColumn': + $selectCell(step.cell); + $insertTableColumnAtSelection(step.after); + break; + case 'tableDeleteRow': + $selectCell(step.cell); + $deleteTableRowAtSelection(); + break; + case 'tableDeleteColumn': + $selectCell(step.cell); + $deleteTableColumnAtSelection(); + break; + } + }); + } + const delivered = drain(400); + return { + delivered, + errors: [...new Set(clients.flatMap(c => c.errors))], + left: queue.length, + }; +} + +for (const kind of ['yjs-v1', 'yjs-v2'] as const) { + test(`${kind}: clients with an owner-gated row repair go quiet after concurrent edits`, () => { + // #9539 72ecb3ca, v1 and v2: messages still queued after 400 + // deliveries, and "update listeners are endlessly enqueueing more + // updates" thrown while applying them. + const r = run(kind, true); + expect({errors: r.errors, left: r.left}).toEqual({errors: [], left: 0}); + }); + test(`${kind}, control: the same edits without the row repair go quiet`, () => { + const r = run(kind, false); + expect({errors: r.errors, left: r.left}).toEqual({errors: [], left: 0}); + }); +} From e78a3621bb659810ac9444678338761c428a9e3e Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 12:14:03 -0400 Subject: [PATCH 4/6] [lexical-table] Bug Fix: Pad the rows that lost cells to the width the table has at the end of the update ## Description Follow-up to the previous commit (pad again only the rows an edit removed cells from). It padded such a row from its own transform, to a grid width computed once per update and table. A row that grew later in the same update (a transform appending cells to it, or raising a cell's span) left that width stale: three rows of three cells, the first two losing a cell, then the third widened to five, ended `[3, 3, 5]` instead of `[5, 5, 5]`. Reported in review. The table's transform pads again, as two commits ago, but now only the rows that lost cells in the update (fewer children than in the previous editor state), when the update did not change the table's rows; rows that gained cells (a column insert, an app's own row padding) are still left as their authors made them, so the owner-gated row repairs of `TableRepairSettles.test.ts` still go quiet. A row that changes after rows of its table lost cells in the update, and a cell that changes then (its span raised), mark the table again, so its transform pads those rows to the width the table has then. `$padTableRows` takes the optional `$shouldPad` of two commits ago again. Each table transform computes the width once: a column insert over 800 rows stays about 55 ms, and removing a cell from each of 800 rows about 40 ms. ## Test plan `packages/lexical-table/src/__tests__/unit/TableRowPadLater.test.ts` (new): the review's two cases (the later row appends two cells; a cell of it gets span 3). ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-table/src/__tests__/unit/TableRowPadLater.test.ts Tests 2 failed (2) AssertionError: expected [ 3, 3, 5 ] to deeply equal [ 5, 5, 5 ] ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-table/src/__tests__/unit/TableRowPadLater.test.ts Tests 2 passed (2) $ pnpm exec vitest run --project unit packages/lexical-table packages/lexical-yjs packages/lexical-react packages/lexical-y/src/__tests__/unit/TableRowRepairCollab.test.ts packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts Test Files 82 passed (82) Tests 883 passed | 4 expected fail (887) ``` `TableRepairSettles.test.ts` (4) and the paste-across-cells cases pass; the catalog's `C6Tables` cells pass (720). `pnpm run ci-check` passes. --- .../lexical-table/flow/LexicalTable.js.flow | 5 +- .../src/LexicalTablePluginHelpers.ts | 108 +++++++++++++-- .../lexical-table/src/LexicalTableUtils.ts | 124 ++---------------- .../__tests__/unit/TableRowPadLater.test.ts | 105 +++++++++++++++ 4 files changed, 214 insertions(+), 128 deletions(-) create mode 100644 packages/lexical-table/src/__tests__/unit/TableRowPadLater.test.ts diff --git a/packages/lexical-table/flow/LexicalTable.js.flow b/packages/lexical-table/flow/LexicalTable.js.flow index 83cd2a9d3c4..a9009dbf644 100644 --- a/packages/lexical-table/flow/LexicalTable.js.flow +++ b/packages/lexical-table/flow/LexicalTable.js.flow @@ -488,4 +488,7 @@ declare export function $moveTableRow( declare export function $mergeCells(cellNodes: TableCellNode[]): TableCellNode | null; /** @experimental */ -declare export function $padTableRows(tableNode: TableNode): boolean; +declare export function $padTableRows( + tableNode: TableNode, + $shouldPad?: (row: TableRowNode) => boolean, +): boolean; diff --git a/packages/lexical-table/src/LexicalTablePluginHelpers.ts b/packages/lexical-table/src/LexicalTablePluginHelpers.ts index 99d19807648..188c8fdeba2 100644 --- a/packages/lexical-table/src/LexicalTablePluginHelpers.ts +++ b/packages/lexical-table/src/LexicalTablePluginHelpers.ts @@ -19,6 +19,7 @@ import { import { $createParagraphNode, $findMatchingParent, + $getEditor, $getNearestNodeFromDOMNode, $getPreviousSelection, $getRoot, @@ -33,6 +34,7 @@ import { COMMAND_PRIORITY_HIGH, COMMAND_PRIORITY_LOW, type CommandPayloadType, + type EditorState, type ElementNode, isDOMNode, type LexicalEditor, @@ -75,7 +77,6 @@ import { $createTableNodeWithDimensions, $getNodeTriplet, $insertTableIntoGrid, - $padTableRow, $padTableRows, } from './LexicalTableUtils'; @@ -108,14 +109,38 @@ function $insertTable( return true; } +/** + * Per update (its pending editor state), by table, the rows that lost cells + * in the update (see $tableRowTransform): what the table's transform pads + * when the update did not change the table's rows (see $tableTransform). + */ +const rowsLost = new WeakMap>>(); + +/** The rows of `table` that lost cells in the running update, if any. */ +function $rowsLost(table: TableNode): Set | undefined { + const pending = $getEditor()._pendingEditorState; + const tables = pending === null ? undefined : rowsLost.get(pending); + return tables === undefined ? undefined : tables.get(table.getKey()); +} + function $tableCellTransform(node: TableCellNode) { - if (!$isTableRowNode(node.getParent())) { + const row = node.getParent(); + if (!$isTableRowNode(row)) { // TableCellNode must be a child of TableRowNode. node.remove(); - } else if (node.isEmpty()) { + return; + } + if (node.isEmpty()) { // TableCellNode should never be empty node.append($createParagraphNode()); } + // A cell changed (its span raised) after rows of its table lost cells in + // this update: the table's width may have changed, so the table's + // transform pads those rows again. + const table = row.getParent(); + if ($isTableNode(table) && $rowsLost(table) !== undefined) { + table.markDirty(); + } } function $tableRowTransform(node: TableRowNode) { @@ -126,23 +151,82 @@ function $tableRowTransform(node: TableRowNode) { node.remove(); } else { $unwrapAndFilterDescendants(node, $isTableCellNode); - // A row whose cells changed (a delete across cells removes some) can be - // shorter than the table: pad it here. Lexical runs the table's own - // transform, which pads every short row, only on a table marked dirty - // itself, which a change to one of its rows does not do. Marking the - // table instead padded rows the update did not change (a table ragged by - // collaborators' concurrent row and column inserts, which each author - // pads), and ran the update's transforms again. - $padTableRow(table, node); + // A row that lost cells (a delete across cells removes some) can be + // shorter than the table. The table's transform pads it, and Lexical + // runs that only on a table marked dirty itself, which a change to one + // of its rows does not do. A row that gained cells (a column insert, an + // app's own padding of its rows, which may count cells where the table + // counts grid positions) is left as its author made it: padding it to + // the table's width had two clients that each pad the rows they created + // pad each other's rows again without end. A row that changed after + // rows lost cells in this update (it grew) marks the table again, so + // those rows are padded to the width the table has now. + const editor = $getEditor(); + const pending = editor._pendingEditorState; + const previous = editor._editorState._nodeMap.get(node.getKey()); + if ( + pending !== null && + $isTableRowNode(previous) && + node.getChildrenSize() < previous.__size + ) { + let tables = rowsLost.get(pending); + if (tables === undefined) { + tables = new Map(); + rowsLost.set(pending, tables); + } + let rows = tables.get(table.getKey()); + if (rows === undefined) { + rows = new Set(); + tables.set(table.getKey(), rows); + } + rows.add(node.getKey()); + table.markDirty(); + } else if ($rowsLost(table) !== undefined) { + table.markDirty(); + } } } +/** + * Whether the running update left the rows of `table` as they were (the + * same rows in the same order, in a table that was there before it). + */ +function $sameRows(table: TableNode): boolean { + const before = $getEditor()._editorState._nodeMap; + const previous = before.get(table.getKey()); + if (!$isTableNode(previous)) { + return false; + } + const keys = table.getChildrenKeys(); + let key = previous.__first; + for (const current of keys) { + if (key !== current) { + return false; + } + const row = before.get(key); + key = row === undefined ? null : row.__next; + } + return key === null; +} + function $tableTransform(node: TableNode) { // TableRowNode is the only valid child for TableNode // TODO: Future support of tbody/thead/tfoot/caption may change this $unwrapAndFilterDescendants(node, $isTableRowNode); - $padTableRows(node); + // A table whose rows the update changed (a row added, removed or moved) + // gets all its rows padded. One whose rows are the ones it had gets the + // rows that lost cells padded, and no other: the other short rows (a + // table ragged by collaborators' concurrent row and column inserts) are + // their authors' to pad, and a client padding them at the same time as + // their authors duplicates cells, which makes the other rows short in + // turn. + const rows = $rowsLost(node); + if (rows !== undefined && $sameRows(node)) { + $padTableRows(node, row => rows.has(row.getKey())); + } else { + $padTableRows(node); + } const colWidths = node.getColWidths(); const columnCount = node.getColumnCount(); if (colWidths && colWidths.length !== columnCount) { diff --git a/packages/lexical-table/src/LexicalTableUtils.ts b/packages/lexical-table/src/LexicalTableUtils.ts index 2c743ecb02d..5c3c2543092 100644 --- a/packages/lexical-table/src/LexicalTableUtils.ts +++ b/packages/lexical-table/src/LexicalTableUtils.ts @@ -15,12 +15,10 @@ import { $createParagraphNode, $createTextNode, $findMatchingParent, - $getEditor, $getSelection, $isParagraphNode, $isRangeSelection, $isTextNode, - type EditorState, type ElementNode, type LexicalNode, type NodeKey, @@ -1184,112 +1182,16 @@ export function $computeTableMapSkipCellCheck( * a column) are merged, since remote updates are applied with transforms * skipped. * - * @experimental - */ -/** - * Per update (its pending editor state), by table, the grid width and the - * grid positions each row covers, computed once (see $padTableRow): a - * column insert changes every row, and computing the map for each would be - * quadratic. Rows $padTableRow pads are updated in place; a change to the - * table's rows (their number, first or last) computes it again. - */ -const rowWidths = new WeakMap< - EditorState, - Map< - NodeKey, - { - size: number; - first: NodeKey | null; - last: NodeKey | null; - width: number; - lengths: Map; - } - > ->(); - -/** - * Pad `row`, a row of `tableNode`, to the table's width (that of its widest - * row, spans included), as {@link $padTableRows} pads each row, and leave - * the other rows as they are. For the row transform: a row whose cells - * changed is padded again where that left it short. Returns true if a cell - * was added. + * `$shouldPad` limits the padding to the rows it accepts (all by default): + * the table transform pads only the rows an update changed when nothing + * else about the table changed. * - * @internal + * @experimental */ -export function $padTableRow(tableNode: TableNode, row: TableRowNode): boolean { - const editor = $getEditor(); - // Only a row that lost cells in this update (a delete across cells): one - // that gained some was given them by its author (a column insert, an - // app's own padding, which may count spans another way), and one the - // update added belongs to a table it changed, which the table's transform - // pads. - const previous = editor._editorState._nodeMap.get(row.getKey()); - if (!$isTableRowNode(previous) || row.getChildrenSize() >= previous.__size) { - return false; - } - const pending = editor._pendingEditorState; - let tables = pending === null ? undefined : rowWidths.get(pending); - if (tables === undefined) { - tables = new Map(); - if (pending !== null) { - rowWidths.set(pending, tables); - } - } - const table = tableNode.getLatest(); - let entry = tables.get(table.__key); - const known = - entry === undefined ? undefined : entry.lengths.get(row.getKey()); - if ( - entry === undefined || - entry.size !== table.__size || - entry.first !== table.__first || - entry.last !== table.__last || - known === undefined || - known[1] !== row.getChildrenSize() - ) { - const [gridMap] = $computeTableMapSkipCellCheck(tableNode, null, null); - const lengths = new Map(); - const rowNodes = tableNode.getChildren(); - let width = 0; - for (let i = 0; i < gridMap.length; i++) { - width = Math.max(width, gridMap[i].length); - const rowNode = rowNodes[i]; - if ($isTableRowNode(rowNode)) { - lengths.set(rowNode.getKey(), [ - gridMap[i].reduce((acc, cell) => (cell ? 1 + acc : acc), 0), - rowNode.getChildrenSize(), - ]); - } - } - entry = { - first: table.__first, - last: table.__last, - lengths, - size: table.__size, - width, - }; - tables.set(table.__key, entry); - } - const cached = entry.lengths.get(row.getKey()); - if (cached === undefined || cached[0] >= entry.width) { - return false; - } - const length = cached[0]; - // The same header bit as $padTableRows gives its padding cells. - const lastCell = row.getLastChild(); - const headerState = $isTableCellNode(lastCell) - ? lastCell.getHeaderStyles() & TableCellHeaderStates.ROW - : TableCellHeaderStates.NO_STATUS; - for (let j = length; j < entry.width; ++j) { - row.append( - $createTableCellNode(headerState).append($createParagraphNode()), - ); - } - entry.lengths.set(row.getKey(), [entry.width, row.getChildrenSize()]); - return true; -} - -export function $padTableRows(tableNode: TableNode): boolean { +export function $padTableRows( + tableNode: TableNode, + $shouldPad: (row: TableRowNode) => boolean = () => true, +): boolean { const [gridMap] = $computeTableMapSkipCellCheck(tableNode, null, null); const maxRowLength = gridMap.reduce( (curLength, row) => Math.max(curLength, row.length), @@ -1299,7 +1201,7 @@ export function $padTableRows(tableNode: TableNode): boolean { let changed = false; for (let i = 0; i < gridMap.length; ++i) { const rowNode = rowNodes[i]; - if (!$isTableRowNode(rowNode)) { + if (!$isTableRowNode(rowNode) || !$shouldPad(rowNode)) { continue; } // Count the grid positions this row covers, including those covered by a @@ -1327,14 +1229,6 @@ export function $padTableRows(tableNode: TableNode): boolean { } changed = true; } - if (changed) { - // The rows' lengths $padTableRow knows are stale. - const pending = $getEditor()._pendingEditorState; - const tables = pending === null ? undefined : rowWidths.get(pending); - if (tables !== undefined) { - tables.delete(tableNode.getKey()); - } - } return changed; } diff --git a/packages/lexical-table/src/__tests__/unit/TableRowPadLater.test.ts b/packages/lexical-table/src/__tests__/unit/TableRowPadLater.test.ts new file mode 100644 index 00000000000..0ff02621551 --- /dev/null +++ b/packages/lexical-table/src/__tests__/unit/TableRowPadLater.test.ts @@ -0,0 +1,105 @@ +/** + * 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 + +// Three rows of three cells; an update removes a cell from the first two, +// and a transform that runs later in the same update widens the third row +// (it appends two cells, or raises a cell's span). The rows that lost +// cells are padded to the width the table has once the update is done: +// every row covers five grid positions. + +import {buildEditorFromExtensions} from '@lexical/extension'; +import { + $computeTableMapSkipCellCheck, + $createTableCellNode, + $createTableNode, + $createTableRowNode, + type TableCellNode, + TableExtension, + type TableNode, + TableRowNode, +} from '@lexical/table'; +import { + $createParagraphNode, + $createTextNode, + $getRoot, + defineExtension, +} from 'lexical'; +import {expect, test} from 'vitest'; + +const $cell = () => + $createTableCellNode().append( + $createParagraphNode().append($createTextNode('x')), + ); + +const $widths = () => { + const [map] = $computeTableMapSkipCellCheck( + $getRoot().getFirstChildOrThrow() as TableNode, + null, + null, + ); + return map.map(row => row.length); +}; + +for (const change of ['append', 'span'] as const) { + test(`rows that lost cells are padded to a width another row reaches later in the update (${change})`, () => { + using editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + dependencies: [TableExtension], + name: 'table-row-pad-later', + onError: e => { + throw e; + }, + }), + ); + let first = ''; + let active = false; + let changed = false; + editor.registerNodeTransform(TableRowNode, row => { + if (active && !changed && row.getKey() === first) { + changed = true; + const table = row.getParent() as TableNode; + const last = table.getLastChildOrThrow() as TableRowNode; + if (change === 'append') { + last.append($cell(), $cell()); + } else { + (last.getFirstChildOrThrow() as TableCellNode).setColSpan(3); + } + } + }); + editor.update( + () => { + const table = $createTableNode(); + for (let i = 0; i < 3; i++) { + const row = $createTableRowNode().append($cell(), $cell(), $cell()); + if (i === 0) { + first = row.getKey(); + } + table.append(row); + } + $getRoot().append(table); + }, + {discrete: true}, + ); + expect(editor.read($widths)).toEqual([3, 3, 3]); + active = true; + editor.update( + () => { + const rows = ( + $getRoot().getFirstChildOrThrow() as TableNode + ).getChildren() as TableRowNode[]; + rows[0].getLastChildOrThrow().remove(); + rows[1].getLastChildOrThrow().remove(); + }, + {discrete: true}, + ); + expect(editor.read($widths)).toEqual([5, 5, 5]); + }); +} From 27e1552f6acfdbf70c5df58c757f35d4cfd8ae5f Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 13:46:30 -0400 Subject: [PATCH 5/6] [lexical-table] Bug Fix: Pad no short row of an existing table when the update changes only the table ## Description Follow-up to the previous commits. The table's transform padded every short row whenever the table node itself was dirtied and no row had lost cells in the update: a table ragged by collaborators' concurrent row and column inserts (widths 3, 2, 3), and an update that only sets its row striping, ended 3, 3, 3. That contradicts what this PR says, that short rows the edit did not touch are left to their authors, and it kept the owner-gated repair race for table-only edits. Reported in review. For a table whose rows are the ones it had, the transform now pads only the rows that lost cells in the update, and none when no row did. A new table, or one whose rows the update changed (a row added, removed or moved), still gets all its rows padded. ## Test plan `TableFieldLeavesRaggedRows.test.ts` (new, from the review): widths 3, 2, 3, then `setRowStriping(true)` only. ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-table/src/__tests__/unit/TableFieldLeavesRaggedRows.test.ts AssertionError: expected [ 3, 3, 3 ] to deeply equal [ 3, 2, 3 ] Tests 1 failed (1) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-table packages/lexical-yjs packages/lexical-react packages/lexical-y/src/__tests__/unit/TableRowRepairCollab.test.ts packages/lexical-y/src/__tests__/unit/TablePasteAcrossCells.test.ts Test Files 83 passed (83) Tests 884 passed | 4 expected fail (888) ``` The catalog's `C6Tables` cells pass (720). `pnpm run ci-check` passes. --- .../src/LexicalTablePluginHelpers.ts | 21 ++--- .../unit/TableFieldLeavesRaggedRows.test.ts | 80 +++++++++++++++++++ 2 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 packages/lexical-table/src/__tests__/unit/TableFieldLeavesRaggedRows.test.ts diff --git a/packages/lexical-table/src/LexicalTablePluginHelpers.ts b/packages/lexical-table/src/LexicalTablePluginHelpers.ts index 188c8fdeba2..03700e26721 100644 --- a/packages/lexical-table/src/LexicalTablePluginHelpers.ts +++ b/packages/lexical-table/src/LexicalTablePluginHelpers.ts @@ -214,18 +214,19 @@ function $tableTransform(node: TableNode) { // TODO: Future support of tbody/thead/tfoot/caption may change this $unwrapAndFilterDescendants(node, $isTableRowNode); - // A table whose rows the update changed (a row added, removed or moved) - // gets all its rows padded. One whose rows are the ones it had gets the - // rows that lost cells padded, and no other: the other short rows (a - // table ragged by collaborators' concurrent row and column inserts) are - // their authors' to pad, and a client padding them at the same time as - // their authors duplicates cells, which makes the other rows short in - // turn. + // A new table, or one whose rows the update changed (a row added, + // removed or moved), gets all its rows padded. One whose rows are the ones + // it had gets the rows that lost cells padded, and no other, also when + // the update changed only the table itself (its striping): the other + // short rows (a table ragged by collaborators' concurrent row and column + // inserts) are their authors' to pad, and a client padding them at the + // same time as their authors duplicates cells, which makes the other rows + // short in turn. const rows = $rowsLost(node); - if (rows !== undefined && $sameRows(node)) { - $padTableRows(node, row => rows.has(row.getKey())); - } else { + if (!$sameRows(node)) { $padTableRows(node); + } else if (rows !== undefined) { + $padTableRows(node, row => rows.has(row.getKey())); } const colWidths = node.getColWidths(); const columnCount = node.getColumnCount(); diff --git a/packages/lexical-table/src/__tests__/unit/TableFieldLeavesRaggedRows.test.ts b/packages/lexical-table/src/__tests__/unit/TableFieldLeavesRaggedRows.test.ts new file mode 100644 index 00000000000..92917ca2e8d --- /dev/null +++ b/packages/lexical-table/src/__tests__/unit/TableFieldLeavesRaggedRows.test.ts @@ -0,0 +1,80 @@ +/** + * 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 + +// A table whose rows are ragged (collaborators' concurrent row and column +// inserts, which each author pads) and an update that changes only the +// table itself: the short row is left to its author. + +import {buildEditorFromExtensions} from '@lexical/extension'; +import {RichTextExtension} from '@lexical/rich-text'; +import { + $createTableCellNode, + $createTableNode, + $createTableRowNode, + $isTableNode, + $isTableRowNode, + TableExtension, +} from '@lexical/table'; +import { + $createParagraphNode, + $getRoot, + configExtension, + defineExtension, +} from 'lexical'; +import {assert, expect, test} from 'vitest'; + +const $widths = () => { + const table = $getRoot().getFirstChild(); + assert($isTableNode(table)); + return table + .getChildren() + .filter($isTableRowNode) + .map(row => row.getChildrenSize()); +}; + +test('a table-only field change leaves pre-existing short rows alone', () => { + using editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + dependencies: [ + RichTextExtension, + configExtension(TableExtension, {hasHorizontalScroll: false}), + ], + name: 'review-9539-table-field', + onError: error => { + throw error; + }, + }), + ); + editor.update( + () => { + const table = $createTableNode(); + for (const width of [3, 2, 3]) { + const row = $createTableRowNode(); + for (let i = 0; i < width; i++) { + row.append($createTableCellNode().append($createParagraphNode())); + } + table.append(row); + } + $getRoot().append(table); + }, + {discrete: true, skipTransforms: true}, + ); + expect(editor.read($widths)).toEqual([3, 2, 3]); + editor.update( + () => { + const table = $getRoot().getFirstChild(); + assert($isTableNode(table)); + table.setRowStriping(true); + }, + {discrete: true}, + ); + expect(editor.read($widths)).toEqual([3, 2, 3]); +}); From 68379f85e09b1e107884f2e460a50c57acc577ac Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 13:51:20 -0400 Subject: [PATCH 6/6] Chore: Ask the review bot for a review (no file changes) ## Description This commit changes no file. The repository's review bot reviews a pull request when its branch is pushed; this branch was last pushed before the bot started reviewing the repository, so it had no review yet.