Repository navigation
[lexical-table][lexical-y] Bug Fix: Pad a table's rows again when an edit removes cells from some of them - #9539
Conversation
…edit removes cells from some of them ## 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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
|
Converted to draft: with this change, three peers on the v2 |
|
Update: not limited to apps that repair rows. With 2 to 4 plain v2 |
…ose cells changed, so peers that pad their own rows after remote changes stop padding each other's
## 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.
…s from, so an app's own row repair does not ping-pong
## 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.
…e 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.
…he 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.
## 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.
There was a problem hiding this comment.
This looks good to me. Marking the table dirty when a row loses cells fixes the #9520 paste, both in @lexical/y-bound editors and in editors without NodeStructureExtension, and the new paste tests fail on main and pass here. Everything below is an optional, non-blocking suggestion.
1. Tables that are already ragged are no longer repaired, and a cell selection across them throws (packages/lexical-table/src/LexicalTablePluginHelpers.ts:226-230, packages/lexical-table/src/LexicalTableSelection.ts:382)
When the rows are unchanged, $tableTransform now pads only the rows that lost cells. That includes the pass Lexical makes when a transform is registered over content that is already loaded. registerTablePlugin after setEditorState (what <TablePlugin /> does when it mounts under a LexicalComposer with a JSON editorState) now leaves a stored 3,2,3 table at 3,2,3; main squares it to 3,3,3. A TableSelection from the first to the last cell of that table then throws TypeError: Cannot destructure property 'cell' of 'map[i][j]' as it is undefined in getNodes(). main throws the same way right after the #9520 paste, which this PR fixes, so what remains exposed is documents that already hold short rows. Skipping undefined map[i][j] entries in getNodes() would cover these, and also the ragged tables that concurrent edits leave behind. The alternative is to keep full padding for content that was loaded before the transform was registered.
2. Changeset Verify: (.changeset/table-pad-rows-after-row-change.md:7)
CONTRIBUTING.md asks a fix that changes observable behavior to carry a Verify: paragraph, and .changeset/README.md describes this under "Behavior changes". Please also mention that a table whose rows are already short keeps them through table-only updates (striping, column widths) and when TablePlugin mounts, until a table command, a row insert, delete or move, or an edit that removes cells from those rows. Moving the second sentence of the summary line into its own paragraph would keep the summary to one line.
3. @lexical/y normalization docs (packages/lexical-website/docs/collaboration/y.md:884-889)
After an update applied with skipTransforms, the binding's author normalization marks the content dirty again. The table's rows now match the committed state, though, so a 3,2,3 table written that way stays 3,2,3; on main it becomes 3,3,3, in the stored document as well. "Is normalized by its author right after" no longer holds for tables. Either note that on the page, or make that normalization pad tables fully.
4. v2 @lexical/yjs under concurrent cross-row pastes
I ran a randomized test with three plain v2 clients making table edits with partial delivery (a paste-heavy mix, 1000 seeds). On main 205 seeds threw and 131 diverged; on this branch 298 threw and 247 diverged, all with Lexical node does not exist in active editor state from syncYjsChangesToLexicalV2__EXPERIMENTAL. v1 and @lexical/y are unchanged, and without pastes the outcomes are identical. So the padding cells a paste now writes expose a v2 binding bug that main already has. A follow-up issue for v2 would keep it visible.
Nits
packages/lexical-table/src/LexicalTableUtils.ts:1172still calls$padTableRows"the same repair the table node transform ... does". Lines 1185-1187 say the transform pads "only the rows an update changed when nothing else about the table changed", but it now pads the rows that lost cells whenever the rows are unchanged, table-only changes included.TableRowPadLater.test.ts:55andTableRepairSettles.test.ts:120useTableExtensionwith the defaulthasHorizontalScroll, so every run logs thetableScrollableWrapperwarning; the other new tests passconfigExtension(TableExtension, {hasHorizontalScroll: false}).
| if (!$sameRows(node)) { | ||
| $padTableRows(node); | ||
| } else if (rows !== undefined) { | ||
| $padTableRows(node, row => rows.has(row.getKey())); | ||
| } |
There was a problem hiding this comment.
Optional: with the rows unchanged, this pads only the rows that lost cells, which also covers the pass Lexical makes when a transform is registered over loaded content. registerTablePlugin after setEditorState (what <TablePlugin /> does when it mounts under a LexicalComposer with a JSON editorState) now leaves a stored 3,2,3 table at 3,2,3; main squares it. A TableSelection across that table then throws Cannot destructure property 'cell' of 'map[i][j]' in TableSelection.getNodes() (LexicalTableSelection.ts:382). Skipping undefined map entries there would make ragged tables safe however they arise.
|
|
||
| [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). |
There was a problem hiding this comment.
Optional: per CONTRIBUTING.md (a fix that changes observable behavior gets a Verify: paragraph), please also cover the narrowed padding. A table whose rows are already short keeps them through table-only updates and when TablePlugin mounts, until a table command, a row insert, delete or move, or an edit that removes cells from those rows.
| * `$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. |
There was a problem hiding this comment.
Nit: the transform pads the rows that lost cells, and it does so whenever the table's rows are unchanged, including when the table's own fields changed (LexicalTablePluginHelpers.ts:225-230). Line 1172's "the same repair the table node transform ... does" could say that too.
| using editor = buildEditorFromExtensions( | ||
| defineExtension({ | ||
| $initialEditorState: null, | ||
| dependencies: [TableExtension], |
There was a problem hiding this comment.
Nit: with the default hasHorizontalScroll, every run logs "TableNode: hasHorizontalScroll is active but theme.tableScrollableWrapper is not defined" (as does TableRepairSettles.test.ts:120). The other new tests use configExtension(TableExtension, {hasHorizontalScroll: false}).
…, and point to D55 ## Description D75 described #9539's padding as current behaviour, but #9539 is an open pull request. The row now says what it would do, and points to D55, the accepted limit for two clients padding a table at once. ## Changeset None: documentation only. ## Test plan ### Before D75 read "Meanwhile #9539 ... pads only the rows that lost cells". ### After ``` $ pnpm run ci-check ... Found 0 errors ```
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,3cells per row instead of3,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 withoutNodeStructureExtensionloses the cells too.The row transform now marks its table dirty when that row lost cells in the update (it has fewer children than in the previous editor state), so the table's own transform pads (it appends the padding cells at the end of the short rows and changes nothing on a full grid). Remote changes applied by
@lexical/yskip transforms as before; the author's editor pads, and the padding is written like any other edit.Which rows it pads: every row when the update changed the table's rows (a row added, removed or moved), as before; when the table's rows are the ones it had, only the rows that lost cells (
$padTableRowstakes an optional$shouldPad), to the width the table has at the end of the update (a row or a cell that changes after them, such as a later transform widening another row, marks the table again). A row that gained cells (a column insert, or an app's own row repair, which may count cells where the table counts grid positions with spans) is left as its author made it. Padding every short row after a change to one of them, or padding the rows that gained cells, made clients that pad the rows they created after remote changes pad each other's rows without end (TableRepairSettles.test.ts, v1 and v2@lexical/yjs). Padding when every client re-runs the table transforms after every remote change does not converge, onmainas here: #9582 tracks that design question.Closes #9520
Changeset
.changeset/table-pad-rows-after-row-change.md(@lexical/table,[fix]).Test plan
PasteAcrossCellsPadsRows.test.ts(@lexical/table, with and withoutNodeStructureExtension) andTablePasteAcrossCells.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).Before
After
Follow-up commit (rows whose cells changed):
TableRowRepairCollab.test.tsin@lexical/yjs(three v2 peers that pad their own rows after remote changes, 40 delivery orders, and two peers): 80/80 pass; 30 failed before it. The same in@lexical/y: 40/40; 30 failed before.PasteAcrossCellsPadsRows.test.ts: a change to one row of a ragged table pads that row only; an update that removes a cell and adds a row (in the update, or in a later row transform) pads every short row; the issue's paste cases still pass.@lexical/y, v1 and v2 bindings with and without owner repairs: 720/720 (22 owner-repair cases failed before the follow-up).main.@lexical/table,@lexical/yjs,@lexical/y,@lexical/react, playground unit tests: 8,511 passed, 46 expected fail, 0 failed.pnpm run ci-checkpasses (TypeScript and Flow types of$padTableRowsmatch).Full unit suite before the follow-up (
pnpm exec vitest run --project unit): 568 files, 14,657 passed, 46 expected fail, 0 failed.@lexical/yfuzz 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 onmainwithout this change.pnpm run ci-checkpasses.