From 52f79291f7636ddb408c2ec7bec2c0095572fc3e Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Sat, 31 Oct 2026 21:09:04 -0700 Subject: [PATCH 1/2] [lexical-y] Bug Fix: A run ends in its own container, and no item is linked next to an item of another container ## Description A format write that restates another client's run at the end of its range gives that run an end of its own, after the end that client's change gave it (#9679, rule 3). That end came from `changeEnd`, which walks the client's items in clock order and could return an item in another container (a write that colored text in a table cell and then in the root). The writer then linked its new item after that item, into the root's list, while naming the cell as its parent. Every other client integrated the item into the cell, but the writer's own document held a malformed item: later changes next to it made the writer's editor stop reading part of the document, for good and without a warning (a downstream app's long notes: 15 of 133 sessions on `main`, 0 of 132 before #9679). Now: - `changeEnd` and `lastOwnItemBefore` only look at items in the run's own shared type: a run cannot end in another container. - The cursor refuses to link a new item next to an item of another type (`checkLinkable` in `newItem` and `insertAfter`, in every build): the write throws, and the existing recovery rewrites the document from the editor and reports `@lexical/y/recovered-write`, instead of storing a malformed item. `closeRestoredRuns` skips such a pair. - Tests check the structure: `misLinkedItems` (no item with a neighbour in another type) runs in the harness's `expectConverged`, after every op of `UndoSpreadFuzz`, and in a new structure fuzz. Recorded as D96 (its issue link is a placeholder until the issue is filed). ## Test plan ### Before ``` $ LEXICAL_Y_GC=0 vitest --run --project unit packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts packages/lexical-y/src/__tests__/unit/regressions/CrossContainerEnd.test.ts Tests 11 failed | 191 passed (202) AssertionError: expected [ '6414644327309859:22', '1:5', '2:2' ] to deeply equal [] $ LEXICAL_Y_GC=1 (same files) Tests 11 failed | 191 passed (202) ``` ### After ``` $ LEXICAL_Y_GC=0 vitest --run --project unit packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts packages/lexical-y/src/__tests__/unit/regressions/CrossContainerEnd.test.ts Tests 202 passed (202) $ LEXICAL_Y_GC=1 (same files) Tests 202 passed (202) $ LEXICAL_Y_GC=0 vitest --run --project unit packages/lexical-y Test Files 240 passed | 2 skipped (242) Tests 19229 passed | 304 expected fail | 130 skipped (19663) $ LEXICAL_Y_GC=1 vitest --run --project unit packages/lexical-y Tests 19226 passed | 307 expected fail | 130 skipped (19663) $ pnpm run ci-check exit 0 ``` With gc on, `DisposeMemory.test.ts` exceeded its heap bound once with 40 workers in parallel; it passes alone (three runs). The 1,000-seed version of the structure fuzz: 48 (gc off) and 49 (gc on) seeds failing before, 0 after. --- .changeset/y-run-end-own-container.md | 7 + packages/lexical-y/DECISIONS.md | 3 + .../src/__tests__/unit/StructureFuzz.test.ts | 201 ++++++++++++++++++ .../src/__tests__/unit/UndoSpreadFuzz.test.ts | 5 + .../regressions/CrossContainerEnd.test.ts | 131 ++++++++++++ .../lexical-y/src/__tests__/utils/harness.ts | 2 + .../src/__tests__/utils/structure.ts | 34 +++ packages/lexical-y/src/internal/Binding.ts | 5 + packages/lexical-y/src/internal/Cursor.ts | 43 +++- 9 files changed, 426 insertions(+), 5 deletions(-) create mode 100644 .changeset/y-run-end-own-container.md create mode 100644 packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/CrossContainerEnd.test.ts create mode 100644 packages/lexical-y/src/__tests__/utils/structure.ts diff --git a/.changeset/y-run-end-own-container.md b/.changeset/y-run-end-own-container.md new file mode 100644 index 00000000000..eda16339f87 --- /dev/null +++ b/.changeset/y-run-end-own-container.md @@ -0,0 +1,7 @@ +--- +'@lexical/y': patch +--- + +[fix] A format write never links an item into another container: an editor no longer stops matching its own document after a style change next to a collaborator's run in a table cell + +Verify: in a document with a table, one client colors the end of a cell's text, clears the color of one character there, then colors a word in the paragraph before the table; after syncing, the other client recolors part of that cell. Both editors keep matching their documents through later edits and syncs, and no `@lexical/y/recovered-write` warning is reported. diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index 8922684bc31..29d56eec7eb 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -58,6 +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 | [#NNNN] | | 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] | @@ -226,3 +227,5 @@ issues and docs that carry it. [#9681]: https://github.com/meta/lexical/issues/9681 [#9682]: https://github.com/meta/lexical/issues/9682 [yjs#382]: https://github.com/yjs/yjs/issues/382 +[#9679]: https://github.com/meta/lexical/pull/9679 +[#NNNN]: https://github.com/meta/lexical/issues/NNNN diff --git a/packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts b/packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts new file mode 100644 index 00000000000..459bc111d96 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/StructureFuzz.test.ts @@ -0,0 +1,201 @@ +/** + * 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 + +// Two clients color ranges of a document with a table (its cells are +// containers of their own) and sync now and then. After every step, no item +// of either document may have a neighbour in another container (see +// misLinkedItems): a write that ended a run after its client's item in +// another container linked its item into the wrong list +// (regressions/CrossContainerEnd.test.ts). Seeds: LEXICAL_Y_STRUCTURE_SEEDS +// (default 200) from LEXICAL_Y_STRUCTURE_FIRST (default 1); LEXICAL_Y_GC=1 +// for garbage collection on. + +import {buildEditorFromExtensions} from '@lexical/extension'; +import {MarkStateExtension} from '@lexical/mark'; +import {RichTextExtension} from '@lexical/rich-text'; +import {$patchStyleText} from '@lexical/selection'; +import { + $createTableCellNode, + $createTableNode, + $createTableRowNode, + TableExtension, +} from '@lexical/table'; +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $isElementNode, + $isParagraphNode, + $setSelection, + configExtension, + defineExtension, + type ElementNode, + type LexicalNode, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {createYDoc, YCollabExtension} from '../..'; +import {seedYState} from '../../headless'; +import {misLinkedItems} from '../utils/structure'; + +const GC = process.env.LEXICAL_Y_GC === '1'; +const SEEDS = Number(process.env.LEXICAL_Y_STRUCTURE_SEEDS ?? 200); +const FIRST = Number(process.env.LEXICAL_Y_STRUCTURE_FIRST ?? 1); +const STEPS = 40; +const EXTENSIONS = [RichTextExtension, TableExtension, MarkStateExtension]; +const COLORS = ['#c00', '#0a0', '#00c']; + +type Op = + | {k: 'sync'} + | {k: 'color'; e: number; p: number; a: number; b: number; v: number}; + +function rng(seed: number) { + let s = seed >>> 0 || 1; + return () => { + s ^= s << 13; + s ^= s >>> 17; + s ^= s << 5; + return (s >>> 0) / 4294967296; + }; +} + +const SEEDED = seedYState({extensions: EXTENSIONS, schemaVersion: 1}, () => { + const $cell = (text: string) => + $createTableCellNode(0).append( + $createParagraphNode().append($createTextNode(text)), + ); + $getRoot() + .clear() + .append( + $createParagraphNode().append($createTextNode('alpha beta gamma')), + $createTableNode().append( + $createTableRowNode().append( + $cell('one two three'), + $cell('four five six'), + ), + ), + $createParagraphNode().append($createTextNode('delta epsilon zeta')), + ); +}); + +function $paragraphs(): ElementNode[] { + const out: ElementNode[] = []; + const walk = (n: LexicalNode) => { + if ($isParagraphNode(n)) { + out.push(n); + } else if ($isElementNode(n)) { + n.getChildren().forEach(walk); + } + }; + walk($getRoot()); + return out; +} + +function $color(op: Extract) { + const paragraphs = $paragraphs(); + const texts = paragraphs[op.p % paragraphs.length].getAllTextNodes(); + const total = texts.reduce((n, t) => n + t.getTextContentSize(), 0); + if (total === 0) { + return; + } + const at = (offset: number) => { + let left = Math.max(0, Math.min(offset, total)); + for (const t of texts) { + const n = t.getTextContentSize(); + if (left <= n) { + return {o: left, t}; + } + left -= n; + } + const last = texts[texts.length - 1]; + return {o: last.getTextContentSize(), t: last}; + }; + const from = Math.min(op.a, total - 1); + const x = at(from); + const y = at(op.b >= 99 ? total : Math.min(total, from + op.b)); + const selection = $createRangeSelection(); + selection.anchor.set(x.t.getKey(), x.o, 'text'); + selection.focus.set(y.t.getKey(), y.o, 'text'); + $setSelection(selection); + $patchStyleText(selection, {color: op.v < 3 ? COLORS[op.v] : null}); + $setSelection(null); +} + +function gen(seed: number): Op[] { + const r = rng(seed * 2654435761 + 5); + const ops: Op[] = []; + for (let s = 0; s < STEPS; s++) { + ops.push({ + a: Math.floor(r() * 14), + b: r() < 0.35 ? 99 : 1 + Math.floor(r() * 8), + e: Math.floor(r() * 2), + k: 'color', + p: Math.floor(r() * 4), + v: Math.floor(r() * 4), + }); + if (r() < 0.6) { + ops.push({k: 'sync'}); + } + } + return ops; +} + +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[] = []; + const docs = [0, 1].map(i => { + const doc = createYDoc(null, {gc: GC}); + doc.clientID = 1001 + i; + Y.applyUpdate(doc, SEEDED); + return doc; + }); + const editors = docs.map((doc, i) => + buildEditorFromExtensions( + defineExtension({ + dependencies: [ + ...EXTENSIONS, + configExtension(YCollabExtension, { + doc, + schemaVersion: 1, + synced: true, + }), + ], + name: `structure/${i}`, + onError: e => errors.push(e), + }), + ), + ); + try { + for (const [i, op] of gen(seed).entries()) { + if (op.k === 'sync') { + Y.applyUpdate( + docs[1], + Y.encodeStateAsUpdate(docs[0], Y.encodeStateVector(docs[1])), + ); + Y.applyUpdate( + docs[0], + Y.encodeStateAsUpdate(docs[1], Y.encodeStateVector(docs[0])), + ); + } else { + editors[op.e].update(() => $color(op), {discrete: true}); + } + for (const doc of docs) { + expect(misLinkedItems(doc), `after step ${i}`).toEqual([]); + } + } + expect(errors).toEqual([]); + } finally { + editors.forEach(e => e.dispose()); + docs.forEach(d => d.destroy()); + } + }); +} diff --git a/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts b/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts index 6998f325834..b4f4a9c5008 100644 --- a/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts +++ b/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts @@ -48,6 +48,7 @@ import {expect, test} from 'vitest'; import {createYDoc, YCollabExtension} from '../..'; import {seedYState} from '../../headless'; +import {misLinkedItems} from '../utils/structure'; const SEEDS = Number(process.env.US_SEEDS ?? 60); const START = Number(process.env.US_START ?? 1); @@ -262,6 +263,10 @@ for (let seed = START; seed < START + SEEDS; seed++) { spread(peer.editor), `after op ${i}: ${JSON.stringify(ops.slice(0, i + 1))}`, ).toEqual([]); + expect( + misLinkedItems(peer.doc), + `after op ${i}: ${JSON.stringify(ops.slice(0, i + 1))}`, + ).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 new file mode 100644 index 00000000000..9cd6f7fc40b --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/CrossContainerEnd.test.ts @@ -0,0 +1,131 @@ +/** + * 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 format write that restates another client's run at the end of its range +// gives that run an end of its own, after the end that client's change gave +// it (see changeEnd). That client's next item of the key, in its clock order, +// can be in another container: here A colors text in a table cell and then +// 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 +// another container: changeEnd only looks in the run's own, and no item is +// written next to an item of another container. + +import {$patchStyleText} from '@lexical/selection'; +import { + $createTableCellNode, + $createTableNode, + $createTableRowNode, +} from '@lexical/table'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $isElementNode, + $isParagraphNode, + $setSelection, + type ElementNode, + type LexicalNode, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; + +import {expectConverged, setup} from '../../utils/harness'; +import {misLinkedItems} from '../../utils/structure'; + +function $paragraphs(): ElementNode[] { + const out: ElementNode[] = []; + const walk = (n: LexicalNode) => { + if ($isParagraphNode(n)) { + out.push(n); + } else if ($isElementNode(n)) { + n.getChildren().forEach(walk); + } + }; + walk($getRoot()); + return out; +} + +/** + * Color offsets `from` to `to` of paragraph `p` (0: the first paragraph, 1 + * and 2: the cells, 3: the last paragraph); null clears the color. + */ +function $color( + p: number, + from: number, + to: number | 'end', + color: string | null, +) { + const texts = $paragraphs()[p].getAllTextNodes(); + const total = texts.reduce((n, t) => n + t.getTextContentSize(), 0); + const at = (offset: number) => { + let left = offset; + for (const t of texts) { + const n = t.getTextContentSize(); + if (left <= n) { + return {o: left, t}; + } + left -= n; + } + const last = texts[texts.length - 1]; + return {o: last.getTextContentSize(), t: last}; + }; + const x = at(from); + const y = at(to === 'end' ? total : to); + const selection = $createRangeSelection(); + selection.anchor.set(x.t.getKey(), x.o, 'text'); + selection.focus.set(y.t.getKey(), y.o, 'text'); + $setSelection(selection); + $patchStyleText(selection, {color}); + $setSelection(null); +} + +describe('a style write next to a run whose client wrote in another container', () => { + for (const ids of [ + [1, 2], + [2, 1], + ]) { + test(`links no item into another container (ids ${ids})`, async () => { + const $cell = (text: string) => + $createTableCellNode(0).append( + $createParagraphNode().append($createTextNode(text)), + ); + const t = await setup( + () => + $getRoot().append( + $createParagraphNode().append($createTextNode('alpha beta gamma')), + $createTableNode().append( + $createTableRowNode().append( + $cell('one two three'), + $cell('four five six'), + ), + ), + $createParagraphNode().append( + $createTextNode('delta epsilon zeta'), + ), + ), + {ids}, + ); + t.a.update(() => $color(2, 1, 'end', '#00c')); + t.a.update(() => $color(2, 7, 8, null)); + t.a.update(() => $color(0, 13, 17, '#00c')); + await t.sync(); + t.b.update(() => $color(2, 4, 8, '#0a0')); + for (const peer of t.peers) { + expect(misLinkedItems(peer.doc)).toEqual([]); + expect(peer.warnings).toEqual([]); + } + await t.sync(); + await expectConverged(t.peers); + }); + } +}); diff --git a/packages/lexical-y/src/__tests__/utils/harness.ts b/packages/lexical-y/src/__tests__/utils/harness.ts index 247cfc03a91..8ed55925269 100644 --- a/packages/lexical-y/src/__tests__/utils/harness.ts +++ b/packages/lexical-y/src/__tests__/utils/harness.ts @@ -51,6 +51,7 @@ import { import {expect} from 'vitest'; import {createYDoc, YBinding, YCollabExtension, type YDoc} from '../..'; +import {misLinkedItems} from './structure'; // An undeclared block container with a schema field ('node'). const boxSchema = nodeSchema()({ @@ -477,6 +478,7 @@ 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([]); + expect(misLinkedItems(peer.doc)).toEqual([]); expect(peer.describe()).toBe(first.describe()); expect(shape(peer)).toEqual(shape(first)); } diff --git a/packages/lexical-y/src/__tests__/utils/structure.ts b/packages/lexical-y/src/__tests__/utils/structure.ts new file mode 100644 index 00000000000..5428371cede --- /dev/null +++ b/packages/lexical-y/src/__tests__/utils/structure.ts @@ -0,0 +1,34 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +import * 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. + */ +export function misLinkedItems(doc: Y.Doc): string[] { + const out: string[] = []; + for (const structs of doc.store.clients.values()) { + for (const s of structs) { + if (!(s instanceof Y.Item)) { + continue; + } + if ( + (s.left !== null && s.left.parent !== s.parent) || + (s.right !== null && s.right.parent !== s.parent) + ) { + out.push(`${s.id.client}:${s.id.clock}`); + } + } + } + return out; +} diff --git a/packages/lexical-y/src/internal/Binding.ts b/packages/lexical-y/src/internal/Binding.ts index 74dedd91d95..9eeb8ac86f8 100644 --- a/packages/lexical-y/src/internal/Binding.ts +++ b/packages/lexical-y/src/internal/Binding.ts @@ -5708,6 +5708,11 @@ export class YBinding { 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) { + continue; + } const item = new Item( createID(client, store.getClock(client)), left, diff --git a/packages/lexical-y/src/internal/Cursor.ts b/packages/lexical-y/src/internal/Cursor.ts index abb390322fd..b480e69c465 100644 --- a/packages/lexical-y/src/internal/Cursor.ts +++ b/packages/lexical-y/src/internal/Cursor.ts @@ -144,11 +144,36 @@ export const markFormatCloser = (item: Item): void => { formatClosers.add(item); }; +/** + * 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 + * from (RECOVERED_WRITE_WARNING) instead of storing the item. + */ +function checkLinkable( + left: Item | null, + right: Item | null, + parent: Item['parent'], +): void { + if ( + (left !== null && left.parent !== parent) || + (right !== null && right.parent !== parent) + ) { + throw new Error( + 'refused to write an item next to an item of another shared type', + ); + } +} + /** * The item that ends the run `item`, a format item, opened in the write that - * made it: the first item of its key, with another value, that its client - * made after it (one write's items are one after the other in its client's - * clock order; looked for among the next 64). Null when none is found. + * made it: the first item of its key, with another value, in the same shared + * type, that its client made after it (one write's items are one after the + * other in its client's clock order; looked for among the next 64). A run + * cannot end in another type: a write's items in other containers are + * passed over. Null when none is found. */ export function changeEnd(doc: Doc, item: Item): Item | null { const {key, value} = item.content as ContentFormat; @@ -161,6 +186,7 @@ export function changeEnd(doc: Doc, item: Item): Item | null { return null; } if ( + next.parent === item.parent && next.content instanceof ContentFormat && next.content.key === key && !equalFormatValue(next.content.value, value) @@ -174,7 +200,8 @@ export function changeEnd(doc: Doc, item: Item): Item | null { /** * The item its client made last before `item` (looked for among the 64 - * structs before it, in clock order) of the key `key`, or null. + * structs before it, in clock order) of the key `key` in the same shared + * type, or null. */ function lastOwnItemBefore(doc: Doc, item: Item, key: string): Item | null { const {client} = item.id; @@ -184,7 +211,11 @@ function lastOwnItemBefore(doc: Doc, item: Item, key: string): Item | null { if (!(prev instanceof Item)) { return null; } - if (prev.content instanceof ContentFormat && prev.content.key === key) { + if ( + prev.parent === item.parent && + prev.content instanceof ContentFormat && + prev.content.key === key + ) { return prev; } clock = prev.id.clock - 1; @@ -446,6 +477,7 @@ export class Cursor { const client = doc.clientID; const left = this.left; const right = this.right; + checkLinkable(left, right, this.parent); const item = new Item( createID(client, doc.store.getClock(client)), left, @@ -782,6 +814,7 @@ export class Cursor { private insertAfter(left: Item, content: ContentFormat): Item { const doc = this.tr.doc; const right = left.right; + checkLinkable(left, right, this.parent); const item = new Item( createID(doc.clientID, doc.store.getClock(doc.clientID)), left, From 294b34e68ed0a8ae2937ba06625ccfbcf03fef92 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Sat, 31 Oct 2026 21:15:03 -0700 Subject: [PATCH 2/2] [lexical-y] Documentation Update: Link D96 to #9694 ## Description D96 links its issue, #9694 (it named a placeholder). Documentation only. ## Test plan ### Before D96 linked a placeholder issue. ### After ``` $ prettier --check packages/lexical-y/DECISIONS.md All matched files use Prettier code style! ``` --- packages/lexical-y/DECISIONS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index 29d56eec7eb..0eed11b4242 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 | [#NNNN] | +| 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] | | 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] | @@ -228,4 +228,4 @@ issues and docs that carry it. [#9682]: https://github.com/meta/lexical/issues/9682 [yjs#382]: https://github.com/yjs/yjs/issues/382 [#9679]: https://github.com/meta/lexical/pull/9679 -[#NNNN]: https://github.com/meta/lexical/issues/NNNN +[#9694]: https://github.com/meta/lexical/issues/9694