From 0609ba28dbd3babcfff0bb94aa24b697ab7093f3 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Sat, 31 Oct 2026 21:29:39 -0700 Subject: [PATCH 1/6] [lexical-y] Bug Fix: An undo that ends a restored run links its null to the items the store holds ## Description Stacked on #9693 (it extends that PR's structure check). An undo that brings back a run gives it an end of this client's own where a closer ended it (`closeRestoredRuns`). It found where in one transaction and wrote the null in a second one, keeping the item objects it had found. The first transaction's scan splits items and its cleanup merges them back, so the item to the null's left could be a piece no longer in the store. The null was linked to that piece: the merged item in the store kept its old right, so a walk of the list skipped the null. Only the undoing client's document is affected (others integrate the null from its origin), and its editor can stop matching its document: every pre-existing editor-vs-document divergence a downstream long-session fuzz found starts with such a link. Now: - `closeRestoredRuns` keeps ids, not items, across the two transactions, and resolves them in the store when it writes (splitting there as Yjs does); it writes nothing where the two are no longer next to each other. - The running undo's own items (`isKeptByUndo`) are looked up by id in the step's insertions instead of by the item objects an earlier transaction saw. - The structure check (`misLinkedItems`, run by `expectConverged`, the undo fuzz and the structure fuzz) also requires each neighbour to link back and to have the same map key. Audit of other item references kept across a transaction: the format-closer marks and the kept format items are format items, which Yjs never splits or merges; the stored nodes an undo removes are nested types, likewise. Nothing else outlives its transaction. ## Test plan ### Before ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/UndoRunEndLinks.test.ts AssertionError: after step 7: expected [ '795599280632996:0' ] to deeply equal [] AssertionError: after step 7: expected [ '2159088886642643:0' ] to deeply equal [] Tests 2 failed (2) ``` ### After ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/UndoRunEndLinks.test.ts Tests 2 passed (2) $ LEXICAL_Y_GC=0 vitest --run --project unit packages/lexical-y Test Files 241 passed | 2 skipped (243) Tests 19231 passed | 304 expected fail | 130 skipped (19665) $ LEXICAL_Y_GC=1 vitest --run --project unit packages/lexical-y Test Files 241 passed | 2 skipped (243) Tests 19228 passed | 307 expected fail | 130 skipped (19665) $ pnpm run ci-check exit 0 ``` --- .changeset/y-undo-run-end-links.md | 7 + .../unit/regressions/UndoRunEndLinks.test.ts | 190 ++++++++++++++++++ .../src/__tests__/utils/structure.ts | 23 ++- packages/lexical-y/src/internal/Binding.ts | 64 ++++-- 4 files changed, 261 insertions(+), 23 deletions(-) create mode 100644 .changeset/y-undo-run-end-links.md create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/UndoRunEndLinks.test.ts diff --git a/.changeset/y-undo-run-end-links.md b/.changeset/y-undo-run-end-links.md new file mode 100644 index 00000000000..4fa3d582361 --- /dev/null +++ b/.changeset/y-undo-run-end-links.md @@ -0,0 +1,7 @@ +--- +'@lexical/y': patch +--- + +[fix] An undo that ends a restored run keeps the document's item list linked: the client that undid no longer holds an item a walk of its list skips, which could make its editor stop matching its document + +Verify: two clients synced after every step; one deletes two characters of a line, the other comments the end of one line and part of another and bolds part of a third, the first types " new " and deletes part of it, removes the first comment and undoes that. Every item's neighbours link back to it on both clients. diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoRunEndLinks.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoRunEndLinks.test.ts new file mode 100644 index 00000000000..f89f943952e --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoRunEndLinks.test.ts @@ -0,0 +1,190 @@ +/** + * 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 + +// An undo that brings back a run gives it an end of this client's own where +// a closer ended it (closeRestoredRuns). It found where in one transaction +// and wrote the null in a second one, keeping the item objects it had found; +// the first transaction split items and merged them back, so the item to its +// left could be a piece no longer in the store. The null was linked to that +// piece: the item in the store kept its old right, so a walk of the list +// skipped the null (a broken link, on the undoing client only). Two clients, +// synced after every step, reduced from an undo fuzz: B deletes two +// characters; A comments the end of one line and part of another (one step) +// and bolds part of the first; B types " new " and deletes part of it, +// removes A's first comment and undoes that. + +import { + buildEditorFromExtensions, + getExtensionDependencyFromEditor, +} from '@lexical/extension'; +import {$addMarkID, $removeMarkID, MarkStateExtension} from '@lexical/mark'; +import {RichTextExtension} from '@lexical/rich-text'; +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $isElementNode, + $isTextNode, + $setSelection, + configExtension, + defineExtension, + type LexicalNode, + type TextNode, + UNDO_COMMAND, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; + +import {createYDoc, YCollabExtension} from '../../..'; +import {seedYState} from '../../../headless'; +import {misLinkedItems} from '../../utils/structure'; + +const EXTENSIONS = [RichTextExtension, MarkStateExtension]; +const WORDS = ['alpha', 'beta', 'gamma', 'delta', 'note', 'run']; + +type Op = {e: number; end?: boolean} & ( + | {k: 'add'; t: number; a: number; b: number; id: string} + | {k: 'remove'; id: string} + | {k: 'format'; t: number; a: number; b: number; f: 'bold'} + | {k: 'type'; t: number; a: number; s: string} + | {k: 'del'; t: number; a: number; b: number} + | {k: 'undo'} +); + +// The undo fuzz's text node indexes (modulo the text nodes there are). +const OPS: Op[] = [ + {a: 18, b: 20, e: 1, k: 'del', t: 1}, + {a: 25, b: 34, e: 0, id: 'c2', k: 'add', t: 13}, + {a: 6, b: 8, e: 0, end: true, id: 'c4', k: 'add', t: 1}, + {a: 21, b: 25, e: 0, f: 'bold', k: 'format', t: 0}, + {a: 19, e: 1, k: 'type', s: ' new ', t: 6}, + {a: 3, b: 5, e: 1, k: 'del', t: 6}, + {e: 1, id: 'c2', k: 'remove'}, + {e: 1, k: 'undo'}, +]; + +function $texts(): TextNode[] { + const out: TextNode[] = []; + const walk = (n: LexicalNode) => { + if ($isTextNode(n) && n.getType() === 'text') { + out.push(n); + } + if ($isElementNode(n)) { + n.getChildren().forEach(walk); + } + }; + walk($getRoot()); + return out; +} + +function $apply(op: Op) { + if (op.k === 'undo') { + return; + } + if (op.k === 'remove') { + $removeMarkID(op.id); + return; + } + const texts = $texts(); + const t = texts[op.t % Math.max(1, texts.length)]; + const size = t.getTextContentSize(); + const s = $createRangeSelection(); + const b = op.k === 'type' ? op.a : op.b; + s.anchor.set(t.getKey(), Math.min(op.a, size), 'text'); + s.focus.set(t.getKey(), Math.min(b, size), 'text'); + $setSelection(s); + if (op.k === 'add') { + $addMarkID(op.id, s); + } else if (op.k === 'format') { + s.formatText(op.f); + } else if (op.k === 'type') { + s.format = 0; + s.insertText(op.s); + } else { + s.removeText(); + } + $setSelection(null); +} + +describe('an undo that ends a restored run', () => { + for (const gc of [false, true]) { + test(`keeps the list's links whole (gc ${gc ? 'on' : 'off'})`, () => { + const seeded = seedYState( + {extensions: EXTENSIONS, schemaVersion: 1}, + () => { + const root = $getRoot().clear(); + WORDS.forEach((word, i) => + root.append( + $createParagraphNode().append( + $createTextNode(`${word} one two three four five ${i}`), + ), + ), + ); + }, + ); + const peers = [0, 1].map(i => { + const doc = createYDoc(seeded, {gc}); + const editor = buildEditorFromExtensions( + defineExtension({ + dependencies: [ + ...EXTENSIONS, + configExtension(YCollabExtension, { + captureTimeout: 2 ** 30, + doc, + schemaVersion: 1, + synced: true, + }), + ], + name: `run-end-links/${i}`, + onError: error => { + throw error; + }, + }), + ); + const {binding} = getExtensionDependencyFromEditor( + editor, + YCollabExtension, + ).output; + return {binding, doc, editor}; + }); + const sync = () => { + const [a, b] = peers; + Y.applyUpdate( + b.doc, + Y.encodeStateAsUpdate(a.doc, Y.encodeStateVector(b.doc)), + ); + Y.applyUpdate( + a.doc, + Y.encodeStateAsUpdate(b.doc, Y.encodeStateVector(a.doc)), + ); + }; + try { + for (const [i, op] of OPS.entries()) { + const {binding, editor} = peers[op.e]; + if (op.k === 'undo') { + editor.dispatchCommand(UNDO_COMMAND, undefined); + } else { + editor.update(() => $apply(op), {discrete: true}); + } + if (op.end) { + binding.undoManager?.stopCapturing(); + } + sync(); + for (const peer of peers) { + expect(misLinkedItems(peer.doc), `after step ${i}`).toEqual([]); + } + } + } finally { + peers.forEach(p => p.editor.dispose()); + } + }); + } +}); diff --git a/packages/lexical-y/src/__tests__/utils/structure.ts b/packages/lexical-y/src/__tests__/utils/structure.ts index 5428371cede..c2737287b89 100644 --- a/packages/lexical-y/src/__tests__/utils/structure.ts +++ b/packages/lexical-y/src/__tests__/utils/structure.ts @@ -9,11 +9,13 @@ import * as Y from '@y/y'; /** - * The items of `doc` whose left or right neighbour belongs to another shared - * type (`client:clock` of each). A well-formed document has none: such an item - * is linked into one type's list while it names another as its parent, which - * only the document that wrote it holds (every other client integrates it - * from its origin), and that document's reads then go wrong. + * The items of `doc` whose list links are broken (`client:clock` of each): a + * left or right neighbour that belongs to another shared type (or another key + * of a map), or that does not link back to the item. A well-formed document + * has none. An item linked into one type's list while it names another as its + * parent, or one a forward walk of its list skips, is held only by the + * document that wrote it (every other client integrates it from its origin), + * and that document's reads then go wrong. */ export function misLinkedItems(doc: Y.Doc): string[] { const out: string[] = []; @@ -22,9 +24,16 @@ export function misLinkedItems(doc: Y.Doc): string[] { if (!(s instanceof Y.Item)) { continue; } + const {left, right} = s; if ( - (s.left !== null && s.left.parent !== s.parent) || - (s.right !== null && s.right.parent !== s.parent) + (left !== null && + (left.parent !== s.parent || + left.parentSub !== s.parentSub || + left.right !== s)) || + (right !== null && + (right.parent !== s.parent || + right.parentSub !== s.parentSub || + right.left !== s)) ) { out.push(`${s.id.client}:${s.id.clock}`); } diff --git a/packages/lexical-y/src/internal/Binding.ts b/packages/lexical-y/src/internal/Binding.ts index 9eeb8ac86f8..5d7e57e18a7 100644 --- a/packages/lexical-y/src/internal/Binding.ts +++ b/packages/lexical-y/src/internal/Binding.ts @@ -24,6 +24,9 @@ import { encodeStateAsUpdate, encodeStateVector, findIndexSS, + getItemCleanStart, + type ID, + type IdSet, Item, iterateStructsByIdSet, Node as YNode, @@ -831,8 +834,8 @@ export class YBinding { private restoring: Set | null = null; /** Stored nodes the running undo/redo deletes (it inserted them). */ private removing: Set | null = null; - /** The items the running undo/redo deletes (see isKeptByUndo). */ - private undoing: Set | null = null; + /** The ids of the items the running undo/redo deletes (see isKeptByUndo). */ + private undoing: IdSet | null = null; private wasEditable = true; private undoCache = new WeakMap>(); private readonly afterTransaction: (tr: Transaction) => void; @@ -5445,7 +5448,7 @@ export class YBinding { if (!(it.content instanceof ContentFormat) || it.content.key !== key) { continue; } - if (this.undoing.has(it)) { + if (this.undoing.hasId(it.id)) { if (it.deleted) { return true; } @@ -5650,8 +5653,14 @@ export class YBinding { const doc = this._doc; const store = doc.store; const client = doc.clientID; - // Where to write each null: after `left`, before `right`. - const ends: [left: Item | null, right: Item | null, key: string][] = []; + // Where to write each null: after the item that ends at `leftEnd`, + // before the item that starts at `right` (null: at that end of the + // list). Ids, not items: the scan's + // transaction splits items and its cleanup merges them back, so an item + // object it saw can be a piece no longer in the store by the time the + // null is written (an item linked to such a piece breaks the list's + // links). + const ends: [leftEnd: ID | null, right: ID | null, key: string][] = []; doc.transact(tr => { const copies: [original: Item, copy: Item][] = []; iterateStructsByIdSet(tr, step.deletes, s => { @@ -5681,7 +5690,7 @@ export class YBinding { for (let it = copy.right; it !== null; it = it.right) { if (it === gone && it.deleted) { if (shown) { - ends.push([it, it.right, key]); + ends.push([it.lastId, it.right && it.right.id, key]); } break; } @@ -5695,7 +5704,7 @@ export class YBinding { it.id.client !== client && it.content.value === null ) { - ends.push([it.left, it, key]); + ends.push([it.left && it.left.lastId, it.id, key]); } break; } @@ -5707,10 +5716,35 @@ export class YBinding { }, this.scanOrigin); if (ends.length > 0) { doc.transact(tr => { - for (const [left, right, key] of ends) { - // In the type of the item it goes before, never next to an item - // of another type (see checkLinkable in Cursor). - if (left !== null && right !== null && left.parent !== right.parent) { + for (const [leftEnd, rightStart, key] of ends) { + // The items as the store holds them now, split at those ids. + const right = + rightStart === null ? null : getItemCleanStart(tr, rightStart); + let left: Item | null = null; + if (leftEnd !== null) { + const at = store.getItem(leftEnd); + if ( + at instanceof Item && + at.id.clock + at.length - 1 > leftEnd.clock + ) { + getItemCleanStart( + tr, + createID(leftEnd.client, leftEnd.clock + 1), + ); + } + const found = store.getItem(leftEnd); + left = found instanceof Item ? found : null; + } + // Still next to each other, in one type (never next to an item of + // another type: see checkLinkable in Cursor). + if ( + (rightStart !== null && !(right instanceof Item)) || + (leftEnd !== null && left === null) || + (left === null && right === null) || + (left !== null && left.right !== right) || + (left === null && right !== null && right.left !== null) || + (left !== null && right !== null && left.parent !== right.parent) + ) { continue; } const item = new Item( @@ -5744,7 +5778,6 @@ export class YBinding { } const names = new Set(); const removing = new Set(); - const undoing = new Set(); const isStored = (s: unknown): s is Item => { if (!(s instanceof Item) || !(s.content instanceof ContentType)) { return false; @@ -5759,9 +5792,6 @@ export class YBinding { } }); iterateStructsByIdSet(tr, top.inserts, s => { - if (s instanceof Item) { - undoing.add(s); - } if (isStored(s)) { removing.add(s); } @@ -5769,7 +5799,9 @@ export class YBinding { }, this.scanOrigin); this.restoring = names; this.removing = removing; - this.undoing = undoing; + // By id: this scan's transaction splits items and merges them back, so + // the item objects it saw need not be those the undo meets. + this.undoing = top.inserts; } // ------------------------------------------------------------------------- From c1c79e00fe06b73c7ca7bebadb53a5c64d5ef460 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Sat, 31 Oct 2026 21:38:35 -0700 Subject: [PATCH 2/6] [lexical-y] Chore: expectConverged fails on a write the binding had to recover from ## Description The harness's `expectConverged` accepted a recovered-write warning (a local change that could not be written, rewritten from the editor). A fix that only refused a bad write and let the recovery repair it would pass every converging test. It now fails on that warning, as it does on a self-check warning; no existing test converges after a recovery. Tests only. ## Test plan ### Before A recovered write passed `expectConverged`. ### After ``` $ LEXICAL_Y_GC=0 vitest --run --project unit packages/lexical-y Test Files 1 failed | 240 passed | 2 skipped (243) Tests 1 failed | 19230 passed | 304 expected fail | 130 skipped (19665) $ LEXICAL_Y_GC=1 vitest --run --project unit packages/lexical-y Test Files 1 failed | 240 passed | 2 skipped (243) Tests 1 failed | 19227 passed | 307 expected fail | 130 skipped (19665) $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/Headless.test.ts packages/lexical-y/src/__tests__/unit/DisposeMemory.test.ts # gc off, then on Tests 33 passed (33) Tests 33 passed (33) ``` The one failure in each full run is a memory-loop test (Headless gc off, DisposeMemory gc on) that exceeded its heap bound with 30 workers and other load on the machine; both pass alone. --- packages/lexical-y/src/__tests__/utils/harness.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/lexical-y/src/__tests__/utils/harness.ts b/packages/lexical-y/src/__tests__/utils/harness.ts index 8ed55925269..e00376f244d 100644 --- a/packages/lexical-y/src/__tests__/utils/harness.ts +++ b/packages/lexical-y/src/__tests__/utils/harness.ts @@ -478,6 +478,12 @@ export async function expectConverged(peers: Peer[], expected?: string) { for (const peer of peers) { expect(peer.errors).toEqual([]); expect(peer.warnings.filter(w => w.includes('self-check'))).toEqual([]); + // A write the binding could not store is recovered from (rewritten from + // the editor) and reported: never expected in a test that converges, as + // a recovery could hide the failure it recovered from. + expect( + peer.warnings.filter(w => w.includes('could not be written')), + ).toEqual([]); expect(misLinkedItems(peer.doc)).toEqual([]); expect(peer.describe()).toBe(first.describe()); expect(shape(peer)).toEqual(shape(first)); From 6eb35bc15f81ce957d121d9527784e70dec01f90 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Sat, 31 Oct 2026 21:44:58 -0700 Subject: [PATCH 3/6] [lexical-y] Chore: Name failing seeds in the structure fuzz; correct D96 on what other clients hold ## Description Review follow-ups to #9693, tests and docs only: - `StructureFuzz.test.ts` records the editors' warnings and expects none after each step. With `checkLinkable`, a regression of the run-end rule is refused and recovered from instead of stored, so the structure check alone stays empty and the warning only surfaced as an unhandled error; now the failing seeds are named. - D96, `Cursor.ts` (checkLinkable's comment) and `CrossContainerEnd.test.ts`: other clients take the stray item's parent from its origin and put it in the root's list, where it changes nothing; only the writer's document holds it wrong. ## Test plan ### Before With the run-end rule's same-type filter removed, the fuzz reported 200 passed plus unhandled errors. ### After ``` $ LEXICAL_Y_GC=0 vitest --run --project unit packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts Tests 200 passed (200) $ LEXICAL_Y_GC=1 (same file) Tests 200 passed (200) $ (same-type filter in changeEnd removed) LEXICAL_Y_GC=0 vitest --run --project unit packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts Tests 9 failed | 191 passed (200) ``` --- packages/lexical-y/DECISIONS.md | 2 +- packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts | 5 +++++ .../src/__tests__/unit/regressions/CrossContainerEnd.test.ts | 5 +++-- packages/lexical-y/src/internal/Cursor.ts | 5 +++-- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index a3f456a3350..f3cac507eb9 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -58,7 +58,7 @@ issues and docs that carry it. | 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. 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 integrated the item into the right list, 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] | +| 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] | | D25 | **Undo is Yjs's `UndoManager`**, scoped to the binding's root and this client's own edits (500 ms capture by default). It never deletes a container a collaborator has put content in (its `deleteFilter`). Do not combine it with `HistoryExtension`. | Collaborative undo must not revert other people's work; keeping a container whole once someone else wrote in it matches y-prosemirror's protected nodes and is cheap to maintain. The `UndoManager` was chosen on 10-23; keeping containers whole was confirmed on 10-30. | `DECIDED` | 10-30 | [#9400], [y.md: Limits][limits] | | D26 | **Undo favours people taking turns over concurrent races.** Where a fix makes undo correct for sequential edits at the cost of rare races between an undo and a simultaneous edit of the same text, the sequential case wins. | Taking turns is the common case; a race with a simultaneous edit of the same text is rare. Picked on 10-28, confirmed on 10-30. | `DECIDED` | 10-30 | [#9400] | diff --git a/packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts b/packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts index 459bc111d96..ecd30541c1e 100644 --- a/packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts +++ b/packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts @@ -152,6 +152,9 @@ function gen(seed: number): Op[] { for (let seed = FIRST; seed < FIRST + SEEDS; seed++) { test(`seed ${seed}: no item has a neighbour in another container (gc ${GC ? 'on' : 'off'})`, () => { const errors: unknown[] = []; + // A write the binding refuses is recovered from and only warned about: + // record the warnings so a failing seed is named. + const warnings: string[] = []; const docs = [0, 1].map(i => { const doc = createYDoc(null, {gc: GC}); doc.clientID = 1001 + i; @@ -171,6 +174,7 @@ for (let seed = FIRST; seed < FIRST + SEEDS; seed++) { ], name: `structure/${i}`, onError: e => errors.push(e), + onWarn: e => warnings.push(String(e && (e as Error).message)), }), ), ); @@ -191,6 +195,7 @@ for (let seed = FIRST; seed < FIRST + SEEDS; seed++) { for (const doc of docs) { expect(misLinkedItems(doc), `after step ${i}`).toEqual([]); } + expect(warnings, `after step ${i}`).toEqual([]); } expect(errors).toEqual([]); } finally { diff --git a/packages/lexical-y/src/__tests__/unit/regressions/CrossContainerEnd.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/CrossContainerEnd.test.ts index 9cd6f7fc40b..250919fa916 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/CrossContainerEnd.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/CrossContainerEnd.test.ts @@ -15,8 +15,9 @@ // in the paragraph before the table, and B recolors part of the cell. B's // write linked its end after A's item in the root while naming the cell as // its parent: B's own document held an item in the wrong list (every other -// client placed it in the cell), and later changes next to it made B's -// editor stop reading part of the document, for good. A run cannot end in +// client takes the parent from the item's origin and puts it in the root, +// where it changes nothing), and later changes next to it made B's editor +// stop reading part of the document, for good. A run cannot end in // another container: changeEnd only looks in the run's own, and no item is // written next to an item of another container. diff --git a/packages/lexical-y/src/internal/Cursor.ts b/packages/lexical-y/src/internal/Cursor.ts index b480e69c465..9abb3ca5f08 100644 --- a/packages/lexical-y/src/internal/Cursor.ts +++ b/packages/lexical-y/src/internal/Cursor.ts @@ -148,8 +148,9 @@ export const markFormatCloser = (item: Item): void => { * Refuse to link a new item next to `left` and `right` unless both are in * `parent`, the type it is written to. An item whose neighbour sits in * another type is malformed: its own document links it into that type's list - * while every other client integrates it into `parent`, and later reads of - * that document go wrong. A write that hits this throws, and is recovered + * while naming `parent` (every other client takes its parent from its origin, + * so only this document holds it wrong), and later reads of that document go + * wrong. A write that hits this throws, and is recovered * from (RECOVERED_WRITE_WARNING) instead of storing the item. */ function checkLinkable( From d7773fcd7e33350da2a1d40a3dc54813c47ce46b Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Sat, 31 Oct 2026 21:56:09 -0700 Subject: [PATCH 4/6] [lexical-y] Bug Fix: An undo restoring two values at one place ends each of them ## Description Review follow-up to this PR's run-end change: `closeRestoredRuns` resolves each saved gap again before writing a null, and skipped the gap when its two items were no longer next to each other. Where two keys need an end in the same gap, the second key found the first key's new null between them and skipped its own: A bolds and italicizes "word"; B removes both in one step; A's bold end is gone while its italic end stays; B undoes; once A's italic end goes too, italic ran on over " plain tail". The check now passes over the nulls this loop wrote, and each null goes right before the gap's right item, after those. No end is deduplicated by place, and none is assigned to a write. ## Test plan ### Before ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/UndoRunEndSharedGap.test.ts Tests 2 failed (2) ``` ### After ``` $ LEXICAL_Y_GC=0 vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/{UndoRunEndSharedGap,UndoRunEndLinks}.test.ts Tests 4 passed (4) $ LEXICAL_Y_GC=1 (same files) Tests 4 passed (4) $ LEXICAL_Y_GC=0 vitest --run --project unit packages/lexical-y Test Files 1 failed | 241 passed | 2 skipped (244) Tests 1 failed | 19232 passed | 304 expected fail | 130 skipped (19667) $ LEXICAL_Y_GC=1 vitest --run --project unit packages/lexical-y Test Files 242 passed | 2 skipped (244) Tests 19230 passed | 307 expected fail | 130 skipped (19667) $ tsc --noEmit exit 0 ``` The gc-off failure is `DisposeMemory.test.ts`'s heap bound under load from other runs on the machine; it passes alone. --- .../regressions/UndoRunEndSharedGap.test.ts | 103 ++++++++++++++++++ packages/lexical-y/src/internal/Binding.ts | 38 ++++++- 2 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/UndoRunEndSharedGap.test.ts diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoRunEndSharedGap.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoRunEndSharedGap.test.ts new file mode 100644 index 00000000000..0eb85bbfabd --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoRunEndSharedGap.test.ts @@ -0,0 +1,103 @@ +/** + * 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 + +// An undo that brings back two values ending at the same place writes an end +// of its own for each where a collaborator's end is gone (closeRestoredRuns). +// A bolds and italicizes "word"; B removes both in one step; A's bold end is +// deleted (by id, as the other restore-end tests do), its italic end stays; +// B undoes. The second key must not read the first key's new null as a +// change of the gap and skip its own: when A's italic end goes later, italic +// would run on over " plain tail". + +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createTextNode, + $getRoot, + $setSelection, + type TextNode, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {$block, $selectIn, freshReader, setup} from '../../utils/harness'; + +for (const gc of [false, true]) { + test(`an undo restoring two values at one place ends both (gc ${gc ? 'on' : 'off'})`, async () => { + const t = await setup( + () => + $getRoot().append( + $createParagraphNode().append($createTextNode('word plain tail')), + ), + {ids: [1001, 1002, 1003]}, + ); + t.peers.forEach(p => (p.doc.gc = gc)); + const $formats = () => + ($block([0]).getAllTextNodes() as TextNode[]).map(n => [ + n.getTextContent(), + n.getFormat(), + ]); + try { + t.a.update(() => { + const s = $selectIn([0], 0, 4); + s.formatText('bold', 1); + s.formatText('italic', 2); + $setSelection(null); + }); + await t.sync(); + let boldEnd: Y.ID | null = null; + let italicEnd: Y.ID | null = null; + for (let it = t.a.doc.get('lexical-y')._start; it; it = it.right) { + if ( + it.content instanceof Y.ContentFormat && + it.content.value === null + ) { + if (it.content.key === 'f:bold') { + boldEnd = it.id; + } else if (it.content.key === 'f:italic') { + italicEnd = it.id; + } + } + } + expect(boldEnd).not.toBeNull(); + expect(italicEnd).not.toBeNull(); + t.b.update(() => { + const s = $selectIn([0], 0, 4); + s.formatText('bold', 0); + s.formatText('italic', 0); + $setSelection(null); + }); + await t.sync(); + t.a.doc.transact(tr => + (t.a.doc.store.getItem(boldEnd!) as Y.Item).delete(tr), + ); + await t.sync(); + t.b.undo(); + await t.sync(); + t.a.doc.transact(tr => + (t.a.doc.store.getItem(italicEnd!) as Y.Item).delete(tr), + ); + await t.sync(); + for (const peer of t.peers) { + expect(peer.read($formats)).toEqual([ + ['word', 3], + [' plain tail', 0], + ]); + } + const fresh = await freshReader(t.b); + expect(fresh.read($formats)).toEqual([ + ['word', 3], + [' plain tail', 0], + ]); + fresh.stop(); + } finally { + t.peers.forEach(p => p.stop()); + } + }); +} diff --git a/packages/lexical-y/src/internal/Binding.ts b/packages/lexical-y/src/internal/Binding.ts index 5d7e57e18a7..84ca1f5d340 100644 --- a/packages/lexical-y/src/internal/Binding.ts +++ b/packages/lexical-y/src/internal/Binding.ts @@ -5716,6 +5716,10 @@ export class YBinding { }, this.scanOrigin); if (ends.length > 0) { doc.transact(tr => { + // The nulls this loop writes: two keys can need an end in the same + // gap, and the second must not take the first's null for a change + // of the gap. + const written = new Set(); for (const [leftEnd, rightStart, key] of ends) { // The items as the store holds them now, split at those ids. const right = @@ -5735,22 +5739,43 @@ export class YBinding { const found = store.getItem(leftEnd); left = found instanceof Item ? found : null; } - // Still next to each other, in one type (never next to an item of - // another type: see checkLinkable in Cursor). if ( (rightStart !== null && !(right instanceof Item)) || (leftEnd !== null && left === null) || (left === null && right === null) || - (left !== null && left.right !== right) || - (left === null && right !== null && right.left !== null) || (left !== null && right !== null && left.parent !== right.parent) ) { continue; } + // Still next to each other but for the nulls written here, in one + // type (never next to an item of another type: see checkLinkable + // in Cursor). The null goes right before `right`, after those. + let before: Item | null; + if (right !== null) { + let l = right.left; + while (l !== null && written.has(l)) { + l = l.left; + } + if (l !== left) { + continue; + } + before = right.left; + } else { + let last = left as Item; + let r = last.right; + while (r !== null && written.has(r)) { + last = r; + r = r.right; + } + if (r !== null) { + continue; + } + before = last; + } const item = new Item( createID(client, store.getClock(client)), - left, - left && left.lastId, + before, + before && before.lastId, right, right && right.id, (left ?? right)!.parent, @@ -5758,6 +5783,7 @@ export class YBinding { new ContentFormat(key, null as unknown as object), ); item.integrate(tr, 0); + written.add(item); markFormatCloser(item); } }, this.runEndOrigin); From 6aa9a74a1df3aec8a779d0d7975e3b061923fd00 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Sat, 31 Oct 2026 21:43:04 -0700 Subject: [PATCH 5/6] [lexical-y] Bug Fix: A write recovered from after it failed part way does not repeat what it stored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Stacked on #9696. When a local write throws part way, the binding rewrites the document from the editor and reports `@lexical/y/recovered-write`. That rewrite wrote a changed line through the keystroke fast path (`$writeTextEdit`), which writes the text edits Lexical recorded against the previous editor state. Where the failed write had already stored such an edit, the rewrite stored it again: the other client showed the inserted text twice, and the author's editor no longer matched its own document. The rewrite now skips the fast path and diffs each line against the document. The failure is not reachable through a known path; it is measured by injecting a throw (the second item a cursor writes) and affects every tree, including the commit before #9679. ## Test plan ### Before ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/RecoveredWriteRepeats.test.ts AssertionError: expected 'paragraph("one INSERT INSERT alpha be…' to be 'paragraph("one INSERT alpha beta") / …' // Object.is equality Tests 2 failed (2) ``` ### After ``` $ LEXICAL_Y_GC=0 vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/RecoveredWriteRepeats.test.ts Tests 2 passed (2) $ LEXICAL_Y_GC=1 (same file) Tests 2 passed (2) $ LEXICAL_Y_GC=0 vitest --run --project unit packages/lexical-y Test Files 1 failed | 241 passed | 2 skipped (244) Tests 1 failed | 19232 passed | 304 expected fail | 130 skipped (19667) $ LEXICAL_Y_GC=1 vitest --run --project unit packages/lexical-y Test Files 1 failed | 241 passed | 2 skipped (244) Tests 1 failed | 19229 passed | 307 expected fail | 130 skipped (19667) $ pnpm run ci-check exit 0 ``` The one failure in each full run is a memory-loop test (Headless gc off, DisposeMemory gc on) that exceeded its heap bound under load from other runs on the machine; both pass alone. --- .changeset/y-recovered-write-diff.md | 7 ++ .../regressions/RecoveredWriteRepeats.test.ts | 79 +++++++++++++++++++ packages/lexical-y/src/internal/Write.ts | 5 +- 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 .changeset/y-recovered-write-diff.md create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/RecoveredWriteRepeats.test.ts diff --git a/.changeset/y-recovered-write-diff.md b/.changeset/y-recovered-write-diff.md new file mode 100644 index 00000000000..c2e0be79613 --- /dev/null +++ b/.changeset/y-recovered-write-diff.md @@ -0,0 +1,7 @@ +--- +'@lexical/y': patch +--- + +[fix] A write that fails part way is recovered from without repeating what it already stored: the rewrite diffs each line against the document instead of writing the recorded keystrokes again + +Verify: when a local write throws after storing part of a change (reported as `@lexical/y/recovered-write`), every client shows the change once, and the author's editor matches its document. diff --git a/packages/lexical-y/src/__tests__/unit/regressions/RecoveredWriteRepeats.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/RecoveredWriteRepeats.test.ts new file mode 100644 index 00000000000..fa3dc41654e --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/RecoveredWriteRepeats.test.ts @@ -0,0 +1,79 @@ +/** + * 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 write that fails part way is recovered from: the document is rewritten +// from the editor. The rewrite wrote a changed line through the keystroke +// fast path, which writes the text edits Lexical recorded against the +// previous editor state; where the failed write had already stored such an +// edit, the rewrite stored it again: the other client showed the inserted +// text twice, and the author's editor no longer matched its own document. +// The rewrite diffs each line against the document instead. The failure is +// injected (the second item a cursor writes throws). + +import {$getRoot} from 'lexical'; +import {expect, test} from 'vitest'; + +import {Cursor} from '../../../internal/Cursor'; +import {$p, $selectIn, freshReader, setup} from '../../utils/harness'; + +for (const ids of [ + [1, 2], + [2, 1], +]) { + test(`the rewrite after a failed write does not repeat what it stored (ids ${ids})`, async () => { + const t = await setup( + () => + $getRoot().append( + $p('one alpha beta'), + $p('two gamma delta'), + $p('untouched words here'), + ), + {ids}, + ); + const proto = Cursor.prototype as unknown as { + newItem: (...args: unknown[]) => unknown; + }; + const newItem = proto.newItem; + let calls = 0; + proto.newItem = function (this: unknown, ...args: unknown[]) { + if (++calls === 2) { + throw new Error('injected failure'); + } + return newItem.apply(this, args); + }; + try { + t.a.update(() => { + $selectIn([0], 3).insertText(' INSERT'); + $selectIn([1], 3).insertText(' CHANGE'); + }); + } finally { + proto.newItem = newItem; + } + expect( + t.a.warnings.filter(w => w.includes('could not be written')), + ).toHaveLength(1); + await t.sync(); + const expected = + 'paragraph("one INSERT alpha beta") / paragraph("two CHANGE gamma delta") / paragraph("untouched words here")'; + for (const peer of t.peers) { + expect(peer.describe()).toBe(expected); + const fresh = await freshReader(peer); + expect(fresh.describe()).toBe(expected); + fresh.stop(); + } + t.a.undo(); + await t.sync(); + for (const peer of t.peers) { + expect(peer.describe()).toBe( + 'paragraph("one alpha beta") / paragraph("two gamma delta") / paragraph("untouched words here")', + ); + } + }); +} diff --git a/packages/lexical-y/src/internal/Write.ts b/packages/lexical-y/src/internal/Write.ts index 9dec2ec4a72..88e5be75cf8 100644 --- a/packages/lexical-y/src/internal/Write.ts +++ b/packages/lexical-y/src/internal/Write.ts @@ -2956,7 +2956,10 @@ export class Writer { /** Rewrite one line (its content and marker) on its own. */ $writeLineAlone(u: Unit, d: Desired): void { - if (this.$writeTextEdit(u, d)) { + // A rewrite after a failed write diffs the line against the document: + // the edits Lexical recorded may be stored already (the failed write + // stored part of its change), and writing them again would repeat them. + if (!this.rewriteAll && this.$writeTextEdit(u, d)) { return; } const old = u.cs.units; From 195d7502ff3219073890a3d9c0fae971a083a23e Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Sat, 31 Oct 2026 22:45:03 -0700 Subject: [PATCH 6/6] [lexical-y][lexical-website] Documentation Update: Record that a recovered write can be two undo steps (D97, open) ## Description The failed write and its rewrite are two transactions, so a rewrite that takes longer than the undo manager's `captureTimeout` (a very long line) lands as its own undo step. Suspending the capture timeout during the rewrite was tried and dropped in review: it merged the rewrite into an app's own step where the app wrote between the two, and did nothing for an app's `UndoManager` tracking the same origin. Recorded as D97 (`OPEN`: accept as a limit, or associate the two at the transaction level), with one sentence in y.md's Limits (Undo), and pinned as `test.fails` naming D97. ## Test plan ### Before Not recorded. ### After ``` $ LEXICAL_Y_GC=0 vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/RecoveredWriteRepeats.test.ts Tests 2 passed | 2 expected fail (4) $ LEXICAL_Y_GC=1 (same file) Tests 2 passed | 2 expected fail (4) ``` The pinned cell uses a capture timeout of 0, which stands for a rewrite slower than any timeout (the same window as a million-character line at 20 ms), so it does not depend on the machine's speed. --- .../lexical-website/docs/collaboration/y.md | 6 ++- packages/lexical-y/DECISIONS.md | 2 + .../regressions/RecoveredWriteRepeats.test.ts | 46 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/packages/lexical-website/docs/collaboration/y.md b/packages/lexical-website/docs/collaboration/y.md index 180273ed710..7b45e4f3e84 100644 --- a/packages/lexical-website/docs/collaboration/y.md +++ b/packages/lexical-website/docs/collaboration/y.md @@ -1324,7 +1324,11 @@ not merge as a user might expect: `Kilo` bolded by one client, another unbolds `il` or types plain text inside `Kilo` as the first or a third bolds ` bb` (or `lo bb`), and with some client id orders ` bb` stays plain. No text is lost, and when that - client undoes its edit, ` bb` is bold again. + client undoes its edit, ` bb` is bold again. A local change the binding + could not write and recovered from (reported as + `@lexical/y/recovered-write`) can land as two undo steps: what the failed + write stored, and its rewrite, when the rewrite takes longer than the + undo manager's `captureTimeout` (a very long line). - **Large lists.** A structural edit in a list costs time proportional to the list on every client. - **Older app versions.** Clients of different `schemaVersion`s never diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index f3cac507eb9..db3a7c2e0f9 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -132,6 +132,7 @@ issues and docs that carry it. | ID | Question | Where it stands | Status | Date | Links | | --- | -------- | --------------- | ------ | ---- | ----- | +| D97 | **A recovered write can be two undo steps.** When a local change cannot be written part way and the binding rewrites the document from the editor (`RECOVERED_WRITE_WARNING`), what the failed write stored and the rewrite are two transactions; a rewrite that takes longer than the undo manager's `captureTimeout` (a line of about a million characters at 20 ms) lands as its own undo step. | Joining them by suspending the capture timeout merged the rewrite into an app's own step where the app wrote between the two, and did nothing for an app's `UndoManager` tracking the same origin ([#9697] review). Accept as a limit (documented in [Limits][limits]), or associate the two at the transaction level. | `OPEN` | 11-01 | [#9697], [limits] | | D59 | **Block containers in the node structure.** A container that is not a shadow root counts as one block, so `$insertList` replaces it; a box inside a box loses the outer box. | The rule should live in `NodeStructureExtension`. Draft in progress. | `OPEN` | 10-30 | [#9570] | | D60 | **Code highlighting as display-only.** Today highlighting is stored token nodes, so code written without the highlighter opens plain until edited (D21). | Right long term, but intrusive; documented for now. | `OPEN` | 10-30 | [#9536], [#9558] | | D61 | **Two concurrent joins into one line** both write its type; Yjs keeps one by client id. | Inherent to the marker layout; a real trade-off. | `OPEN` | 10-30 | [#9511] | @@ -236,3 +237,4 @@ issues and docs that carry it. [yjs#382]: https://github.com/yjs/yjs/issues/382 [#9679]: https://github.com/meta/lexical/pull/9679 [#9694]: https://github.com/meta/lexical/issues/9694 +[#9697]: https://github.com/meta/lexical/pull/9697 diff --git a/packages/lexical-y/src/__tests__/unit/regressions/RecoveredWriteRepeats.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/RecoveredWriteRepeats.test.ts index fa3dc41654e..681508b9428 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/RecoveredWriteRepeats.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/RecoveredWriteRepeats.test.ts @@ -77,3 +77,49 @@ for (const ids of [ } }); } + +// The failed write and its rewrite are two transactions: a rewrite slower +// than the undo manager's capture timeout (a very long line) makes one edit +// two undo steps. An open question (D97): accept it as a limit, or join the +// two at the transaction level. A capture timeout of 0 stands for a rewrite +// slower than any timeout. +for (const ids of [ + [1, 2], + [2, 1], +]) { + test.fails( + `D97: the rewrite after a failed write is in the same undo step (ids ${ids})`, + async () => { + const t = await setup( + () => $getRoot().append($p('one alpha beta'), $p('two gamma delta')), + {ids}, + ); + const um = t.a.binding.undoManager!; + um.captureTimeout = 0; + const steps = um.undoStack.length; + const proto = Cursor.prototype as unknown as { + newItem: (...args: unknown[]) => unknown; + }; + const newItem = proto.newItem; + let calls = 0; + proto.newItem = function (this: unknown, ...args: unknown[]) { + if (++calls === 2) { + throw new Error('injected failure'); + } + return newItem.apply(this, args); + }; + try { + t.a.update(() => { + $selectIn([0], 3).insertText(' INSERT'); + $selectIn([1], 3).insertText(' CHANGE'); + }); + } finally { + proto.newItem = newItem; + } + expect( + t.a.warnings.filter(w => w.includes('could not be written')), + ).toHaveLength(1); + expect(um.undoStack.length).toBe(steps + 1); + }, + ); +}