Repository navigation
[lexical-y][lexical-website] Documentation Update: Scope D99, decide D95, record #9589 and the #9725 text entity limit - #9727
Merged
Conversation
…D95, record #9589 and the #9725 text entity limit ## Description Follow-ups to #9723 and three decided limits. Docs and one regression test; no source change. - **D99 (#9721, after #9723).** D99 and its y.md bullet said an undo or redo inside a container always keeps it and that a clear never releases it. That is true only when the undo step's deletion was the first keep on the cell: a cell the binding already kept for a format of its own (D88) is let go when the binding stops, and a deletion in the same cell that is still on a stack is released by the clear, parents included. D99 now says "can keep" with that qualifier (as D98 does), D88 and D98 point to D99 where they say a clear before stopping prevents the residue, and the y.md bullet gets a short label like its neighbours. D99 and the bullet link the Yjs report, yjs/yjs#833. `UndoRedoContainerKeeps.test.ts` (from #9723, kept and amended): asserts the cell's text after every edit, undo and redo (so a step that changed nothing cannot pass), adds the boundary cases (bold in the cell first, then type/undo/redo or delete/undo: 0 left; two deletes and an undo of the second: 0 left), fixes the header comment (destroying an UndoManager releases nothing), and checks the plain-Yjs undo and redo results. - **D95: DECIDED, keep.** The rules that fixed #9671 and #9599 stay; the row moves from Open to the storage model section with its trade numbers unchanged and a decision sentence (taken together the rules fix many more runs than they break, though the null and restore rules only show breaks in two lanes). - **D100, #9589's main variant: ACCEPTED LIMIT.** A format a collaborator applies starting where a run of that format ends can be lost when a client other than the run's author changes part of the run at the same time (12 of 24 cases: both edits, every order of three client ids, gc on and off). The y.md sentence that already describes it now links #9589. - **#9725 (text entities).** A matcher that looks back into a run of repeated characters (`/(?<=\$AA)A/`) can drop the NodeState of one existing character when typing extends the run. Recorded as a known limit under `registerLexicalTextEntity` in transforms.md. ## Changeset None: documentation and a test. ## Test plan ### Before `UndoRedoContainerKeeps.test.ts` on main (d5b30f0): 7 cases, none with a cell already kept, a second deletion, or a check that the edits happened. ``` $ ./node_modules/.bin/vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/UndoRedoContainerKeeps.test.ts ✓ |unit| packages/lexical-y/src/__tests__/unit/regressions/UndoRedoContainerKeeps.test.ts (7 tests) Tests 7 passed (7) ``` ### After ``` $ ./node_modules/.bin/vitest --run --project unit --reporter=verbose packages/lexical-y/src/__tests__/unit/regressions/UndoRedoContainerKeeps.test.ts ✓ a table a collaborator deletes is collected when A typed (gc on) ✓ a table a collaborator deletes is collected when A typed and undid (gc on) ✓ a table a collaborator deletes stays in A’s document when A typed, undid and redid (gc on) ✓ a table a collaborator deletes is collected when A deleted (gc on) ✓ a table a collaborator deletes stays in A’s document when A deleted and undid (gc on) ✓ a table a collaborator deletes is collected when A bolded first, then typed, undid and redid (gc on) ✓ a table a collaborator deletes is collected when A bolded first, then deleted and undid (gc on) ✓ a table a collaborator deletes is collected when A deleted twice and undid the second (gc on) ✓ an undo restoring deleted text in a container keeps it from gc (plain Yjs) ✓ a redo restoring an insert in a container keeps it from gc (plain Yjs) Tests 10 passed (10) ``` `prettier --check` and `eslint` pass on the changed files.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
zurfyx
requested review from
acywatson,
etrepum,
ivailop7 and
potatowagon
as code owners
November 2, 2026 16:28
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
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: |
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
This looks good to me. The changes match the linked issues: D99 is narrowed to "can keep" with its exceptions, D95 is recorded as decided, D100 is added and #9589 is linked. The amended UndoRedoContainerKeeps.test.ts now checks the cell's text after every step, so a step that changed nothing can no longer pass. Everything below is optional and non-blocking.
- D99's condition (
packages/lexical-y/DECISIONS.md:132,packages/lexical-website/docs/collaboration/y.md:1219-1221). Yjs collects a deleted table together with its cells, so the residue depends on whether the step the undo or redo took off the stack was the first thing to keep the table, not the cell. A deletion still on a stack in another cell of the same table releases the row and table on clear. #9721's plain-Yjs repro shows this: C1 stays kept, the row and table do not. With bound editors, deleting in cell 2, then in cell 1, then undoing, clearing and stopping leaves nothing behind once a collaborator deletes the table. The current wording predicts that the table, row and cell stay. Possible wording: "It does when that step's deletion was the first thing to keep the table, walking up from the cell: a deletion elsewhere in the table that is still on a stack releases the row and table with the clear, and a deleted table is collected with its cells." A two-cell case in the test would pin it. - The new pointers to D99 read as unconditional (
DECISIONS.md:60for D88,:131for D98,y.md:1207). D88 says "unless an undo or redo restored content in it", and D98 and y.md say "except for content an undo or redo restored". But the new bold-first cases are collected even though an undo or redo restored content in the cell. Adding "…and that kept it (D99)" would match D99's "can keep". - #9690 in y.md. D95 is now
DECIDED, and the #9690 outcomes it keeps include a single-client undo that removes bold from a word no undone step touched. y.md mentions them in neither Limits nor Open issues.packages/lexical-y/AGENTS.mdasks for accepted limits to be documented in y.md's Limits, and one sentence linking #9690 would cover it. - D100's last sentence (
DECISIONS.md:133). The sentence says "D52 records the same race for the client that formatted the run", but that case passes on this branch in both client id orders (OwnRunRestoreEnd.test.ts, "keeps a collaborator's bold that starts where the author's bold ends"). Only the parked #9562 regressed it. y.md's sentence already limits the race to another client, so the pointer could go, or D52 could note that it applies to #9562 only. - Pinning the other limits (nit). D100 and the #9725 note have no test in the repository.
FormatRuleTrade.test.tsalready does this for D95. Two more tests would catch changes to these outcomes: one asserting #9589's 24-case matrix underpackages/lexical-y/src/__tests__/unit/regressions/, and atest.failsfor #9725 inpackages/lexical-text.
No changeset is needed: CONTRIBUTING.md says tests and the website have no effect on people who use Lexical.
| undo history is cleared and the binding stops: Yjs keeps what an undo step | ||
| deleted, parents included, and a clear releases only the steps still on | ||
| the stacks, not the one the undo or redo took off them. It does when that | ||
| step was the first thing to keep the cell; a cell the binding already |
There was a problem hiding this comment.
Optional: Yjs collects a deleted table together with its cells, so it is whether the popped step was the first thing to keep the table that decides. A deletion still on a stack in another cell of the same table releases the row and table on clear (#9721's plain-Yjs repro: C1 stays kept, the row and table do not), and the table is then collected. Maybe: "It does when that step was the first thing to keep the table; a deletion elsewhere in the table that is still on a stack is released by the clear, row and table included, and a deleted table is collected with its cells." The same applies to D99 in DECISIONS.md and the test header.
| | D24 | **Yjs's format cleanup is off** in `@lexical/y` containers and on every relay that loads a state (`createYDoc`). A delete keeps the ends of each deleted formatted run (within a bound per format key until D86 removed the bound: a delete keeps every format item it leaves). Not reported upstream. | The cleanup deletes format items a concurrent writer may still rely on. Upstream already knows that skipping it is the app's trade-off. | `DECIDED` | 10-28 | [#9400], [y.md: Servers][y-servers] | | ||
| | D86 | **A delete keeps every format item it leaves (no collapse).** The collapse of the run of deleted content and format items a delete leaves (`Cursor#collapseGap`, D24's bound per format key) is removed; D24's cleanup-off stays. | The collapse dropped ends that a concurrent writer's restore or an undo relied on, and the value ran on: #9575, #9609, a #9620 cell, the concurrent restore cells of #9640, a KeepEndsSessions seed. Every attempt to keep the right ends in the collapse (#9617) needed to know which write relies on which item. Measured against main, same seeds: a three-client view fuzz of 15,000 seeds: 0 seeds wrong where main is right, 631 where main spreads a value and this does not. Cost: each formatted run whose text is deleted keeps its two items, none per keystroke or plain delete (F24A-1: 40, 80, 120 after 20, 40, 60 words typed and deleted at one place); a comment added and its word deleted 3,000 times at one place: 6,002 live format items (main 6); a downstream document library's corpus of 189 documents x1.003 at 10 cycles, x1.26 at 150; Jot x1.01 on its default profile, a concentrated recolor 438 items (main 345). Accepted: the linear growth with deleted formatted text, over the bound and its spreads. | `DECIDED` | 10-31 | [#9575], [#9609] | | ||
| | D88 | **A client keeps its own format items from garbage collection while they are undoable (memory only).** The binding sets Yjs's `Item.keep` on the format items its own transactions and its undo manager's insert, and on the end of each run they delete; it releases them, between transactions, when both undo stacks are empty (after a clear, an undo that leaves nothing, or a write they did not capture), and when it is disposed or its document destroyed. An item another binding or an `UndoManager` step also keeps stays kept. A clear of the binding's undo history makes its `UndoManager` release what the cleared steps kept, parents included, also where another `UndoManager` relies on them (Yjs's keep is one flag): the binding keeps again what it still relies on, and releases those keeps when it stops (the last binding on the document that keeps such an item releases it). A container a collaborator deletes meanwhile (a table) stays in this client's state until it reloads, about 108 to 178 bytes per cycle of an edit and a format in a cell, a clear and the table deleted (the same bytes as before this rule; releasing collects nothing); one deleted after the binding stops is collected (when the undo history was cleared before it stopped; D98). After the stop, another `UndoManager` that still needs such an item relies on it unkept, as after a clear of a Yjs `UndoManager`. Releasing collects nothing, as Yjs's `UndoManager` collects nothing when it releases what its steps kept: any `UndoManager` on the document may still restore the item. | An undo reads its change's format items, and the end each gave a run it deleted, after a collaborator deleted them (`isKeptByUndo`, `changeEnd`, `closeRestoredRuns`). With Yjs's default garbage collection, which products use, those were collected, the undo could not tell them apart from other content, and the restored value ran on over the rest of the document ([#9623]; a two-client undo fuzz: 221 of 3,000 seeds to 0). Nothing new is written and other clients are unchanged. The cost is local: a format item a collaborator deletes while this client can undo the change that wrote it keeps its deleted content in this client's document, and in a state it encodes, until the document reloads or a relay that loads its state with gc on compacts it: about 25 bytes per such deletion, in this client only (+0.6% to 0.8% of the client's state in a realistic 4,500-edit session). Accepted over a stored change (a `gcFilter` keeping every format item: +1.9% to 7.6% bytes on long notes, in every peer and relay) and over collecting on release, which was tried and dropped: it destroyed what another `UndoManager` on the same document still needed. A `gcFilter` keeping every format item fixed the same cases, but changes what every peer and relay stores (+1.9% to 7.6% bytes on long notes). | `DECIDED` | 10-31 | [#9623], [y.md: Limits][limits] | | ||
| | D88 | **A client keeps its own format items from garbage collection while they are undoable (memory only).** The binding sets Yjs's `Item.keep` on the format items its own transactions and its undo manager's insert, and on the end of each run they delete; it releases them, between transactions, when both undo stacks are empty (after a clear, an undo that leaves nothing, or a write they did not capture), and when it is disposed or its document destroyed. An item another binding or an `UndoManager` step also keeps stays kept. A clear of the binding's undo history makes its `UndoManager` release what the cleared steps kept, parents included, also where another `UndoManager` relies on them (Yjs's keep is one flag): the binding keeps again what it still relies on, and releases those keeps when it stops (the last binding on the document that keeps such an item releases it). A container a collaborator deletes meanwhile (a table) stays in this client's state until it reloads, about 108 to 178 bytes per cycle of an edit and a format in a cell, a clear and the table deleted (the same bytes as before this rule; releasing collects nothing); one deleted after the binding stops is collected (when the undo history was cleared before it stopped; D98), unless an undo or redo restored content in it (D99). After the stop, another `UndoManager` that still needs such an item relies on it unkept, as after a clear of a Yjs `UndoManager`. Releasing collects nothing, as Yjs's `UndoManager` collects nothing when it releases what its steps kept: any `UndoManager` on the document may still restore the item. | An undo reads its change's format items, and the end each gave a run it deleted, after a collaborator deleted them (`isKeptByUndo`, `changeEnd`, `closeRestoredRuns`). With Yjs's default garbage collection, which products use, those were collected, the undo could not tell them apart from other content, and the restored value ran on over the rest of the document ([#9623]; a two-client undo fuzz: 221 of 3,000 seeds to 0). Nothing new is written and other clients are unchanged. The cost is local: a format item a collaborator deletes while this client can undo the change that wrote it keeps its deleted content in this client's document, and in a state it encodes, until the document reloads or a relay that loads its state with gc on compacts it: about 25 bytes per such deletion, in this client only (+0.6% to 0.8% of the client's state in a realistic 4,500-edit session). Accepted over a stored change (a `gcFilter` keeping every format item: +1.9% to 7.6% bytes on long notes, in every peer and relay) and over collecting on release, which was tried and dropped: it destroyed what another `UndoManager` on the same document still needed. A `gcFilter` keeping every format item fixed the same cases, but changes what every peer and relay stores (+1.9% to 7.6% bytes on long notes). | `DECIDED` | 10-31 | [#9623], [y.md: Limits][limits] | |
There was a problem hiding this comment.
Optional: "unless an undo or redo restored content in it (D99)" reads as unconditional. In the new bold-first cases the binding kept the cell for its bold, an undo or redo then restored content in it, and the table is still collected. "…restored content in it and that kept it (D99)" would match D99's "can keep"; the same goes for D98 and y.md:1207.
| | D88 | **A client keeps its own format items from garbage collection while they are undoable (memory only).** The binding sets Yjs's `Item.keep` on the format items its own transactions and its undo manager's insert, and on the end of each run they delete; it releases them, between transactions, when both undo stacks are empty (after a clear, an undo that leaves nothing, or a write they did not capture), and when it is disposed or its document destroyed. An item another binding or an `UndoManager` step also keeps stays kept. A clear of the binding's undo history makes its `UndoManager` release what the cleared steps kept, parents included, also where another `UndoManager` relies on them (Yjs's keep is one flag): the binding keeps again what it still relies on, and releases those keeps when it stops (the last binding on the document that keeps such an item releases it). A container a collaborator deletes meanwhile (a table) stays in this client's state until it reloads, about 108 to 178 bytes per cycle of an edit and a format in a cell, a clear and the table deleted (the same bytes as before this rule; releasing collects nothing); one deleted after the binding stops is collected (when the undo history was cleared before it stopped; D98), unless an undo or redo restored content in it (D99). After the stop, another `UndoManager` that still needs such an item relies on it unkept, as after a clear of a Yjs `UndoManager`. Releasing collects nothing, as Yjs's `UndoManager` collects nothing when it releases what its steps kept: any `UndoManager` on the document may still restore the item. | An undo reads its change's format items, and the end each gave a run it deleted, after a collaborator deleted them (`isKeptByUndo`, `changeEnd`, `closeRestoredRuns`). With Yjs's default garbage collection, which products use, those were collected, the undo could not tell them apart from other content, and the restored value ran on over the rest of the document ([#9623]; a two-client undo fuzz: 221 of 3,000 seeds to 0). Nothing new is written and other clients are unchanged. The cost is local: a format item a collaborator deletes while this client can undo the change that wrote it keeps its deleted content in this client's document, and in a state it encodes, until the document reloads or a relay that loads its state with gc on compacts it: about 25 bytes per such deletion, in this client only (+0.6% to 0.8% of the client's state in a realistic 4,500-edit session). Accepted over a stored change (a `gcFilter` keeping every format item: +1.9% to 7.6% bytes on long notes, in every peer and relay) and over collecting on release, which was tried and dropped: it destroyed what another `UndoManager` on the same document still needed. A `gcFilter` keeping every format item fixed the same cases, but changes what every peer and relay stores (+1.9% to 7.6% bytes on long notes). | `DECIDED` | 10-31 | [#9623], [y.md: Limits][limits] | | ||
| | D96 | **A run ends in its own container, and no item is linked next to an item of another container.** `changeEnd` and `lastOwnItemBefore` look only at items in the run's shared type; the cursor refuses to link a new item next to an item of another type (the write throws and is recovered from, `RECOVERED_WRITE_WARNING`). | The end a write gives a restated run at its range's end (D78's rule 3, [#9679]) came from `changeEnd`, which walks the client's items in clock order across containers and could return an item in another type; the writer linked its end into that type's list while naming the run's container as its parent. Every other client took the item's parent from its origin and put it in the root's list, where it changes nothing, but the writer's own document held a malformed item, and its editor stopped reading part of the document for good (a downstream app's long notes: 15 of 133 sessions). A structure check (no item's neighbour in another container) now runs in every harness convergence check, the undo fuzz and a structure fuzz. | `DECIDED` | 11-01 | [#9694] | | ||
| | D78 | **A format spreading after one client's triple undo:** a client formats a run, types plain text, formats again and undoes all three while a collaborator types plain text inside the first run. | Fixed at the source: a restore's scan (`runOpener`) stops at content of another client, so the writer's earlier end and later opener with a collaborator's text between them are no longer read as one restored run, and the undo writes the run's end. A two-client run-sequence fuzz: 8 of 1,000 seeds to 0, gc on and off. | `DECIDED` | 10-31 | [#9599] | | ||
| | D95 | **Keep the rules that fixed #9671 and #9599, although they make rare other cases wrong, one of them for a single client.** A format over a range that starts at the writer's own earlier insert null removes that null (the null rule), one whose range ends at the writer's own earlier insert restore removes that restore and joins the runs (the restore rule; both [#9671]), and a restore's scan for the run it restates stops at another client's text (the scan rule, D78, [#9599]). | The restore rule also changes a single client's own undo, with no collaborator acting: type plain text into a bold run, bold a range ending at it, type plain text later on, undo the three steps, and a word none of them touched loses its bold (main before these rules was right) ([#9690]). Each rule disabled alone, same fresh seeds, gc on and off (counts of runs main gets right and the rule's absence wrong, then the reverse): a raw-Yjs-oracle op-list generator, 36,000 runs: null rule 18 / 4, restore rule 28 / 0, scan rule 2 / 1 (gc on and off identical; all three off equals main before them); its 18,000-run original set: 20 / 0, 6 / 0, 0 / 0; a three-client view property, 15,000 seeds: null rule 0 / 2 (gc off), restore rule 0 / 1 (gc on), scan rule 2 / 0; a sequential undo fuzz, 3,000 seeds: restore rule 0 / 2 (one single-client); a two-client run-sequence fuzz, 1,000: scan rule 7 / 0; unit cells: null rule 8, restore rule 16, scan rule 5; a long-session soak, about 20,600 sessions: 2 sessions wrong (null rule 1, scan rule 1) against 25 the other way. None of the narrower variants tried kept the gains: the writer cannot tell from the document whether a collaborator's lone null ends a format range or starts a removal (a variant of the null rule that writes the range's own opener fixed its cases and lost all 8 of its unit cells). Decided on 11-02 to keep all three: taken together they fix many more runs than they break (in the 36,000-run generator, 18, 28 and 2 runs against 4, 0 and 1 for the null, restore and scan rules), although in the three-client view property and the sequential undo fuzz the null and restore rules only break runs (null rule 2, restore rule 1 and 2), and stored provenance, which would avoid both sides, is not ready to ship. Dropping the restore rule for its single-client case was weighed and not taken. | `DECIDED` | 11-02 | [#9690], [#9679], [#9671], [#9599] | |
There was a problem hiding this comment.
| | D99 | **An undo or redo that restores content inside a container keeps that container from garbage collection until reload.** When content inside a container (such as a table cell) is deleted—either directly by an edit, or when undoing an insertion—Yjs's `UndoManager` records that deletion on a stack item, keeping the deleted item and walking up the parents (the cell, row and table). When an undo restores that deleted content, or a redo re-creates the inserted text, `popStackItem` removes that deletion step from the stack. Clearing every `UndoManager` and stopping the binding does not release them: Yjs's `clearUndoManagerStackItem` only calls `keepItem(item, false)` for items in deletions of steps still on the stacks, so the keeps set by the popped deletion step are never un-kept (and in the redo case, setting only the re-created copy's keep back to false still leaves the parents kept). If a collaborator later deletes that container, this client's document and encoded state retain the container's structs until reload: about 139 to 151 bytes for a one-cell table with nine characters, growing with the table's content. Reloading the document, or loading its encoded state into a fresh document with garbage collection on (a server or relay), compacts it. Nothing reaches other peers. | Not a binding bug: reproduced on plain Yjs `UndoManager` calls without an editor and across every binding tree. A binding-side release would have to know which keeps came from a popped deletion step; Yjs's `keep` is a single boolean shared by every `UndoManager` on the document, so the binding cannot tell them apart safely without risking un-keeping items another manager relies on. | `ACCEPTED LIMIT` | 11-02 | [#9721], [limits] | | ||
| | D98 | **A Yjs `UndoManager` destroyed without a clear keeps what its steps protected.** The binding destroys its undo manager without clearing it when it stops (`binding.dispose()`, or the cleanup its start returns: an editor unmounted or mounted again), and Yjs destroys an `UndoManager` when its document is destroyed. Yjs then keeps, until the document is reloaded, the items the destroyed manager's steps kept: a character deleted in a table cell can keep that cell, its row and its table. It does when the delete's step is the first thing to keep the cell (Yjs's `keepItem` stops at an ancestor already kept); where the binding already kept the cell for one of its own format items (D88) before the delete, the step keeps only the character, and the binding's release at stop lets the cell, row and table go. When it does, a table a collaborator deletes afterwards stays in that client's document, and in any state it encodes: 108 to 175 bytes per table where one character was deleted and one word bolded in one cell of a 1x1 to a 4x4 table, more where more was deleted or more cells were edited (about 1.3 KB for one character deleted in every cell of a 4x4 table). Binding the same document again does not free it. Clearing the history (`binding.undoManager?.clear()`, while the binding is started) before stopping and before the table is deleted prevents it, where no other `UndoManager` on the document still needs what it kept (a clear releases that too), except for content an undo or redo restored in the cell (D99); a table deleted while the history still held such a step stays after a later clear too, as releasing a keep collects nothing (D88). Reloading the document, or loading its encoded state into a new document with garbage collection on (a server or relay), compacts it. | Yjs's `UndoManager#destroy` does not clear, and its keep is one flag set on an item and its parents; any editor that destroys a Yjs `UndoManager` without clearing it first keeps the same items. Measured over 50 cycles per table size (an edit in a cell, the binding stopped, the table deleted by a collaborator): the same bytes on the release before D88's own-format keep, with that keep turned off and on the current release, and none when the history was cleared before stopping. Clearing at stop in the binding was weighed and not taken: Yjs's clear releases with that one flag, so it would also release what another `UndoManager` on the document (an app's, or a second binding's) still keeps for its own undo, and a later collection could drop what that undo restores (the hazard D88 records for a clear). | `ACCEPTED LIMIT` | 11-01 | [#9698], [limits] | | ||
| | D99 | **An undo or redo that restores content inside a container can keep that container from garbage collection until reload.** Yjs's `UndoManager` keeps what a step deletes, walking up its parents (a table cell, its row and its table). An undo or redo that restores that content takes the step off the stacks, and Yjs's `clear()` releases only the steps still on them, so clearing the history before the binding stops does not release the cell, row and table. It does when that step's deletion is the first thing to keep the cell (Yjs's `keepItem` stops at an ancestor already kept): a cell the binding already kept for one of its own format items (D88) is let go when the binding stops, and a deletion in the same cell that is still on a stack is released by the clear, parents included. When it does, a table a collaborator deletes afterwards stays in that client's document, and in any state it encodes, until it reloads: 139 to 151 bytes for a one-cell table with nine characters, more with more content. Reloading the document, or loading its encoded state into a new document with garbage collection on (a server or relay), compacts it. Nothing reaches other clients. | Yjs behaviour, reproduced with plain `UndoManager` calls and no editor, and on every binding release. A release in the binding would have to know which keeps came from a step an undo or redo took off the stacks; Yjs's keep is one flag on each item, shared by every `UndoManager` on the document, so the binding cannot release it without risking what another undo manager still keeps for its own undo (the hazard D88 and D98 record). The fix belongs in Yjs: reported as [yjs#833]. | `ACCEPTED LIMIT` | 11-02 | [#9721], [#9723], [yjs#833], [limits] | | ||
| | D100 | **A format a collaborator applies starting right where a run of that format ends can be lost when a client other than the run's author changes part of the run at the same time:** with `Kilo` bolded by one client, another unbolds `il` or types plain text inside `Kilo` while the first or a third client bolds ` bb`; with some client id orders ` bb` stays plain on every client (12 of 24 cases in a test of both edits, every order of three client ids and gc on and off, on every version tested). No text is lost, and when the client that changed the run undoes its change, ` bb` is bold again. | The change inside the run writes a null right after the run's end, so that the run's value does not run on onto text a collaborator types there, and the other client's opener of ` bb` has the same neighbours; Yjs orders the two by client id, so with the writer's id higher its null lands between that opener and ` bb`. Writing the null anywhere else gives up that protection. A repair after the fact, or stored provenance, could avoid it; neither is worth its cost now. D52 records the same race for the client that formatted the run. | `ACCEPTED LIMIT` | 11-02 | [#9589], [limits] | |
There was a problem hiding this comment.
Nit: the case of the author changing its own run, which D52 describes, passes on this branch in both id orders (OwnRunRestoreEnd.test.ts, "keeps a collaborator's bold that starts where the author's bold ends"). Only the parked #9562 regressed it. Consider dropping this pointer or marking D52 as #9562-only.
…wo-cell case - D99, y.md and the test header: the table is kept when the popped step was the first thing to keep it; a deletion elsewhere in the table that is still on a stack releases the row and table with the clear. - D88, D98 and y.md pointers to D99 say "and that kept it". - y.md documents the #9690 single-client undo that D95 keeps. - D100: drop the D52 pointer; the author-run case passes on main. - FormatAtRunEndRace.test.ts pins #9589 (12 orders pass, 12 test.fails). - registerLexicalTextEntityLookbehind.test.ts pins #9725 (test.fails). - UndoRedoContainerKeeps: a two-cell case collects the table.
There was a problem hiding this comment.
This looks good. The D99 qualifier, the D95 decision, D100 and the #9725 caveat are each backed by a test, and the new test.fails pins fail on the assertion they document. A few optional, non-blocking suggestions:
- D99's condition misses later deletions. This affects the D99 row (
packages/lexical-y/DECISIONS.md, line 132),packages/lexical-website/docs/collaboration/y.mdlines 1218-1222, and the header ofUndoRedoContainerKeeps.test.ts(lines 18-22). A clear calls Yjs'skeepItem(item, false)on every step still on a stack, and that call walks up whilekeepis set. So a deletion in the table made after the undo also releases the cell, row and table. With the test's own harness,['delete', 'undo', 'delete again']and['type', 'undo', 'redo', 'delete']each leave 0 containers, even though the undone step was the first thing to keep the table. Only the binding's own format keep depends on order:['delete', 'undo', 'bold']leaves 3. Possible wording: "It does unless something releases the table afterwards: a clear releases every step still on a stack that deleted in the table, before or after the undo, row and table included; and the binding lets go a cell it kept for a format of its own before that step." Adding['delete', 'undo', 'delete again']→0toCASESwould pin it. - Cross-reference D52 and D100 (
packages/lexical-y/DECISIONS.md, lines 119 and 133). D52 still records the run author's own variant as an accepted limit whose "repair is tracked in #9589", while D100 now accepts the other-client variant and declines a repair. #9589 reports thatmainkeeps the format in the author's variant; it reproduced only with #9562's parked rule. DECISIONS.md's header asks that "a replaced decision says what replaced it", andpackages/lexical-y/AGENTS.mdasks a pull request that changes a recorded limit to update its row. D52 could point to D100, and the other way round. - Nit: pin "No text is lost" in
FormatAtRunEndRace.test.ts(line 75). Asserting afterexpectConvergedthat$getRoot().getTextContent()isabcdefgh(orabcXdefgh) would cover the second half of D100's claim.
No changeset is needed. CONTRIBUTING.md says tests and website docs need none, and DECISIONS.md is not in @lexical/y's published files.
| undo history is cleared and the binding stops: Yjs keeps what an undo step | ||
| deleted, parents included, and a clear releases only the steps still on | ||
| the stacks, not the one the undo or redo took off them. It does when that | ||
| step was the first thing to keep the table; a cell the binding already |
There was a problem hiding this comment.
Optional: "the first thing to keep the table" is not the deciding condition for deletions. A clear's keepItem(item, false) walks up while keep is set, so a deletion in the table made after the undo, still on the stack, releases the cell, row and table too. With the test's harness, ['delete', 'undo', 'delete again'] leaves 0 containers, and so does ['type', 'undo', 'redo', 'delete']. Only the binding's own format keep depends on order. Possible wording: "It does unless something releases the table afterwards: a clear releases every step still on a stack that deleted in the table, before or after the undo, row and table included; and the binding lets go a cell it kept for a format of its own before that step."
| | D99 | **An undo or redo that restores content inside a container keeps that container from garbage collection until reload.** When content inside a container (such as a table cell) is deleted—either directly by an edit, or when undoing an insertion—Yjs's `UndoManager` records that deletion on a stack item, keeping the deleted item and walking up the parents (the cell, row and table). When an undo restores that deleted content, or a redo re-creates the inserted text, `popStackItem` removes that deletion step from the stack. Clearing every `UndoManager` and stopping the binding does not release them: Yjs's `clearUndoManagerStackItem` only calls `keepItem(item, false)` for items in deletions of steps still on the stacks, so the keeps set by the popped deletion step are never un-kept (and in the redo case, setting only the re-created copy's keep back to false still leaves the parents kept). If a collaborator later deletes that container, this client's document and encoded state retain the container's structs until reload: about 139 to 151 bytes for a one-cell table with nine characters, growing with the table's content. Reloading the document, or loading its encoded state into a fresh document with garbage collection on (a server or relay), compacts it. Nothing reaches other peers. | Not a binding bug: reproduced on plain Yjs `UndoManager` calls without an editor and across every binding tree. A binding-side release would have to know which keeps came from a popped deletion step; Yjs's `keep` is a single boolean shared by every `UndoManager` on the document, so the binding cannot tell them apart safely without risking un-keeping items another manager relies on. | `ACCEPTED LIMIT` | 11-02 | [#9721], [limits] | | ||
| | D98 | **A Yjs `UndoManager` destroyed without a clear keeps what its steps protected.** The binding destroys its undo manager without clearing it when it stops (`binding.dispose()`, or the cleanup its start returns: an editor unmounted or mounted again), and Yjs destroys an `UndoManager` when its document is destroyed. Yjs then keeps, until the document is reloaded, the items the destroyed manager's steps kept: a character deleted in a table cell can keep that cell, its row and its table. It does when the delete's step is the first thing to keep the cell (Yjs's `keepItem` stops at an ancestor already kept); where the binding already kept the cell for one of its own format items (D88) before the delete, the step keeps only the character, and the binding's release at stop lets the cell, row and table go. When it does, a table a collaborator deletes afterwards stays in that client's document, and in any state it encodes: 108 to 175 bytes per table where one character was deleted and one word bolded in one cell of a 1x1 to a 4x4 table, more where more was deleted or more cells were edited (about 1.3 KB for one character deleted in every cell of a 4x4 table). Binding the same document again does not free it. Clearing the history (`binding.undoManager?.clear()`, while the binding is started) before stopping and before the table is deleted prevents it, where no other `UndoManager` on the document still needs what it kept (a clear releases that too), except where an undo or redo restored content in the cell and that kept it (D99); a table deleted while the history still held such a step stays after a later clear too, as releasing a keep collects nothing (D88). Reloading the document, or loading its encoded state into a new document with garbage collection on (a server or relay), compacts it. | Yjs's `UndoManager#destroy` does not clear, and its keep is one flag set on an item and its parents; any editor that destroys a Yjs `UndoManager` without clearing it first keeps the same items. Measured over 50 cycles per table size (an edit in a cell, the binding stopped, the table deleted by a collaborator): the same bytes on the release before D88's own-format keep, with that keep turned off and on the current release, and none when the history was cleared before stopping. Clearing at stop in the binding was weighed and not taken: Yjs's clear releases with that one flag, so it would also release what another `UndoManager` on the document (an app's, or a second binding's) still keeps for its own undo, and a later collection could drop what that undo restores (the hazard D88 records for a clear). | `ACCEPTED LIMIT` | 11-01 | [#9698], [limits] | | ||
| | D99 | **An undo or redo that restores content inside a container can keep that container from garbage collection until reload.** Yjs's `UndoManager` keeps what a step deletes, walking up its parents (a table cell, its row and its table). An undo or redo that restores that content takes the step off the stacks, and Yjs's `clear()` releases only the steps still on them, so clearing the history before the binding stops does not release the cell, row and table. It does when that step's deletion is the first thing to keep the table, walking up from the cell (Yjs's `keepItem` stops at an ancestor already kept): a cell the binding already kept for one of its own format items (D88) is let go when the binding stops, and a deletion elsewhere in the table that is still on a stack is released by the clear, row and table included (a deleted table is collected with its cells). When it does, a table a collaborator deletes afterwards stays in that client's document, and in any state it encodes, until it reloads: 139 to 151 bytes for a one-cell table with nine characters, more with more content. Reloading the document, or loading its encoded state into a new document with garbage collection on (a server or relay), compacts it. Nothing reaches other clients. | Yjs behaviour, reproduced with plain `UndoManager` calls and no editor, and on every binding release. A release in the binding would have to know which keeps came from a step an undo or redo took off the stacks; Yjs's keep is one flag on each item, shared by every `UndoManager` on the document, so the binding cannot release it without risking what another undo manager still keeps for its own undo (the hazard D88 and D98 record). The fix belongs in Yjs: reported as [yjs#833]. | `ACCEPTED LIMIT` | 11-02 | [#9721], [#9723], [yjs#833], [limits] | | ||
| | D100 | **A format a collaborator applies starting right where a run of that format ends can be lost when a client other than the run's author changes part of the run at the same time:** with `Kilo` bolded by one client, another unbolds `il` or types plain text inside `Kilo` while the first or a third client bolds ` bb`; with some client id orders ` bb` stays plain on every client (12 of 24 cases in a test of both edits, every order of three client ids and gc on and off, on every version tested). No text is lost, and when the client that changed the run undoes its change, ` bb` is bold again. | The change inside the run writes a null right after the run's end, so that the run's value does not run on onto text a collaborator types there, and the other client's opener of ` bb` has the same neighbours; Yjs orders the two by client id, so with the writer's id higher its null lands between that opener and ` bb`. Writing the null anywhere else gives up that protection. A repair after the fact, or stored provenance, could avoid it; neither is worth its cost now. | `ACCEPTED LIMIT` | 11-02 | [#9589], [limits] | |
There was a problem hiding this comment.
Optional: D52 (line 119) still records the run author's own variant of #9589 as an accepted limit whose "repair is tracked in #9589", while this row declines a repair. #9589 reports that main keeps the format in the author's variant (only #9562's parked rule loses it). Per this file's header ("a replaced decision says what replaced it"), D52 and D100 could point to each other.
| .filter((n: TextNode) => /[fg]/.test(n.getTextContent())), | ||
| ); | ||
| expect(fg.length).toBeGreaterThan(0); | ||
| expect(a.read(() => fg.every(n => n.hasFormat('bold')))).toBe(true); |
There was a problem hiding this comment.
Nit: D100 also says no text is lost. Asserting expect(a.read(() => $getRoot().getTextContent())).toBe(how === 'remove' ? 'abcdefgh' : 'abcXdefgh') here would pin that half of the claim.
- D99, y.md and the test header: a clear releases every step still on a stack that deleted in the table, before or after the undo; only the binding's own format keep depends on order. New case: delete, undo, delete again leaves 0. - D52 and D100 point to each other. - FormatAtRunEndRace asserts that no text is lost.
There was a problem hiding this comment.
This looks good to me. The D99 wording now matches how Yjs keeps and releases items (a step's keep stops at an ancestor that is already kept, and a clear releases only the steps still on a stack), D52's replacement matches the run-end rule skipping the run's own author, and each new test.fails fails on the assertion it documents. A few optional, non-blocking suggestions:
packages/lexical-y/src/__tests__/unit/regressions/FormatAtRunEndRace.test.ts:36: in the 12 losing orders the whole body runs undertest.fails, so a divergence (expectConverged, line 68) or lost text (lines 70-72) in those orders would also pass as the expected failure. Splitting each losing case into a plain test for convergence and text and atest.failsfor the bold check alone keeps thetest.failsrule inpackages/lexical-y/AGENTS.mdand would catch that.packages/lexical-text/src/__tests__/unit/registerLexicalTextEntityLookbehind.test.ts:110has the same shape.- The run author's variant of D100: D100 and
y.md(line 1382, "as the first or a third boldsbb") also cover the run's own author bolding at the run's end, but the matrix only has a third client doing it. That variant loses the bold the same way when the editing client's id is above the author's, so adding it to the matrix would pin the claim. .changeset/lexical-text-entity-node-state.md: this pending entry from #9713 tells upgraders that plain text keeps its NodeState when it becomes an entity, and #9725 is an exception in the same release. CONTRIBUTING.md describes the changelog as how people upgrading learn what changed, so one sentence there naming the exception (or a fix before the release) would keep it consistent with the new note intransforms.md.packages/lexical-website/docs/collaboration/y.md:1347:D95is the only decision-register id on that page and has no link, so website readers cannot look it up. Linkingpackages/lexical-y/DECISIONS.mdon GitHub, like the issue link beside it, or dropping the id would help.
No changeset is needed for this pull request: tests, the website and DECISIONS.md need none under CONTRIBUTING.md.
| for (const gc of [false, true]) { | ||
| // ids are [A, B, C] | ||
| const loses = ids[0] > ids[2]; | ||
| (loses ? test.fails : test)( |
There was a problem hiding this comment.
Optional: in the losing orders this test.fails also wraps the convergence check (line 68) and the text check (lines 70-72), so a divergence or lost text there would still count as the expected failure. Splitting each losing case into a plain test for convergence and text plus a test.fails for the bold check alone keeps the test.fails convention in packages/lexical-y/AGENTS.md and would make such a regression fail.
| A client's own undo can remove a format from a word that none of its | ||
| undone steps touched: type plain text into a bold run, bold a range that | ||
| ends at that text, type plain text later on, and undo the three steps | ||
| ([#9690](https://github.com/meta/lexical/issues/9690); kept, D95). |
There was a problem hiding this comment.
Optional: D95 is the only decision-register id on this page and has no link, so website readers cannot look it up. Consider linking packages/lexical-y/DECISIONS.md on GitHub, like the issue link beside it, or dropping the id.
| ): Array<() => void>; | ||
| ``` | ||
|
|
||
| Known limit: a matcher that looks back into a run of repeated characters |
There was a problem hiding this comment.
Thanks, this looks good. The D99 qualifier matches how Yjs 14's UndoManager keeps and releases (keepItem walks up while keep is set, and clear() only releases steps still on the stacks), the new boundary cases in UndoRedoContainerKeeps.test.ts exercise exactly those paths with the cell text asserted after every step, and the D52 and D100 matrices reproduce as described. A few optional, non-blocking suggestions:
- Pin D100's other variant (
packages/lexical-y/src/__tests__/unit/regressions/FormatAtRunEndRace.test.ts:79). D100 and the sentence incollaboration/y.mdsay the bold is lost when "the first or a third" client boldsbb, but the matrix only has C bolding. With B (the run's author) boldingfgwhile A unboldsbcor typesX, the same harness loses the bold in 6 of 12 runs per edit, the orders where A's id is above B's. Abolderdimension (loses = ids[0] > ids[bolder]) would cover both halves. - Pin the single-client #9690 case that y.md now documents (
packages/lexical-website/docs/collaboration/y.md:1344-1348). With one client: boldabcd, type plainZaftera, boldaZ, type plainYbetweencandd, undo three times, anddloses its bold (gc on and off). Atest.failsbesideFormatRuleTrade.test.tswould guard the documented behavior and flag when a provenance fix lands. - Widen the #9725 wording (
packages/lexical-website/docs/concepts/transforms.md:196-200and.changeset/lexical-text-entity-node-state.md:10).$isInsertedTexttreats the characters after the common prefix as inserted, so any typed text that repeats what follows the caret is affected, not only single-character runs: typingABafter$in$ABwith/(?<=\$AB)AB/drops the NodeState of the existingAB. Something like "typing text that repeats the characters right after the caret" would cover it. - Nit: the comment at
UndoRedoContainerKeeps.test.ts:107-109says the binding lets the cell go when it stops; in that flow it already lets go atundoManager.clear(), once both stacks are empty (D88).
| for (const ids of IDS) { | ||
| for (const gc of [false, true]) { | ||
| // ids are [A, B, C] | ||
| const loses = ids[0] > ids[2]; |
There was a problem hiding this comment.
Optional: D100 and the y.md sentence say the bold is lost when "the first or a third" client bolds after the run, but the matrix only has C bolding. With B (the run's author) bolding fg while A unbolds bc or types X, the bold is lost in 6 of 12 runs per edit, exactly where A's id is above B's. A bolder dimension (loses = ids[0] > ids[bolder]) would pin both halves.
Comment on lines
+1344
to
+1348
| A client's own undo can remove a format from a word that none of its | ||
| undone steps touched: type plain text into a bold run, bold a range that | ||
| ends at that text, type plain text later on, and undo the three steps | ||
| ([#9690](https://github.com/meta/lexical/issues/9690); kept, because | ||
| the format rules behind it fix many more cases than they break). |
There was a problem hiding this comment.
Optional: this single-client case reproduces deterministically with one client (bold abcd, type plain Z after a, bold aZ, type plain Y between c and d, undo three times: d loses its bold, gc on and off), and no test pins it. A test.fails beside FormatRuleTrade.test.ts would guard the behavior documented here.
Comment on lines
+196
to
+200
| Known limit: a matcher that looks back into a run of repeated characters | ||
| (such as `/(?<=\$AA)A/`, an `A` right after `$AA`) can drop the NodeState of | ||
| one existing character when typing extends the run, because the edit is read | ||
| as text inserted at the end of the run | ||
| ([#9725](https://github.com/meta/lexical/issues/9725)). |
There was a problem hiding this comment.
Optional: $isInsertedText treats the characters after the common prefix as inserted, so this is not limited to runs of one repeated character: typing AB after $ in $AB with /(?<=\$AB)AB/ also drops the NodeState of the existing AB. Something like "typing text that repeats the characters right after the caret" would cover it (the changeset sentence too).
Comment on lines
+107
to
+109
| // The binding kept the cell for its own bold (D88) before the undo step's | ||
| // deletion, so that step kept only what it deleted (keepItem stops at a | ||
| // kept parent), and the binding lets the cell go when it stops. |
There was a problem hiding this comment.
Nit: in this flow the binding already lets the cell, row and table go at undoManager.clear() (both stacks empty, D88), before A.stop(); "when it stops" could say "once its undo history is cleared".
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-ups to #9723 and three decided limits. Docs and tests; no source change.
UndoRedoContainerKeeps.test.ts(from [lexical-website][lexical-y] Documentation Update: Record that an undo or redo restoring content inside a container keeps it from garbage collection until reload (D99) #9723, kept and amended): asserts the cell's text after every edit, undo and redo (so a step that changed nothing cannot pass), adds the boundary cases (bold in the cell first, then type/undo/redo or delete/undo: 0 left; two deletes in one cell and an undo of the second: 0 left; deletes in two cells and an undo of the second: 0 left; delete, undo, delete again: 0 left), fixes the header comment (destroying an UndoManager releases nothing), and checks the plain-Yjs undo and redo results.FormatAtRunEndRace.test.tspins the matrix: convergence and text as plain tests in all 24 cases, the bold as a test in the 12 passing orders and atest.failsin the 12 losing ones./(?<=\$AA)A/) can drop the NodeState of one existing character when typing extends the run. Recorded as a known limit underregisterLexicalTextEntityin transforms.md and in the pending [lexical-text][lexical-playground] Bug Fix: Text entity transforms settle between text nodes of other formats (re-land of #9686) #9713 changeset, and pinned inregisterLexicalTextEntityLookbehind.test.ts(the text as a test, the NodeState as atest.fails).Changeset
None new: documentation and tests. The pending #9713 changeset gets one sentence about the #9725 exception.
Test plan
Before
UndoRedoContainerKeeps.test.tson main (d5b30f0): 7 cases, none with a cell already kept, a second deletion, or a check that the edits happened.After
prettier --checkandeslintpass on the changed files.