diff --git a/.changeset/y-undo-restored-run-ends.md b/.changeset/y-undo-restored-run-ends.md new file mode 100644 index 00000000000..916a920548f --- /dev/null +++ b/.changeset/y-undo-restored-run-ends.md @@ -0,0 +1,7 @@ +--- +'@lexical/y': patch +--- + +[fix] An undo that brings back a comment, format, link, style or NodeState value ends it where it ended before, also when the change that wrote that end was undone before it or is undone at the same time + +Verify: undo a collaborator's removal of a comment (or a format) after its author undid it, or while the author undoes it: the value comes back on its own text only, or not at all, instead of covering everything after it. Each such undo writes one format item of its own. diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index 5393d6811fd..e8240fb123c 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. 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] | | 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] | +| D83 | **An undo that brings back a value brings back its end.** Where an undo or redo restores a format item that opens a value (a format, link, style, NodeState value or comment id), it ends the run with a null of its own where the run ended when the item was deleted (the end is noted at the delete, as Yjs may collect the deleted end's content), and before another change's live end of the run even where no content shows between them yet. A value an undo restores no longer runs past the place it ended at the delete when the change that wrote that end is undone, before or at the same time. | The restored item relied on another change's end, and the undo of that change (sequential: [#9623]; concurrent: [#9620]) left the value open over everything after it. Written inside the local undo, never on a remote change (D20). Cost: one null per restored run that another change's end closed, also where nothing shows yet. A null by which a change made text inside the run plain (a hole its client restores the value after) is not taken for the end. The undo of a write whose end a live run still relies on is [#9624] (D26's sequential case), and runs a collaborator's write relies on while their writer undoes them at the same time stay open ([#9599], [#9622]). | `DECIDED` | 10-30 | [#9623], [#9620] | | D27 | **Moves are a delete and a re-insert** at launch; the binding rewrites the cheaper side. Real moves within a container ([#9449], records after Yjs's unmerged `ContentMove` design) are paused and land last, backward compatible: a new layout version that older clients open read only. Move records leave tombstones, as every Yjs binding does. Moves across containers stay copies. | Every Yjs binding turns moves into delete plus insert; Yjs dropped its own move for complexity. Reliability of the rest came first. | `DECIDED` | 10-25 | [#9449], [#9455], [y.md: Limits][limits] | | D28 | **Text identity is fixed at the source.** While a binding is attached, core records text edits (`spliceText`, insert and remove, split and merge), and text entity plugins record a grow as a move and a remake as a retype. Text only `setTextContent` or the DOM changed is diffed. The binding adds no per-case heuristics. | No binding can recover intent an API dropped; special-case rules were fragile and regressed neighbouring cases. | `DECIDED` | 10-27 | [#9483], [y.md: Edits made by code][y-code] | | D29 | **Remade text keeps its common start and end by character**, whatever the node boundaries (y-prosemirror's `updateYText` rule), and never brings back a collaborator's delete. It replaces the word matching of [#9565]. | It is the industry-standard behaviour, and word matching glued words together and merged numbers oddly. | `DECIDED` | 10-30 | [#9565] | @@ -198,3 +199,7 @@ issues and docs that carry it. [#9608]: https://github.com/meta/lexical/issues/9608 [#9609]: https://github.com/meta/lexical/issues/9609 [#9610]: https://github.com/meta/lexical/issues/9610 +[#9620]: https://github.com/meta/lexical/issues/9620 +[#9622]: https://github.com/meta/lexical/issues/9622 +[#9623]: https://github.com/meta/lexical/issues/9623 +[#9624]: https://github.com/meta/lexical/issues/9624 diff --git a/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts b/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts new file mode 100644 index 00000000000..6b8465656de --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts @@ -0,0 +1,251 @@ +/** + * 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 + +// Fuzz: does an undo spread a comment onto other blocks? Two bound editors, +// synced after every op, run random comment adds (each inside one text +// node), comment removals ($removeMarkID over the whole document), formats, +// plain typing, deletes and undos. A comment id on two blocks is a spread +// (#9623: "X adds, Y removes, X undoes, Y undoes" put it on everything +// after it). Env: US_SEEDS (default 60), US_START, US_OPS (default 30), +// US_UNDO (default 0.15). + +import {buildEditorFromExtensions} from '@lexical/extension'; +import { + $addMarkID, + $getNodeMarkIDs, + $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 LexicalEditor, + type LexicalNode, + type TextNode, + UNDO_COMMAND, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {createYDoc, YCollabExtension} from '../..'; +import {seedYState} from '../../headless'; + +const SEEDS = Number(process.env.US_SEEDS ?? 60); +const START = Number(process.env.US_START ?? 1); +const OPS = Number(process.env.US_OPS ?? 30); +const UNDO = Number(process.env.US_UNDO ?? 0.15); +const BLOCKS = 6; +const CONFIG = { + extensions: [RichTextExtension, MarkStateExtension], + schemaVersion: 1, +}; + +function rng(seed: number) { + let s = seed >>> 0 || 1; + return () => { + s ^= s << 13; + s ^= s >>> 17; + s ^= s << 5; + return (s >>> 0) / 4294967296; + }; +} + +type Op = {e: number} & ( + | {k: 'add'; t: number; a: number; b: number; id: string} + | {k: 'remove'; id: string} + | {k: 'format'; t: number; a: number; b: number; f: 'bold' | 'italic'} + | {k: 'type'; t: number; a: number; s: string} + | {k: 'del'; t: number; a: number; b: number} + | {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 $select(t: TextNode, a: number, b: number) { + const s = $createRangeSelection(); + const n = t.getTextContentSize(); + s.anchor.set(t.getKey(), Math.min(a, n), 'text'); + s.focus.set(t.getKey(), Math.min(b, n), 'text'); + $setSelection(s); + return s; +} + +function $apply(op: Op) { + if (op.k === 'undo') { + return; + } + if (op.k === 'remove') { + $removeMarkID(op.id); + return; + } + const ts = $texts(); + const t = ts[op.t % Math.max(1, ts.length)]; + if (t === undefined) { + return; + } + const size = t.getTextContentSize(); + if (op.k === 'add') { + if (Math.min(op.b, size) > Math.min(op.a, size)) { + $addMarkID(op.id, $select(t, op.a, op.b)); + } + } else if (op.k === 'format') { + $select(t, op.a, op.b).formatText(op.f); + } else if (op.k === 'type') { + const s = $select(t, op.a, op.a); + s.format = 0; + s.insertText(op.s); + } else if (Math.min(op.b, size) > Math.min(op.a, size)) { + $select(t, op.a, op.b).removeText(); + } + $setSelection(null); +} + +/** The comment ids that are on more than one block. */ +function spread(editor: LexicalEditor): string[] { + return editor.read(() => { + const blocks = new Map>(); + for (const t of $texts()) { + const block = t.getParentOrThrow().getKey(); + for (const id of $getNodeMarkIDs(MarkStateExtension, t)) { + const set = blocks.get(id) ?? new Set(); + set.add(block); + blocks.set(id, set); + } + } + return [...blocks].filter(([, s]) => s.size > 1).map(([id]) => id); + }); +} + +function gen(seed: number): Op[] { + const r = rng(seed * 2654435761); + const ops: Op[] = []; + const live: string[] = []; + let n = 0; + const pos = () => Math.floor(r() * 30); + for (let i = 0; i < OPS; i++) { + const e = Math.floor(r() * 2); + const v = r() * (0.88 + UNDO); + const t = Math.floor(r() * BLOCKS * 3); + if (v < 0.25) { + const a = pos(); + const id = `c${++n}`; + live.push(id); + ops.push({a, b: a + 1 + Math.floor(r() * 12), e, id, k: 'add', t}); + } else if (v < 0.37 && live.length > 0) { + const [id] = live.splice(Math.floor(r() * live.length), 1); + ops.push({e, id, k: 'remove'}); + } else if (v < 0.5) { + const a = pos(); + const f = r() < 0.5 ? 'bold' : 'italic'; + ops.push({a, b: a + 1 + Math.floor(r() * 8), e, f, k: 'format', t}); + } else if (v < 0.68) { + const s = ['x', ' new ', 'yy'][Math.floor(r() * 3)]; + ops.push({a: pos(), e, k: 'type', s, t}); + } else if (v < 0.88) { + const a = pos(); + ops.push({a, b: a + 1 + Math.floor(r() * 6), e, k: 'del', t}); + } else { + ops.push({e, k: 'undo'}); + } + } + return ops; +} + +const WORDS = ['alpha', 'beta', 'gamma', 'delta', 'note', 'run']; + +test.each(Array.from({length: SEEDS}, (_, i) => START + i))( + 'an undo never spreads a comment onto another block (seed %i)', + seed => { + const seeded = seedYState(CONFIG, () => { + const root = $getRoot().clear(); + for (let i = 0; i < BLOCKS; i++) { + root.append( + $createParagraphNode().append( + $createTextNode(`${WORDS[i]} one two three four five ${i}`), + ), + ); + } + }); + const peers = ['A', 'B'].map(name => { + const doc = createYDoc(seeded); + const editor = buildEditorFromExtensions( + defineExtension({ + dependencies: [ + RichTextExtension, + MarkStateExtension, + configExtension(YCollabExtension, { + doc, + schemaVersion: 1, + synced: true, + }), + ], + name: `undo-spread-fuzz/${name}`, + onError: error => { + throw error; + }, + }), + ); + return {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)), + ); + }; + const ops = gen(seed); + try { + ops.forEach((op, i) => { + const {editor} = peers[op.e]; + if (op.k === 'undo') { + editor.dispatchCommand(UNDO_COMMAND, undefined); + } else { + editor.update(() => $apply(op), {discrete: true}); + } + sync(); + for (const peer of peers) { + expect( + spread(peer.editor), + `after op ${i}: ${JSON.stringify(ops.slice(0, i + 1))}`, + ).toEqual([]); + } + }); + } finally { + for (const peer of peers) { + peer.editor.dispose(); + } + } + }, +); diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts new file mode 100644 index 00000000000..ad1276dacc6 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts @@ -0,0 +1,415 @@ +/** + * 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 + +import {buildEditorFromExtensions} from '@lexical/extension'; +import { + $addMarkID, + $getNodeMarkIDs, + $removeMarkID, + MarkStateExtension, +} from '@lexical/mark'; +import {RichTextExtension} from '@lexical/rich-text'; +import {$patchStyleText} from '@lexical/selection'; +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $getState, + $isElementNode, + $isTextNode, + $setSelection, + $setState, + configExtension, + defineExtension, + type ElementNode, + type LexicalEditor, + type LexicalNode, + REDO_COMMAND, + type TextNode, + UNDO_COMMAND, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; + +import {createYDoc, YCollabExtension, type YDoc} from '../../..'; +import {readYState, seedYState} from '../../../headless'; +import {$block, $selectIn, expectConverged, setup} from '../../utils/harness'; +import {markState, prepareMarkState as prepare} from '../../utils/sizeOps'; + +// #9623, #9620: an undo that brings back an item opening a value (a comment, +// a format, a style, a NodeState value) whose run's end is gone, because +// another change's undo deleted it before (#9623) or at the same time +// (#9620), left the value open: it covered everything after it. The undo +// now ends the run it brings back where that run ended when the item was +// deleted, with a null of its own (see Binding#closeRestoredRuns and +// #recordRunEnds). +// +// Input space: the value is a comment id (MarkState), bold, a color style or +// a NodeState value; a collaborator removes it from the whole word, from part +// of it, or (comments) from the whole document; the value's author undoes +// its change before the removal is undone, or not; another client types +// plain text right after the word, or not; then a redo and another undo; +// garbage collection on and off; two client id orders. + +const PARAS = ['zero zero', 'one one', 'two two two', 'three three', 'four']; +const WORD: [block: number, start: number, end: number] = [2, 0, 5]; +const NODE_STATE = markState('n'); + +type Value = 'comment' | 'bold' | 'color' | 'NodeState'; +type Removal = 'the word' | 'part of the word' | 'the whole document'; + +function $wordSelection(start = WORD[1], end = WORD[2]) { + const block = $getRoot().getChildAtIndex(WORD[0]) as ElementNode; + let offset = 0; + const selection = $createRangeSelection(); + for (const node of block.getAllTextNodes()) { + const size = node.getTextContentSize(); + if (start >= offset && start <= offset + size) { + selection.anchor.set(node.getKey(), start - offset, 'text'); + break; + } + offset += size; + } + offset = 0; + for (const node of block.getAllTextNodes()) { + const size = node.getTextContentSize(); + if (end >= offset && end <= offset + size) { + selection.focus.set(node.getKey(), end - offset, 'text'); + break; + } + offset += size; + } + $setSelection(selection); + return selection; +} + +function $give(value: Value, on: boolean, start?: number, end?: number) { + const selection = $wordSelection(start, end); + if (value === 'comment') { + if (on) { + $addMarkID('c2', selection); + } else { + $removeMarkID('c2', selection); + } + } else if (value === 'bold') { + // 1 is bold's format flag: bold on, or off, whatever the text has. + selection.formatText('bold', on ? 1 : 0); + } else if (value === 'color') { + $patchStyleText(selection, {color: on ? 'red' : null}); + } else { + for (const node of selection.extract()) { + if ($isTextNode(node)) { + $setState(node, NODE_STATE, on); + } + } + } + $setSelection(null); +} + +function $has(value: Value, node: TextNode): boolean { + switch (value) { + case 'comment': + return $getNodeMarkIDs(MarkStateExtension, node).includes('c2'); + case 'bold': + return node.hasFormat('bold'); + case 'color': + return node.getStyle().includes('red'); + case 'NodeState': + return $getState(node, NODE_STATE); + } +} + +/** Each block's text with every character that has the value as `^`. */ +function $marked(value: Value): string[] { + return $getRoot() + .getChildren() + .map(block => + $isElementNode(block) + ? block + .getAllTextNodes() + .map(node => + $has(value, node) + ? '^'.repeat(node.getTextContentSize()) + : node.getTextContent(), + ) + .join('') + : '?', + ); +} + +const CONFIG = { + extensions: [RichTextExtension, MarkStateExtension], + schemaVersion: 1, +}; + +function clients(gc: boolean, ids: number[]) { + const seeded = seedYState(CONFIG, () => { + $getRoot() + .clear() + .append( + ...PARAS.map(text => + $createParagraphNode().append($createTextNode(text)), + ), + ); + }); + const peers = ids.map(id => { + const doc = createYDoc(seeded, {gc}); + doc.clientID = id; + const editor = buildEditorFromExtensions( + defineExtension({ + dependencies: [ + RichTextExtension, + MarkStateExtension, + configExtension(YCollabExtension, { + captureTimeout: 0, + doc, + schemaVersion: 1, + synced: true, + }), + ], + name: `undo-restored-run-ends/${id}`, + onError: error => { + throw error; + }, + }), + ); + return {doc, editor}; + }); + const sync = () => { + for (const from of peers) { + for (const to of peers) { + if (from !== to) { + Y.applyUpdate( + to.doc, + Y.encodeStateAsUpdate(from.doc, Y.encodeStateVector(to.doc)), + ); + } + } + } + }; + return {peers, sync}; +} + +function update(editor: LexicalEditor, fn: () => void) { + editor.update(fn, {discrete: true}); +} + +function liveFormatItems(doc: YDoc): number { + let n = 0; + const root = doc.get('lexical-y') as unknown as {_start: Y.Item | null}; + for (let it = root._start; it !== null; it = it.right) { + if (!it.deleted && it.content instanceof Y.ContentFormat) { + n++; + } + } + return n; +} + +/** + * Every client and a fresh read of the document agree, and the value is on + * nothing but what `allowed` gives for the word's block (a string of the + * word's block with `^` where the value may be); the other blocks have none. + */ +function check( + value: Value, + t: ReturnType, + allowed: (marked: string) => boolean, +) { + const views = t.peers.map(p => p.editor.read(() => $marked(value))); + const fresh = readYState(Y.encodeStateAsUpdate(t.peers[0].doc), CONFIG, () => + $marked(value), + ); + for (const view of views) { + expect(view).toEqual(views[0]); + } + expect(fresh).toEqual(views[0]); + views[0].forEach((text, i) => { + if (i !== WORD[0]) { + expect(text).not.toContain('^'); + } + }); + expect(allowed(views[0][WORD[0]])).toBe(true); +} + +/** The value on the word (or the part `[a, b)` of it) only, or nowhere. */ +const onWordOrNowhere = (marked: string, a = WORD[1], b = WORD[2]) => { + const text = PARAS[WORD[0]]; + const typed = marked.length - text.length; + const plain = marked.replace(/\^/g, ''); + if (!marked.includes('^')) { + return true; + } + // Text typed after the word ("ZZ") is never marked. + const first = marked.indexOf('^'); + const last = marked.lastIndexOf('^') + 1; + return ( + typed >= 0 && + first >= a && + last <= b && + plain.length <= marked.length && + marked.slice(b).replace(/\^/g, '') === marked.slice(b) + ); +}; + +const VALUES: Value[] = ['comment', 'bold', 'color', 'NodeState']; +const REMOVALS: Removal[] = [ + 'the word', + 'part of the word', + 'the whole document', +]; + +describe('an undo that brings back a value ends its run where it ended (#9623)', () => { + for (const gc of [false, true]) { + for (const ids of [ + [1, 2, 3], + [3, 2, 1], + ]) { + for (const value of VALUES) { + for (const removal of REMOVALS) { + if (removal === 'the whole document' && value !== 'comment') { + continue; + } + for (const authorUndoes of [true, false]) { + for (const typedAfter of [false, true]) { + test(`${value}, removed from ${removal}, ${authorUndoes ? 'its author undoes it first' : 'kept by its author'}${typedAfter ? ', text typed after the word' : ''} (gc ${gc ? 'on' : 'off'}, ids ${ids})`, () => { + const t = clients(gc, ids); + const [a, b, c] = t.peers; + update(b.editor, () => $give(value, true)); + t.sync(); + if (typedAfter) { + update(c.editor, () => { + const selection = $wordSelection(WORD[2], WORD[2]); + selection.format = 0; + selection.style = ''; + selection.insertNodes([$createTextNode('ZZ')]); + $setSelection(null); + }); + t.sync(); + } + update(a.editor, () => { + if (removal === 'the whole document') { + $removeMarkID('c2'); + } else if (removal === 'the word') { + $give(value, false); + } else { + $give(value, false, WORD[1], WORD[1] + 3); + } + }); + t.sync(); + if (authorUndoes) { + b.editor.dispatchCommand(UNDO_COMMAND, undefined); + t.sync(); + } + a.editor.dispatchCommand(UNDO_COMMAND, undefined); + t.sync(); + check(value, t, marked => onWordOrNowhere(marked)); + a.editor.dispatchCommand(REDO_COMMAND, undefined); + t.sync(); + check(value, t, marked => onWordOrNowhere(marked)); + a.editor.dispatchCommand(UNDO_COMMAND, undefined); + t.sync(); + check(value, t, marked => onWordOrNowhere(marked)); + for (const p of t.peers) { + p.editor.dispose(); + } + }); + } + } + } + } + } + } + + test('repeated undo and redo of the removal keep the items bounded', () => { + const t = clients(false, [1, 2, 3]); + const [a, b] = t.peers; + update(b.editor, () => $give('comment', true)); + t.sync(); + update(a.editor, () => $give('comment', false)); + t.sync(); + b.editor.dispatchCommand(UNDO_COMMAND, undefined); + t.sync(); + const counts: number[] = []; + for (let i = 1; i <= 40; i++) { + a.editor.dispatchCommand(UNDO_COMMAND, undefined); + t.sync(); + a.editor.dispatchCommand(REDO_COMMAND, undefined); + t.sync(); + if (i % 10 === 0) { + counts.push(liveFormatItems(a.doc)); + } + } + check('comment', t, marked => onWordOrNowhere(marked)); + // As many live format items after 40 cycles as after 10. + expect(counts[3]).toBe(counts[0]); + for (const p of t.peers) { + p.editor.dispose(); + } + }); +}); + +// #9620: the end goes at the same time as the undo brings the value back. +describe('an undo that brings back a value while its end goes at the same time (#9620)', () => { + function show(peer: {read: (fn: () => T) => T}): string { + return peer.read(() => + $getRoot() + .getChildren() + .map((_, i) => + ($block([i]).getAllTextNodes() as TextNode[]) + .map(n => { + const ids = $getNodeMarkIDs(n as LexicalNode); + return ( + JSON.stringify(n.getTextContent()) + + (ids.length > 0 ? `[${ids.join(',')}]` : '') + ); + }) + .join(''), + ) + .join(' / '), + ); + } + for (const ids of [ + [1, 2], + [2, 1], + ]) { + test(`a comment its author undid does not cover the rest of the document (ids ${ids})`, async () => { + const t = await setup( + () => + $getRoot().append( + $createParagraphNode().append($createTextNode('abcd Kilo bb cc')), + $createParagraphNode().append($createTextNode('tail line')), + ), + {ids, prepare}, + ); + t.a.update(() => void $addMarkID('c1', $selectIn([0], 5, 9))); + t.b.update(() => void $addMarkID('c2', $selectIn([0], 0, 3))); + t.b.update(() => { + const [, tail] = ( + $block([0]).getAllTextNodes() as TextNode[] + )[0].splitText(2); + tail.insertBefore($createTextNode('XY')); + }); + await t.sync(); + expect(show(t.a)).toBe( + '"ab"[c2]"XY""c"[c2]"d ""Kilo"[c1]" bb cc" / "tail line"', + ); + t.offline(); + t.a.update(() => void $selectIn([0], 2, 5).removeText()); + t.a.update(() => void $selectIn([0], 2, 7).removeText()); + t.a.undo(); + t.b.undo(); + t.b.undo(); + await t.sync(); + await expectConverged(t.peers); + expect(show(t.a)).toBe('"abd ""Kilo"[c1]" bb cc" / "tail line"'); + }); + } +}); diff --git a/packages/lexical-y/src/internal/Binding.ts b/packages/lexical-y/src/internal/Binding.ts index 671394ceee2..b496c4d8078 100644 --- a/packages/lexical-y/src/internal/Binding.ts +++ b/packages/lexical-y/src/internal/Binding.ts @@ -24,6 +24,7 @@ import { encodeStateAsUpdate, encodeStateVector, findIndexSS, + type ID, Item, iterateStructsByIdSet, Node as YNode, @@ -86,6 +87,7 @@ import { import { changeEnd, equalFormatValue, + isChangeEnd, isFormatCloser, markFormatCloser, } from './Cursor'; @@ -806,6 +808,12 @@ export class YBinding { * collaborator's change is, and not part of the undo stack. */ private readonly runEndOrigin = {binding: this, runEnd: true}; + /** + * Of each format item that opens a value and that a change of this + * client's (a write, an undo or a redo) deleted, the null that ended its + * run right after that change, where a null did (see recordRunEnds). + */ + private readonly runEndAtDelete = new WeakMap(); /** Stored nodes the running undo/redo restores (see isKeptByUndo). */ private restoring: Set | null = null; /** Stored nodes the running undo/redo deletes (it inserted them). */ @@ -1094,6 +1102,13 @@ export class YBinding { trackedOrigins: new Set([this]), }); this._undoManager = um; + const recordEnds = (tr: Transaction) => { + if (tr.origin === this || tr.origin === um) { + this.recordRunEnds(tr); + } + }; + this._doc.on('afterTransaction', recordEnds); + this.disposers.push(() => this._doc.off('afterTransaction', recordEnds)); // Each step keeps the selection from before it, which undoing it // restores (as @lexical/history does): a local edit's from before the // edit, and the step an undo or redo pushes to the other stack the @@ -5231,12 +5246,51 @@ export class YBinding { }, this.scanOrigin); } + /** + * Note, for each format item opening a value that `tr` (a change of this + * client's) deleted, the null that ended its run then: the first item of + * its key after it, live or deleted by `tr`, with another value, where + * that is a null (items of its own value continue the run). An undo that + * brings the item back ends its run there (see closeRestoredRuns), also + * when that null is gone by then: deleted by the undo of its own change + * (a comment added, removed by a collaborator, its add undone and then + * the removal undone put the comment on everything after it), and + * possibly collected (its place stays, as a deleted item). + */ + private recordRunEnds(tr: Transaction): void { + iterateStructsByIdSet(tr, tr.deleteSet, s => { + if ( + !(s instanceof Item) || + !(s.content instanceof ContentFormat) || + s.content.value === null + ) { + return; + } + const {key, value} = s.content; + for (let it = s.right; it !== null; it = it.right) { + if ( + it.content instanceof ContentFormat && + it.content.key === key && + (!it.deleted || tr.deleteSet.hasId(it.id)) && + !equalFormatValue(it.content.value, value) + ) { + if (it.content.value === null) { + this.runEndAtDelete.set(s, it.id); + } + return; + } + } + }); + } + /** * End each run that a format item the undo or redo of `step` brought back * opens with an item of this client's own, where the item that ended it is * another client's: right before a live null of another client's that ends - * it, or right after the end the original's own change gave its run (see - * changeEnd), where that is gone and nothing ends the run before it. An + * it, or right after the null that ended the run when this client's + * change deleted the original (see recordRunEnds; else the end the + * original's own change gave its run, see changeEnd), where that is gone + * and nothing ends the run before it. An * undo brings back the items its change deleted, and the closer that ended * such a run was not among them: a collaborator's concurrent undo of the * change that wrote it removed it, and the value ran on to the end of the @@ -5281,13 +5335,22 @@ export class YBinding { // The end of the run the original opened in its own write (see // changeEnd): where that is gone (the undo of that write), the // copy's run ends there. - const gone = opensRun(copy) ? changeEnd(doc, original) : null; + const recorded = this.runEndAtDelete.get(original); + const gone = opensRun(copy) + ? (changeEnd(doc, original) ?? + (recorded === undefined ? null : store.getItem(recorded))) + : null; + // A gone end is ended where it was, and a live null that ends a + // run its client opened in the same write (see isChangeEnd) also + // where no content shows between the copy and it yet: that end can + // go at the same time as this undo (a collaborator's undo of the + // change that wrote it), and the run then took everything after. A + // null by which a change made text inside the run plain (a hole its + // client restores the value after) is no such end. let shown = false; 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, it.right, key]); break; } if (it.deleted) { @@ -5296,9 +5359,9 @@ export class YBinding { if (it.content instanceof ContentFormat) { if (it.content.key === key) { if ( - shown && it.id.client !== client && - it.content.value === null + it.content.value === null && + (shown || isChangeEnd(doc, it)) ) { ends.push([it.left, it, key]); } diff --git a/packages/lexical-y/src/internal/Cursor.ts b/packages/lexical-y/src/internal/Cursor.ts index cdc638f1cfc..f8a180d5ea7 100644 --- a/packages/lexical-y/src/internal/Cursor.ts +++ b/packages/lexical-y/src/internal/Cursor.ts @@ -197,7 +197,7 @@ function lastOwnItemBefore(doc: Doc, item: Item, key: string): Item | null { * same write (see changeEnd): the item of its key its client made last * before it opens a run that ends at `item`. */ -function isChangeEnd(doc: Doc, item: Item): boolean { +export function isChangeEnd(doc: Doc, item: Item): boolean { const opener = lastOwnItemBefore(doc, item, keyOf(item)); return ( opener !== null &&