diff --git a/.changeset/y-delete-keeps-format-items.md b/.changeset/y-delete-keeps-format-items.md new file mode 100644 index 00000000000..3a37eb6e48b --- /dev/null +++ b/.changeset/y-delete-keeps-format-items.md @@ -0,0 +1,9 @@ +--- +'@lexical/y': patch +--- + +[fix] A delete keeps every format item it leaves, so a comment or format a concurrent write or an undo restores ends where it ended instead of running on + +A document now keeps two format items for each formatted run whose text is deleted; see "Deleted formatted text" in the collaboration docs' Limits. + +Verify: comment a word, type plain text inside it on one client, and on another client that has not seen it delete the word with its neighbours, then the space after; after syncing (and undoing either edit), the comment covers only what it covered, never the following words or line. diff --git a/.changeset/y-keep-own-format-items.md b/.changeset/y-keep-own-format-items.md new file mode 100644 index 00000000000..9da081334d9 --- /dev/null +++ b/.changeset/y-keep-own-format-items.md @@ -0,0 +1,7 @@ +--- +'@lexical/y': patch +--- + +[fix] An undo puts a format or comment back only where it was, also after a collaborator deleted or retyped around it, with Yjs's default garbage collection + +Verify: comment a word on one client, remove the comment on another, undo on the first and then on the second (syncing after each step), with documents made by `createYDoc()`: the comment comes back on that word or nowhere, never over the following paragraphs. diff --git a/packages/lexical-website/docs/collaboration/y.md b/packages/lexical-website/docs/collaboration/y.md index 6fb2743fa6e..4be8fa02df2 100644 --- a/packages/lexical-website/docs/collaboration/y.md +++ b/packages/lexical-website/docs/collaboration/y.md @@ -1096,6 +1096,25 @@ not merge as a user might expect: undoing it leaves nothing. For comments, keep resolved ids in the text and mark the threads resolved in your own store instead (see [Resolving comments](./comments.md#resolving-comments)). +- **Deleted formatted text.** A delete keeps every format item it leaves, + as a collaborator's concurrent edit there and an undo of the delete rely + on them: each formatted run (a format, a comment, a NodeState value) + whose text is deleted leaves its two items (its opener and its closer). + Plain typing and deleting add none, and neither does typing + inside a formatted run. So a document grows with the formatted text + deleted from it, not only with its content: most in a place where + formatted text is typed and deleted over and over, where edits also get + slower (a comment added and its word deleted 3,000 times at one place: + 41 ms per edit, against 27 ms when a delete still removed those items). +- **Deleted format items a client can undo.** While a client can undo a + format change, it keeps that change's format items, even after a + collaborator deletes them, so its undo still reads them; it lets them go + when its undo history is cleared or the binding stops. Until the + document reloads, that client's document, and any state it encodes, + holds those deleted items: about 25 bytes each (100 rounds of one client + bolding a word and another clearing it: 2,603 bytes for the first + client's state against 229 for the second). A server or relay that loads + such a state with garbage collection on drops them. - **Named slots.** A slot holds one node: two collaborators setting the same slot at once keep one writer's node, and text typed in a slot that a collaborator replaces or removes at the same time is lost (as with delete @@ -1218,11 +1237,7 @@ not merge as a user might expect: entirely, the undo manager skips that step as changing nothing, and the same undo also reverts your step before it. When you undo a format change while a collaborator removes that format and undoes the removal, the text - keeps the format: the collaborator's undo brings it back. A comment or - format can run on past its end when writes two collaborators made inside - it are undone while another client deletes text across its end: it - carries on over the following words, possibly into the next line. No - text is lost. A format a collaborator applies + keeps the format: the collaborator's undo brings it back. A format a collaborator applies starting right where a run of that format ends, or from inside the run past its end, can be lost on their text after the run when a client other than the one that formatted the run, at the same time, removes the @@ -1314,12 +1329,9 @@ not merge as a user might expect: text typed inside it at the same time lands next to it, in an inline element of its old form. Where text was deleted, the format items it leaves are kept, so that a collaborator's concurrent edit inside or at - the deleted text keeps its formats, until a later delete at the same - place keeps a few per format key of them; text a collaborator types - inside the earlier deleted text after that, where its format changed - more than once along it, takes the format that text had at its start, - and so does that text where an undo brings it back after another - collaborator's delete there. Two joins at once into the same line both + the deleted text keeps its formats, and an undo of the delete brings the + text back with them (see Deleted formatted text in [Limits](#limits)). + Two joins at once into the same line both write its type, and Yjs keeps one of them by client id: a heading joined across a quote to the paragraph after it, while a collaborator joins that paragraph into the quote, can become a quote. @@ -1346,8 +1358,9 @@ reproduces it: leave the format on text typed next to the range (see Undo above): [#9560](https://github.com/meta/lexical/issues/9560). - A comment can spread onto later words when a client types uncommented - text inside it offline while a collaborator's deletes there drop the item - that ended it: [#9564](https://github.com/meta/lexical/issues/9564). + text inside it offline while a collaborator's write removes the item that + ended it (a delete no longer does: it keeps every format item it leaves): + [#9564](https://github.com/meta/lexical/issues/9564). - Turning a paragraph inside a container that is itself inside a container (both not shadow roots) into a list removes the outer container and makes no list, in every `@lexical/y` editor (it runs diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index 6d8deaca10a..98d9df9b4a8 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -55,7 +55,10 @@ issues and docs that carry it. | D21 | **A transform never rewrites what a client read.** Derived fields (`derived`: a list item's `value` and `indent`) are refreshed on open by every client and written by no one. A transform registered while a document is open writes nothing; the author's next edit of that content does. | If transforms rewrote remote content, every client would write the same rewrite. | `DECIDED` | 10-25 | [#9400], [y.md: Set up an editor][y-setup] | | D22 | **Seeding.** `binding.initialize()` writes the initial content into a pristine root once, under a client id hashed from that content, so clients that seed the same content write it once. Seeding on the server with `seedYState` is preferred. | Writing nothing until the first edit was measured and rejected: two first edits in a new document split into two paragraphs, because each line needs a marker. | `DECIDED` | 10-25 | [#9400], [#9437], [y.md: Limits][limits] | | D23 | **Garbage collection stays on**, as Yjs defaults; headless results are compacted as clients store them. Only temporary documents turn it off. Turning it off for history is an app's choice, on every client and server. | A server that skips compaction while clients compact is a trap, and undo and insert attribution work with GC on. | `DECIDED` | 10-23 | [#9437], [y-server.md][y-server] | -| 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] | +| 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] | +| 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] | | 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] | @@ -108,7 +111,7 @@ issues and docs that carry it. | D48 | **Moves duplicate or drop concurrent typing:** text typed into a block while it moves is lost or lands at its old place, and two people moving the same block can duplicate it. | D27. | `ACCEPTED LIMIT` | 10-25 | [#9449], [limits] | | D49 | **Delete wins:** text typed into, or a block moved into, a container a collaborator deletes at the same time is deleted with it. | Yjs deletes a parent's children; anything else would resurrect deleted content. | `ACCEPTED LIMIT` | 10-25 | [limits] | | D50 | **Two clients removing the same text and both undoing show it twice.** | Plain `UndoManager` behaviour; avoiding it needs a layout feature. | `ACCEPTED LIMIT` | 10-23 | [limits] | -| D51 | **Rare undo and marker-cleanup timings can spread a comment or format onto untouched words** (two writes of one client both undone while a collaborator collapses markers; three authors in [#9575]). | Rare, no text lost. A provenance marker would need a layout version bump and about a week. | `ACCEPTED LIMIT` | 10-30 | [#9562], [#9575] | +| D51 | **Rare undo and marker-cleanup timings could spread a comment or format onto untouched words** (two writes of one client both undone while a collaborator collapses markers; three authors in [#9575]). | Rare, no text lost. A provenance marker would need a layout version bump and about a week. Replaced by D86 (10-31): a delete no longer collapses markers, and the [#9575] cells pass. | `DECIDED` | 10-31 | [#9562], [#9575] | | D52 | **Unbolding inside your own bold while a collaborator bolds text starting at its end** can leave the new text plain (when the unbolding client's id is higher). | Rare and cosmetic, no text lost. The repair is tracked in [#9589]; nothing more happens now. | `ACCEPTED LIMIT` | 10-30 | [#9562], [#9589] | | D53 | **Removing a format while a collaborator types inside it** brings the format back on the rest of the range, up to its end. | The collaborator's write restores the format it saw; formats are ranges in one sequence (D16). On Yjs 13 it runs to the end of the paragraph. | `ACCEPTED LIMIT` | 10-27 | [#9400], [limits] | | D54 | **Text changed without a record of the edit** (`setTextContent`, DOM text changes) racing a collaborator puts their typing at the start of the new text. | The remaining cases of D28; recording intent at the source shrinks them. | `ACCEPTED LIMIT` | 10-27 | [#9483], [y.md: Edits made by code][y-code] | @@ -128,7 +131,7 @@ issues and docs that carry it. | D62 | **Two app versions that store a type differently insert its first node at once.** | Needs a storage-kind record protocol; `schemaVersion` (D08) covers apps that bump it. | `OPEN` | 10-30 | [#9512] | | D63 | **A line without a marker joins a paragraph a collaborator appends.** | Every reader rule considered changes how stored documents read. | `OPEN` | 10-30 | [#9524] | | D64 | **Undoing a format while a collaborator edits inside its range.** | Yjs format items cannot be conditional on another item; the remaining option is a core or storage change. | `OPEN` | 10-30 | [#9560], [#9521] | -| D65 | **A comment spreading after an offline client types plain text into it.** | Keeping the last nulls breaks growth bounds; the alternative needs recorded provenance. [#9562], which extended the restore's end rule to runs the writer opened itself, is parked: each attempt regressed a case main handles. The case is tracked in [#9609], with the tests moving to main in [#9605]. A restore in a run whose format change wrote more than 64 items gets no end, because the end scan stops there ([#9603]). | `OPEN` | 10-30 | [#9564], [#9609], [#9562], [#9605], [#9603] | +| D65 | **A comment spreading after an offline client types plain text into it.** | Since D86 a delete keeps every format item it leaves, so the [#9609] cells where a collaborator's delete took the run's end pass; the cells where the restore's run has no end of its own (`OwnRunRestoreEnd.test.ts`) still fail. [#9562], which extended the restore's end rule to runs the writer opened itself, is parked: each attempt regressed a case main handles. The case is tracked in [#9609], with the tests moving to main in [#9605]. A restore in a run whose format change wrote more than 64 items gets no end, because the end scan stops there ([#9603]). | `OPEN` | 10-30 | [#9564], [#9609], [#9562], [#9605], [#9603] | | D66 | **Server identity:** claim a client id on first use, or server-issued leases. | Claim on first use is the recommendation; not decided. | `OPEN` | 10-29 | | | D67 | **The rest of the server-defence proposals:** a relay gate judging version records against a published ceiling, budgets, recovery, an author map, agent write policies. | Researched and proposed; nothing decided beyond D46. | `OPEN` | 10-29 | | | D68 | **Splice-based text edit helpers exported from Lexical** for agents that rewrite text, and named APIs for intent that `setTextContent` drops. | Agents should edit text in place; the API shape is undecided. | `OPEN` | 10-27 | [#9483] | @@ -137,7 +140,6 @@ issues and docs that carry it. | D75 | **The table padding contract under collaboration.** When every client re-runs the table transforms after remote changes, concurrent pads add cells and the table never settles. | Padding that converges needs a rule: only the client that created a row pads it, or a width that is a function of the document instead of each client's write. Meanwhile the open [#9539] would pad, when an edit leaves the table's rows in place, only the rows that lost cells. Two clients padding at once is the accepted limit D55. | `OPEN` | 10-30 | [#9582], [#9539] | | D76 | **A code-token-aware trim for remade text.** A stale replace of a word can take the first letter of the word a rewrite made from it, where a node boundary cut the old word. | Writing a changed word whole fixes it, but also rewrites highlighter tokens glued to a pasted-over token in a code line. A follow-up would apply the rule everywhere except at code highlight token boundaries (D29). | `OPEN` | 10-30 | [#9608] | | D77 | **A stale comment on a word a collaborator rewrites** lands on the new word for one client id order only. | Likely the documented race where text typed at the start of a range a collaborator formats takes that format, with a comment instead of a format. Either the docs name comments and whole-word replacements, or a fix makes both orders agree. | `OPEN` | 10-30 | [#9610], [limits] | -| 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. | Reproduced on main in both client id orders; no design chosen. | `OPEN` | 10-30 | [#9599] | [root-agents]: ../../AGENTS.md [limits]: ../lexical-website/docs/collaboration/y.md#limits @@ -203,6 +205,7 @@ issues and docs that carry it. [#9609]: https://github.com/meta/lexical/issues/9609 [#9610]: https://github.com/meta/lexical/issues/9610 [#9621]: https://github.com/meta/lexical/issues/9621 +[#9623]: https://github.com/meta/lexical/issues/9623 [#9626]: https://github.com/meta/lexical/issues/9626 [#9627]: https://github.com/meta/lexical/issues/9627 [#9635]: https://github.com/meta/lexical/issues/9635 diff --git a/packages/lexical-y/src/__tests__/unit/CollapseKeepsEndsProperty.test.ts b/packages/lexical-y/src/__tests__/unit/CollapseKeepsEndsProperty.test.ts deleted file mode 100644 index 28fff2ac88c..00000000000 --- a/packages/lexical-y/src/__tests__/unit/CollapseKeepsEndsProperty.test.ts +++ /dev/null @@ -1,363 +0,0 @@ -/** - * 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 {$isLinkNode, $toggleLink} from '@lexical/link'; -import {$addMarkID, $getNodeMarkIDs, $removeMarkID} from '@lexical/mark'; -import {$patchStyleText} from '@lexical/selection'; -import { - $createTextNode, - $getRoot, - $isElementNode, - $setSelection, - type TextNode, -} from 'lexical'; -import {describe, expect, test, vi} from 'vitest'; - -import {Cursor} from '../../internal/Cursor'; -import {mulberry32} from '../utils/fuzz'; -import { - $block, - $p, - $selectIn, - freshReader, - type Peer, - setup, -} from '../utils/harness'; -import {prepareMarkState as prepare} from '../utils/sizeOps'; - -// A delete collapses the format items of the run of deleted content it -// leaves (Cursor#collapseGap). On main the concurrent case fails -// (test.fails, #9609): the collapse deletes the end a typist's restore -// relies on. #9617 (parked) kept it. -// Property: the collapse never changes what anyone shows. Random documents -// (comments on most words, with ids that repeat so that one comment has -// several runs, and formats, styles and links), then one peer types inside a -// word while the other deletes the words around it and then text next to -// them. Every peer and a fresh reader are compared, character by character, -// with the same session run with no collapse at all, and no peer writes -// more items for it: -// - taking turns (each peer sees every edit before the next), everything -// shows the same; -// - typing and deleting at the same time, every character of the document's -// own text shows the same (the typed text may take the value of other -// content deleted in a collapsed run: "what the bound gives up" in -// collapseGap). Before the collapse kept ends, 33 of the first 1,000 such -// sessions failed in each client id order (the first is seed 53: a comment -// whose end went ran on over the following words). -// KEEP_ENDS_SEEDS (sessions typing and deleting at the same time; a third as -// many taking turns) and KEEP_ENDS_SEED_START run other sessions. - -const SEEDS = Number(process.env.KEEP_ENDS_SEEDS ?? 60); -const START = Number(process.env.KEEP_ENDS_SEED_START ?? 1); - -const WORDS = ['aa', 'bb', 'cc', 'dd', 'ee', 'ff', 'gg', 'hh']; -const IDS = ['c0', 'c1', 'c2', 'c3']; - -/** Each character of every line, with its format, style, comments and link. */ -function $view(): string[] { - const out: string[] = []; - for (const block of $getRoot().getChildren()) { - if ($isElementNode(block)) { - for (const node of block.getAllTextNodes() as TextNode[]) { - const parent = node.getParent(); - const sig = [ - node.getFormat(), - node.getStyle(), - [...$getNodeMarkIDs(node)].sort().join('+'), - $isLinkNode(parent) ? parent.getURL() : '', - ].join('/'); - for (const ch of node.getTextContent()) { - out.push(`${ch}${sig}`); - } - } - } - out.push('|'); - } - return out; -} - -type Rand = () => number; -const pick = (rand: Rand, xs: readonly T[]): T => - xs[Math.floor(rand() * xs.length)]; -const int = (rand: Rand, n: number) => Math.floor(rand() * n); - -/** The text size of `peer`'s line `line`. */ -const size = (peer: Peer, line: number) => - peer.read(() => $block([line]).getTextContentSize()); - -/** A range of at least one character of `peer`'s line `line`, or null. */ -function range(rand: Rand, peer: Peer, line: number): [number, number] | null { - const n = size(peer, line); - if (n === 0) { - return null; - } - const from = int(rand, n); - return [from, Math.min(n, from + 1 + int(rand, 6))]; -} - -/** A format change, comment or link over a random range of `peer`. */ -function formatOp(rand: Rand, peer: Peer, line: number) { - const r = range(rand, peer, line); - if (r === null) { - return; - } - const kind = int(rand, 7); - peer.update(() => { - const selection = $selectIn([line], r[0], r[1]); - if (kind <= 2) { - const id = pick(rand, IDS); - if (kind === 2) { - $removeMarkID(id, selection); - } else { - $addMarkID(id, selection); - } - } else if (kind === 3) { - selection.formatText(pick(rand, ['bold', 'italic'] as const)); - } else if (kind === 4) { - $patchStyleText(selection, {color: pick(rand, ['red', 'blue', null])}); - } else { - $toggleLink(kind === 5 ? 'https://a.test' : 'https://b.test'); - } - $setSelection(null); - }); -} - -/** - * Typed text: upper-case letters, either inheriting what is around (the - * selection's insertText) or as a text node of its own with nothing (a - * restore of everything after it). - */ -function typeOp(rand: Rand, peer: Peer, line: number, word?: [number, number]) { - const n = size(peer, line); - const off = - word !== undefined && word[1] - word[0] > 1 - ? word[0] + 1 + int(rand, word[1] - word[0] - 1) - : int(rand, n + 1); - const text = pick(rand, ['X', 'YZ', 'Q']); - const plain = rand() < 0.6; - peer.update(() => { - if (plain && off > 0 && off < n) { - let o = off; - for (const node of $block([line]).getAllTextNodes() as TextNode[]) { - if (o > 0 && o < node.getTextContentSize()) { - const [, tail] = node.splitText(o); - tail.insertBefore($createTextNode(text)); - break; - } - if (o === node.getTextContentSize()) { - node.insertAfter($createTextNode(text)); - break; - } - o -= node.getTextContentSize(); - } - } else { - $selectIn([line], off).insertText(text); - } - $setSelection(null); - }); -} - -type Session = {views: string[][]; errors: unknown[]; clocks: number[]}; - -/** The word spans of `peer`'s line `line`. */ -function wordSpans(peer: Peer, line: number): [number, number][] { - const text = peer.read(() => $block([line]).getTextContent()); - const spans: [number, number][] = []; - for (const m of text.matchAll(/\S+/g)) { - spans.push([m.index, m.index + m[0].length]); - } - return spans; -} - -async function run( - seed: number, - ids: number[], - together: boolean, - collapse: boolean, -): Promise { - const spy = collapse - ? null - : vi - .spyOn( - Cursor.prototype as unknown as {collapseGap: () => void}, - 'collapseGap', - ) - .mockImplementation(() => {}); - try { - const rand = mulberry32(seed); - const line = () => - Array.from({length: 6 + int(rand, 4)}, () => pick(rand, WORDS)).join(' '); - const lines = [line(), line()]; - const t = await setup(() => $getRoot().append(...lines.map($p)), { - ids, - prepare, - }); - // The document's history, taking turns: most words get a comment (ids - // repeat, so a comment can have several runs), and a few other format - // changes, comments, links and removals over random ranges. - for (let l = 0; l < 2; l++) { - for (const [from, to] of wordSpans(t.a, l)) { - if (rand() < 0.7) { - const id = pick(rand, IDS); - pick(rand, [t.a, t.b]).update(() => { - $addMarkID(id, $selectIn([l], from, to)); - $setSelection(null); - }); - } - } - } - for (let i = 0, n = 1 + int(rand, 5); i < n; i++) { - formatOp(rand, pick(rand, [t.a, t.b]), int(rand, 2)); - } - await t.sync(); - if (!together) { - t.offline(); - } - // A types inside a word (or changes a format there) while B deletes - // the words around it and then, in later deletes, next to them. - const where = int(rand, 2); - const spans = wordSpans(t.b, where); - const target = pick(rand, spans); - for (let i = 0, n = 1 + int(rand, 2); i < n; i++) { - if (rand() < 0.85) { - typeOp(rand, t.a, where, target); - } else { - formatOp(rand, t.a, where); - } - if (together) { - await t.sync(); - } - } - // B's words (with A's text in them when B has seen it). - const seen = wordSpans(t.b, where); - const k = Math.min(spans.indexOf(target), seen.length - 1); - let first = seen[Math.max(0, k - int(rand, 3))][0]; - const last = seen[Math.min(seen.length - 1, k + int(rand, 3))][1]; - t.b.update(() => { - $selectIn([where], first, last).removeText(); - $setSelection(null); - }); - if (together) { - await t.sync(); - } - for (let i = 0, n = 1 + int(rand, 3); i < n; i++) { - const lineSize = size(t.b, where); - const len = 1 + int(rand, 4); - first = Math.min(first, lineSize); - // Right after the deleted words, or right before them. - const [from, to] = - rand() < 0.6 - ? [first, Math.min(lineSize, first + len)] - : [Math.max(0, first - len), first]; - if (from < to) { - t.b.update(() => { - $selectIn([where], from, to).removeText(); - $setSelection(null); - }); - first = from; - } - if (together) { - await t.sync(); - } - } - await t.sync(); - const fresh = await freshReader(t.a); - return { - clocks: [t.a, t.b].map(p => p.doc.store.getClock(p.doc.clientID)), - errors: [...t.a.errors, ...t.b.errors, ...fresh.errors], - views: [t.a, t.b, fresh].map(p => p.read($view)), - }; - } finally { - spy?.mockRestore(); - } -} - -// Seeds (of the default range) where main's collapse lets a comment spread -// over the document's own text when typing and deleting at the same time. -const MAIN_SPREADS = new Set([53]); - -/** The document's own text: the typist's capitals stand for themselves. */ -const own = (v: string[]) => - v.map(c => (/[A-Z]/.test(c.charAt(0)) ? c.charAt(0) : c)); - -/** - * A session typing and deleting at the same time, with and without the - * collapse, and the checks that hold whatever the collapse keeps: no - * errors, no more writes, the same characters, every peer and a fresh - * reader agreeing. - */ -async function concurrent( - seed: number, - ids: number[], -): Promise<[Session, Session]> { - const kept = await run(seed, ids, false, true); - const none = await run(seed, ids, false, false); - expect(kept.errors).toEqual([]); - expect({clocks: kept.clocks, seed}).toEqual({clocks: none.clocks, seed}); - const chars = (views: string[][]) => - views.map(v => v.map(c => c.charAt(0)).join('')); - expect(chars(kept.views)).toEqual(chars(none.views)); - expect(kept.views[1]).toEqual(kept.views[0]); - expect(kept.views[2]).toEqual(kept.views[0]); - return [kept, none]; -} - -describe('@lexical/y a delete collapsing a run never changes what anyone shows', () => { - for (const ids of [ - [1, 2], - [2, 1], - ]) { - test(`taking turns: every character shows as with no collapse (ids ${ids})`, async () => { - for (let seed = START; seed < START + Math.ceil(SEEDS / 3); seed++) { - const kept = await run(seed, ids, true, true); - const none = await run(seed, ids, true, false); - expect(kept.errors).toEqual([]); - expect({seed, views: kept.views}).toEqual({seed, views: none.views}); - // No peer writes more for it. - expect({clocks: kept.clocks, seed}).toEqual({ - clocks: none.clocks, - seed, - }); - } - }, 600_000); - - test(`typing ‖ deletes: the document's own text shows as with no collapse (ids ${ids})`, async () => { - for (let seed = START; seed < START + SEEDS; seed++) { - const [kept, none] = await concurrent(seed, ids); - if (!MAIN_SPREADS.has(seed)) { - expect({seed, view: own(kept.views[0])}).toEqual({ - seed, - view: own(none.views[0]), - }); - } - } - }, 600_000); - - // The sessions where main's collapse deletes the end a typist's restore - // relies on, so a comment spreads over the document's own text (#9609): - // everything else holds there, and the own text fails until a fix. - for (const seed of MAIN_SPREADS) { - if (seed < START || seed >= START + SEEDS) { - continue; - } - test.fails( - `typing ‖ deletes, seed ${seed}: the document's own text shows as with no collapse (ids ${ids}, #9609)`, - async () => { - const [kept, none] = await concurrent(seed, ids); - expect({seed, view: own(kept.views[0])}).toEqual({ - seed, - view: own(none.views[0]), - }); - }, - 600_000, - ); - } - } -}); diff --git a/packages/lexical-y/src/__tests__/unit/Cursor.test.ts b/packages/lexical-y/src/__tests__/unit/Cursor.test.ts index b67c7be7c70..1069ce532e3 100644 --- a/packages/lexical-y/src/__tests__/unit/Cursor.test.ts +++ b/packages/lexical-y/src/__tests__/unit/Cursor.test.ts @@ -18,7 +18,7 @@ import {Cursor} from '../../internal/Cursor'; // equivalent to Y.Node#applyDelta: the same operations through both, from // the same client, give the same content. It keeps format items Yjs removes // as redundant where they bound content outside the edit (see -// Cursor#collapseGap and Cursor#format), so it holds at least as many. Run +// Cursor#delete and Cursor#format), so it holds at least as many. Run // on every @y/y version bump. type Op = @@ -132,13 +132,10 @@ describe('@lexical/y write cursor', () => { ); }); -// A format change followed by a delete on one cursor. The value each key had -// at the start of the run of items without live content the cursor is in -// (which a delete's collapse of that run keeps, see Cursor#collapseGap) must -// stay the value of the content before the run, also where the format -// change passes, after its range's last content, items of a key it does not -// change: a delete on the same cursor then keeps the run's first item that -// changes that value, which text a peer types after it relies on. +// A format change followed by a delete on one cursor, after its range's last +// content, past items of a key it does not change: the delete keeps every +// format item there (D86), so text a peer types after the first of them keeps +// its value. describe('a format change then a delete on one cursor', () => { type Raw = { forward(): void; @@ -149,7 +146,7 @@ describe('a format change then a delete on one cursor', () => { ['old', 'new'], [true, null], ] as const) { - test(`a run of five items of K=${JSON.stringify(next)} after the range, K=${JSON.stringify(old)} before it: the first stays`, () => { + test(`a run of five items of K=${JSON.stringify(next)} after the range, K=${JSON.stringify(old)} before it: they stay`, () => { const doc = new Y.Doc(); doc.clientID = 7; doc.cleanupFormatting = false; @@ -182,13 +179,7 @@ describe('a format change then a delete on one cursor', () => { expect(cursor.formats.get('K') ?? null).toBe(next); cursor.delete(1); }); - expect(ks.map(it => !it.deleted)).toEqual([ - true, - false, - false, - true, - true, - ]); + expect(ks.map(it => !it.deleted)).toEqual([true, true, true, true, true]); // A peer that types after the first item, offline, keeps its value. peer.transact(tr => { let anchor = peer.get('t')._start; diff --git a/packages/lexical-y/src/__tests__/unit/CursorFamily.test.ts b/packages/lexical-y/src/__tests__/unit/CursorFamily.test.ts index b7b2ab4806a..b9c3b3b634b 100644 --- a/packages/lexical-y/src/__tests__/unit/CursorFamily.test.ts +++ b/packages/lexical-y/src/__tests__/unit/CursorFamily.test.ts @@ -17,7 +17,7 @@ import {Cursor} from '../../internal/Cursor'; // stored one key per member, `m|` = true) names them one family // (see formatFamily). The write cursor's rules for format items then hold // for the attribute as they hold for a key stored whole (see -// Cursor#collapseGap, Cursor#format and Cursor#minimizeFormatChanges). +// Cursor#format and Cursor#minimizeFormatChanges). vi.mock('../../internal/Schema', async importOriginal => { const actual = await importOriginal(); @@ -105,7 +105,7 @@ describe('a set stored one format key per member', () => { }); } - test('words of new members typed and deleted again at one place leave a bounded number of format items', () => { + test('words of new members typed and deleted again at one place leave two format items each', () => { const a = doc(1); a.get('t').insert(0, 'start middle end'); const counts: number[] = []; @@ -119,11 +119,9 @@ describe('a set stored one format key per member', () => { } } expect(members(a).join('')).toBe('start middle end'); - expect(counts[2]).toBe(counts[0]); - // The last word's two items, the opener and closer of the word deleted - // before it (that opener gives the place of the last word's closer its - // value), and the run's last two items. - expect(counts[2]).toBeLessThanOrEqual(6); + // Each word's opener and closer stay (a delete keeps the format items + // around and inside what it deletes, D86): 40, 80 and 120. + expect(counts).toEqual([40, 80, 120]); }); // S6-9: where a run keeps a unit of a family of several keys, the value diff --git a/packages/lexical-y/src/__tests__/unit/DeleteNextToTyping.test.ts b/packages/lexical-y/src/__tests__/unit/DeleteNextToTyping.test.ts new file mode 100644 index 00000000000..9a38e66241a --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/DeleteNextToTyping.test.ts @@ -0,0 +1,273 @@ +/** + * 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 {$isLinkNode, $toggleLink} from '@lexical/link'; +import {$addMarkID, $getNodeMarkIDs, $removeMarkID} from '@lexical/mark'; +import {$patchStyleText} from '@lexical/selection'; +import { + $createTextNode, + $getRoot, + $isElementNode, + $setSelection, + type TextNode, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; + +import {mulberry32} from '../utils/fuzz'; +import { + $block, + $p, + $selectIn, + freshReader, + type Peer, + setup, +} from '../utils/harness'; +import {prepareMarkState as prepare} from '../utils/sizeOps'; + +// Random documents (comments on most words, with ids that repeat so that +// one comment has several runs, and formats, styles and links), then one +// peer types inside a word while the other deletes the words around it and +// then text next to them, taking turns or at the same time. No peer throws, +// and every peer and a fresh reader show the same, character by character. +// (Until D86 these sessions also compared each run with one where a delete +// did not collapse the run it left; a delete now never does.) +// DELETE_TYPING_SEEDS (sessions typing and deleting at the same time; a +// third as many taking turns) and DELETE_TYPING_SEED_START run other +// sessions. + +const SEEDS = Number(process.env.DELETE_TYPING_SEEDS ?? 60); +const START = Number(process.env.DELETE_TYPING_SEED_START ?? 1); + +const WORDS = ['aa', 'bb', 'cc', 'dd', 'ee', 'ff', 'gg', 'hh']; +const IDS = ['c0', 'c1', 'c2', 'c3']; + +/** Each character of every line, with its format, style, comments and link. */ +function $view(): string[] { + const out: string[] = []; + for (const block of $getRoot().getChildren()) { + if ($isElementNode(block)) { + for (const node of block.getAllTextNodes() as TextNode[]) { + const parent = node.getParent(); + const sig = [ + node.getFormat(), + node.getStyle(), + [...$getNodeMarkIDs(node)].sort().join('+'), + $isLinkNode(parent) ? parent.getURL() : '', + ].join('/'); + for (const ch of node.getTextContent()) { + out.push(`${ch}${sig}`); + } + } + } + out.push('|'); + } + return out; +} + +type Rand = () => number; +const pick = (rand: Rand, xs: readonly T[]): T => + xs[Math.floor(rand() * xs.length)]; +const int = (rand: Rand, n: number) => Math.floor(rand() * n); + +/** The text size of `peer`'s line `line`. */ +const size = (peer: Peer, line: number) => + peer.read(() => $block([line]).getTextContentSize()); + +/** A range of at least one character of `peer`'s line `line`, or null. */ +function range(rand: Rand, peer: Peer, line: number): [number, number] | null { + const n = size(peer, line); + if (n === 0) { + return null; + } + const from = int(rand, n); + return [from, Math.min(n, from + 1 + int(rand, 6))]; +} + +/** A format change, comment or link over a random range of `peer`. */ +function formatOp(rand: Rand, peer: Peer, line: number) { + const r = range(rand, peer, line); + if (r === null) { + return; + } + const kind = int(rand, 7); + peer.update(() => { + const selection = $selectIn([line], r[0], r[1]); + if (kind <= 2) { + const id = pick(rand, IDS); + if (kind === 2) { + $removeMarkID(id, selection); + } else { + $addMarkID(id, selection); + } + } else if (kind === 3) { + selection.formatText(pick(rand, ['bold', 'italic'] as const)); + } else if (kind === 4) { + $patchStyleText(selection, {color: pick(rand, ['red', 'blue', null])}); + } else { + $toggleLink(kind === 5 ? 'https://a.test' : 'https://b.test'); + } + $setSelection(null); + }); +} + +/** + * Typed text: upper-case letters, either inheriting what is around (the + * selection's insertText) or as a text node of its own with nothing (a + * restore of everything after it). + */ +function typeOp(rand: Rand, peer: Peer, line: number, word?: [number, number]) { + const n = size(peer, line); + const off = + word !== undefined && word[1] - word[0] > 1 + ? word[0] + 1 + int(rand, word[1] - word[0] - 1) + : int(rand, n + 1); + const text = pick(rand, ['X', 'YZ', 'Q']); + const plain = rand() < 0.6; + peer.update(() => { + if (plain && off > 0 && off < n) { + let o = off; + for (const node of $block([line]).getAllTextNodes() as TextNode[]) { + if (o > 0 && o < node.getTextContentSize()) { + const [, tail] = node.splitText(o); + tail.insertBefore($createTextNode(text)); + break; + } + if (o === node.getTextContentSize()) { + node.insertAfter($createTextNode(text)); + break; + } + o -= node.getTextContentSize(); + } + } else { + $selectIn([line], off).insertText(text); + } + $setSelection(null); + }); +} + +type Session = {views: string[][]; errors: unknown[]}; + +/** The word spans of `peer`'s line `line`. */ +function wordSpans(peer: Peer, line: number): [number, number][] { + const text = peer.read(() => $block([line]).getTextContent()); + const spans: [number, number][] = []; + for (const m of text.matchAll(/\S+/g)) { + spans.push([m.index, m.index + m[0].length]); + } + return spans; +} + +async function run( + seed: number, + ids: number[], + together: boolean, +): Promise { + const rand = mulberry32(seed); + const line = () => + Array.from({length: 6 + int(rand, 4)}, () => pick(rand, WORDS)).join(' '); + const lines = [line(), line()]; + const t = await setup(() => $getRoot().append(...lines.map($p)), { + ids, + prepare, + }); + // The document's history, taking turns: most words get a comment (ids + // repeat, so a comment can have several runs), and a few other format + // changes, comments, links and removals over random ranges. + for (let l = 0; l < 2; l++) { + for (const [from, to] of wordSpans(t.a, l)) { + if (rand() < 0.7) { + const id = pick(rand, IDS); + pick(rand, [t.a, t.b]).update(() => { + $addMarkID(id, $selectIn([l], from, to)); + $setSelection(null); + }); + } + } + } + for (let i = 0, n = 1 + int(rand, 5); i < n; i++) { + formatOp(rand, pick(rand, [t.a, t.b]), int(rand, 2)); + } + await t.sync(); + if (!together) { + t.offline(); + } + // A types inside a word (or changes a format there) while B deletes + // the words around it and then, in later deletes, next to them. + const where = int(rand, 2); + const spans = wordSpans(t.b, where); + const target = pick(rand, spans); + for (let i = 0, n = 1 + int(rand, 2); i < n; i++) { + if (rand() < 0.85) { + typeOp(rand, t.a, where, target); + } else { + formatOp(rand, t.a, where); + } + if (together) { + await t.sync(); + } + } + // B's words (with A's text in them when B has seen it). + const seen = wordSpans(t.b, where); + const k = Math.min(spans.indexOf(target), seen.length - 1); + let first = seen[Math.max(0, k - int(rand, 3))][0]; + const last = seen[Math.min(seen.length - 1, k + int(rand, 3))][1]; + t.b.update(() => { + $selectIn([where], first, last).removeText(); + $setSelection(null); + }); + if (together) { + await t.sync(); + } + for (let i = 0, n = 1 + int(rand, 3); i < n; i++) { + const lineSize = size(t.b, where); + const len = 1 + int(rand, 4); + first = Math.min(first, lineSize); + // Right after the deleted words, or right before them. + const [from, to] = + rand() < 0.6 + ? [first, Math.min(lineSize, first + len)] + : [Math.max(0, first - len), first]; + if (from < to) { + t.b.update(() => { + $selectIn([where], from, to).removeText(); + $setSelection(null); + }); + first = from; + } + if (together) { + await t.sync(); + } + } + await t.sync(); + const fresh = await freshReader(t.a); + return { + errors: [...t.a.errors, ...t.b.errors, ...fresh.errors], + views: [t.a, t.b, fresh].map(p => p.read($view)), + }; +} + +describe('@lexical/y a delete next to typing: every peer and a fresh reader agree', () => { + for (const ids of [ + [1, 2], + [2, 1], + ]) { + for (const together of [true, false]) { + test(`${together ? 'taking turns' : 'typing ‖ deletes'} (ids ${ids})`, async () => { + const n = together ? Math.ceil(SEEDS / 3) : SEEDS; + for (let seed = START; seed < START + n; seed++) { + const {errors, views} = await run(seed, ids, together); + expect({errors, seed}).toEqual({errors: [], seed}); + expect({seed, view: views[1]}).toEqual({seed, view: views[0]}); + expect({seed, view: views[2]}).toEqual({seed, view: views[0]}); + } + }, 600_000); + } + } +}); diff --git a/packages/lexical-y/src/__tests__/unit/IsInline.test.ts b/packages/lexical-y/src/__tests__/unit/IsInline.test.ts index ffe248da77f..6bd25c064bf 100644 --- a/packages/lexical-y/src/__tests__/unit/IsInline.test.ts +++ b/packages/lexical-y/src/__tests__/unit/IsInline.test.ts @@ -301,7 +301,7 @@ describe('@lexical/y when an instance is not what its class declares', () => { ); expect(stored(t.a)).toContain('b:test_toggle_tag'); // No character carries the inline form any more (the delete of its - // text leaves its format items, see Cursor#collapseGap). + // text leaves its format items, see Cursor#delete). expect(stored(t.a, false)).not.toContain('w:test_toggle_tag'); const $lifted = () => $assertNodeType( @@ -362,7 +362,7 @@ describe('@lexical/y when an instance is not what its class declares', () => { // of the node or the other depending on the client ids, as any // concurrent insert at a deleted position does, and keeps the format it // was typed with, the inline form (a delete keeps the format items - // around the deleted text, see Cursor#collapseGap). + // around the deleted text, see Cursor#delete). await expectConverged( t.peers, ids[0] === 1 diff --git a/packages/lexical-y/src/__tests__/unit/OwnCommentGapCollapse.test.ts b/packages/lexical-y/src/__tests__/unit/OwnCommentGapCollapse.test.ts index 3a3d019b349..a92fd8b7140 100644 --- a/packages/lexical-y/src/__tests__/unit/OwnCommentGapCollapse.test.ts +++ b/packages/lexical-y/src/__tests__/unit/OwnCommentGapCollapse.test.ts @@ -56,8 +56,9 @@ function $insertPlainAt(off: number, text: string) { // A comments "Kilo" and B comments "bb"; A types plain "X" inside "Kilo" in // one write, and both sync. Then A undoes it while B, having seen it, -// deletes past the comment's end ("o", then the space or " bb"), so the -// delete collapses a gap that holds the other comment's items. The comment +// deletes past the comment's end ("o", then the space or " bb"), next to a +// gap that holds the other comment's items (a delete collapsed such a gap +// before D86; it now keeps every format item it leaves). The comment // ends where it ended on every client. Main passes; #9562 (parked) wrote a // second null of A's own right after the comment's end, the collapse kept // it (the key's last item) and dropped the comment's end, and A's undo diff --git a/packages/lexical-y/src/__tests__/unit/OwnRunRestoreEnd.test.ts b/packages/lexical-y/src/__tests__/unit/OwnRunRestoreEnd.test.ts index 084c2127835..04ccd90e2cb 100644 --- a/packages/lexical-y/src/__tests__/unit/OwnRunRestoreEnd.test.ts +++ b/packages/lexical-y/src/__tests__/unit/OwnRunRestoreEnd.test.ts @@ -39,9 +39,9 @@ import {prepareMarkState as prepare} from '../utils/sizeOps'; // comment's opener and end there, which for it bracket deleted content only, // so deleting them changes nothing it shows. That second step is simulated // with raw Yjs deletes (dropCommentItems): it stands for any concurrent -// deletion of that end, such as the bounded collapse a delete next to a run -// of deleted content runs (Cursor#collapseGap, which keeps a bounded number -// of the format items there), and isolates the condition this guards: the +// deletion of that end (a delete next to a run of deleted content used to +// collapse its format items, keeping a bounded number; it keeps them all +// now, D86), and isolates the condition this guards: the // restore's run losing the end it relied on. Merged, the restore must still // end where the comment ended, not run on over the rest of the document. // @@ -250,8 +250,8 @@ describe('@lexical/y a restore of a comment its own client opened', () => { // S9-12: one write of A types uncommented text at two places inside A's // own comment, so both of its restores end at the comment's end. Rule 5 // gives that end one null of A's own (not one per restore: the undo - // removes them together, and a second one would push the comment's end - // out of the last items a collaborator's collapse keeps). A undoes the + // removes them together, and a second one pushed the comment's end out + // of the last items a collaborator's collapse kept, before D86). A undoes the // write while B, which has not seen the undo, deletes the rest of the // comment and then the text after it. test(`S9-12: ends where the comment ended when its author undoes a write with two inserts inside it while a collaborator deletes past its end (ids ${ids})`, async () => { @@ -290,9 +290,9 @@ describe('@lexical/y a restore of a comment its own client opened', () => { }); // S9-12b: as S9-12, but A types "X" and "Y" in two writes. Main passes: // it writes no null of A's own for A's own run. With #9562 each write put - // a null of A's own right after the comment's end, B's collapse kept - // those two nulls (a key's last items) and dropped the end, and the - // comment ran on once A undid both writes. + // a null of A's own right after the comment's end, B's collapse (gone + // since D86) kept those two nulls (a key's last items) and dropped the + // end, and the comment ran on once A undid both writes. test(`S9-12b: ends where the comment ended when its author undoes two writes inside it while a collaborator deletes past its end (ids ${ids})`, async () => { const t = await setup( () => $getRoot().append($p('aa Kilometer bb cc dd'), $p('tail line')), diff --git a/packages/lexical-y/src/__tests__/unit/Races.test.ts b/packages/lexical-y/src/__tests__/unit/Races.test.ts index ee9abbb1a2e..08ba7844734 100644 --- a/packages/lexical-y/src/__tests__/unit/Races.test.ts +++ b/packages/lexical-y/src/__tests__/unit/Races.test.ts @@ -592,9 +592,10 @@ describe('@lexical/y undo of a format ‖ plain text typed inside its range', () // #9575: writes inside a comment undone (here two collaborators', three // writes) while another client deletes text across the comment's end. Each // write ended the comment before its text and restored it after, leaving a -// null of its writer right after the comment's end; the delete collapses -// the deleted run there and keeps those nulls but not the comment's own end, -// and the undos then remove them, so the comment runs on (y.md Limits). +// null of its writer right after the comment's end. The delete's collapse of +// the deleted run kept those nulls but not the comment's own end, and the +// undos then removed them, so the comment ran on; a delete now keeps every +// format item it leaves (D86), so the comment's end stays. describe('@lexical/y undo of writes inside a comment ‖ delete across its end', () => { /** Inserts uncommented `text` in the first line right after `prefix`. */ function typeAfter(peer: Peer, prefix: string, text: string) { @@ -656,10 +657,7 @@ describe('@lexical/y undo of writes inside a comment ‖ delete across its end', [1, 2, 3, 4], [4, 3, 2, 1], ]) { - test(`#9575 as documented: the comment runs on over the next words and line (${ids})`, async () => { - expect(await run(ids)).toEqual(['aa |Kilome cc dd[c1]', 'tail line[c1]']); - }); - test.fails(`#9575: the comment ends where it ended (${ids})`, async () => { + test(`#9575: the comment ends where it ended (${ids})`, async () => { expect(await run(ids)).toEqual(['aa |Kilome[c1]| cc dd', 'tail line']); }); } diff --git a/packages/lexical-y/src/__tests__/unit/RunSequenceUndo.test.ts b/packages/lexical-y/src/__tests__/unit/RunSequenceUndo.test.ts index 2702a202363..7d8286e6b1a 100644 --- a/packages/lexical-y/src/__tests__/unit/RunSequenceUndo.test.ts +++ b/packages/lexical-y/src/__tests__/unit/RunSequenceUndo.test.ts @@ -48,8 +48,9 @@ const FORMATS: TextFormatType[] = [ 'strikethrough', ]; const SEEDS = 200; -// Seeds of this range that fail on main as well (#9599). -const FAIL_ON_MAIN = [87]; +// Seeds of this range that fail. Seed 87 failed (#9599) until a restore's +// scan stopped at another client's content (`runOpener`). +const FAIL_ON_MAIN: number[] = []; function rngOf(seed: number) { let a = seed >>> 0; diff --git a/packages/lexical-y/src/__tests__/unit/UndoRunEnds.test.ts b/packages/lexical-y/src/__tests__/unit/UndoRunEnds.test.ts index aa8410c87f7..76bf75cfe9b 100644 --- a/packages/lexical-y/src/__tests__/unit/UndoRunEnds.test.ts +++ b/packages/lexical-y/src/__tests__/unit/UndoRunEnds.test.ts @@ -645,7 +645,8 @@ describe('undo run ends, more orders of events', () => { await t.sync(); // H's restore after " x" stays (with #9561), and so does the // null H's write put right after B's closer, which that restore relies - // on: a collapse of B's run ends by a later delete leaves it. + // on: a later delete leaves it (and, before D86, a collapse of B's run + // ends by that delete did too). const root = h.doc.get('lexical-y') as unknown as { _start: Y.Item | null; }; @@ -674,8 +675,9 @@ describe('undo run ends, more orders of events', () => { "a comment's ends that a collaborator's deletes drop do not come back with an undo of text typed inside it (ids %i %i)", async (...ids) => { // A comments three words and, offline, types "X" inside the middle one - // and undoes it; B deletes the four words one by one, which collapses - // the comment's opener and end away. The run ends A's undo keeps rely + // and undoes it; B deletes the four words one by one, which collapsed + // the comment's opener and end away before D86 (a delete now keeps + // them). The run ends A's undo keeps rely // on that end (the null A's write put after it, rule 5 of // Cursor#format, needs #9562): without it the comment covered the rest // of the document. diff --git a/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts b/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts index c617737a4cf..6998f325834 100644 --- a/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts +++ b/packages/lexical-y/src/__tests__/unit/UndoSpreadFuzz.test.ts @@ -185,95 +185,89 @@ function gen(seed: number): Op[] { const WORDS = ['alpha', 'beta', 'gamma', 'delta', 'note', 'run']; -// Seeds that spread a comment on main (#9623) with the default profile -// (US_OPS, US_UNDO and US_STEP_END unchanged), pinned as test.fails until -// the fix lands. Another profile makes other ops from the same seed, so it -// pins nothing. -const DEFAULT_PROFILE = OPS === 30 && UNDO === 0.15 && STEP_END === 0.1; -const SPREADS_ON_MAIN = new Set([10, 23, 26, 35, 51]); - +// Seeds 10, 23, 26, 35 and 51 spread a comment with the default profile +// (#9623) until the binding kept its own format items from garbage collection +// while they are undoable: the documents here use Yjs's default, gc on, as +// products do. for (let seed = START; seed < START + SEEDS; seed++) { - (DEFAULT_PROFILE && SPREADS_ON_MAIN.has(seed) ? test.fails : test)( - `an undo never spreads a comment onto another block (seed ${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, { - // Undo steps end where the script says, whatever the - // runner's speed (see below). - captureTimeout: 2 ** 30, - doc, - schemaVersion: 1, - synced: true, - }), - ], - name: `undo-spread-fuzz/${name}`, - 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)), + test(`an undo never spreads a comment onto another block (seed ${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 ops = gen(seed); - // Where an undo step ends: after an op with this chance (as edits a - // person makes in a burst share one step), from its own sequence so - // the ops stay those of the seed. - const steps = rng(seed * 40503 + 1); - 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}); - } - if (steps() < STEP_END) { - peers[op.e].binding.undoManager?.stopCapturing(); - } - sync(); - for (const peer of peers) { - expect( - spread(peer.editor), - `after op ${i}: ${JSON.stringify(ops.slice(0, i + 1))}`, - ).toEqual([]); - } - }); - } finally { + } + }); + const peers = ['A', 'B'].map(name => { + const doc = createYDoc(seeded); + const editor = buildEditorFromExtensions( + defineExtension({ + dependencies: [ + RichTextExtension, + MarkStateExtension, + configExtension(YCollabExtension, { + // Undo steps end where the script says, whatever the + // runner's speed (see below). + captureTimeout: 2 ** 30, + doc, + schemaVersion: 1, + synced: true, + }), + ], + name: `undo-spread-fuzz/${name}`, + 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)), + ); + }; + const ops = gen(seed); + // Where an undo step ends: after an op with this chance (as edits a + // person makes in a burst share one step), from its own sequence so + // the ops stay those of the seed. + const steps = rng(seed * 40503 + 1); + 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}); + } + if (steps() < STEP_END) { + peers[op.e].binding.undoManager?.stopCapturing(); + } + sync(); for (const peer of peers) { - peer.editor.dispose(); + 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/CollapseKeepsEnds.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/CollapseKeepsEnds.test.ts index 1f658e912c8..c1795202103 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/CollapseKeepsEnds.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/CollapseKeepsEnds.test.ts @@ -30,16 +30,17 @@ import { import {prepareMarkState as prepare} from '../../utils/sizeOps'; // The tests of #9617 (parked): a collapse that keeps each key's run end. -// The #9609 cases fail on main (test.fails): the collapse deletes the -// comment's end. The others pass on main and hold for any design. +// The #9609 cases failed while a delete collapsed the run (the collapse +// deleted the comment's end); a delete now keeps every format item it leaves +// (D86). The others held before and hold for any design. // // #9609: a client types text without a run's value inside the run (an // uncommented "X" inside a commented word), so its write restores the value // after that text and relies on the run's end (the null the comment wrote // after the word) to end it again. A collaborator that has not seen that // text deletes the word with its neighbours, then deletes the space after -// them, and that second delete collapses the run of deleted content and -// format items it leaves (Cursor#collapseGap): the commented word's opener +// them, and that second delete collapsed the run of deleted content and +// format items it left (before D86 removed that collapse): the commented word's opener // and end are neither around the deleted space, nor the first change of the // comments' value, nor the run's last items, and for the collaborator they // bracket deleted content only. The collapse used to delete both, and once @@ -113,60 +114,94 @@ const WORDS: [number, number][] = [ [14, 16], ]; -describe('@lexical/y a delete collapsing a run keeps the end a concurrent restore relies on', () => { +describe('@lexical/y a delete next to a run keeps the end a concurrent restore relies on', () => { for (const ids of [ [1, 2], [2, 1], ]) { - test.fails( - `#9609: a comment's restore ends where the comment ended (ids ${ids})`, - async () => { - const t = await setup( - () => $getRoot().append($p(LINE), $p('tail line')), - { - ids, - prepare, - }, - ); - t.a.update(() => { - WORDS.forEach(([from, to], i) => { - $addMarkID(`c${i}`, $selectIn([0], from, to)); - }); - $setSelection(null); + test(`#9609: a comment's restore ends where the comment ended (ids ${ids})`, async () => { + const t = await setup( + () => $getRoot().append($p(LINE), $p('tail line')), + { + ids, + prepare, + }, + ); + t.a.update(() => { + WORDS.forEach(([from, to], i) => { + $addMarkID(`c${i}`, $selectIn([0], from, to)); }); - await t.sync(); - expect(words(t.a)).toBe('xx aa[c0] Kilo[c1] bb[c2] cc[c3] dd'); - t.offline(); - // A types uncommented "X" inside its own commented "Kilo" - t.a.update(() => $insertPlainAt(8, 'X')); - expect(words(t.a)).toBe('xx aa[c0] Ki[c1] X lo[c1] bb[c2] cc[c3] dd'); - // B deletes "aa Kilo bb", then the space after it - t.b.update($deleteIn(3, 13)); - t.b.update($deleteIn(3, 4)); - expect(words(t.b)).toBe('xx cc[c3] dd'); - await t.sync(); - await expectConverged(t.peers); - expect(words(t.a)).toBe('xx X cc[c3] dd'); - expect(words(t.a, 1)).toBe('tail line'); - }, - ); + $setSelection(null); + }); + await t.sync(); + expect(words(t.a)).toBe('xx aa[c0] Kilo[c1] bb[c2] cc[c3] dd'); + t.offline(); + // A types uncommented "X" inside its own commented "Kilo" + t.a.update(() => $insertPlainAt(8, 'X')); + expect(words(t.a)).toBe('xx aa[c0] Ki[c1] X lo[c1] bb[c2] cc[c3] dd'); + // B deletes "aa Kilo bb", then the space after it + t.b.update($deleteIn(3, 13)); + t.b.update($deleteIn(3, 4)); + expect(words(t.b)).toBe('xx cc[c3] dd'); + await t.sync(); + await expectConverged(t.peers); + expect(words(t.a)).toBe('xx X cc[c3] dd'); + expect(words(t.a, 1)).toBe('tail line'); + }); - test.fails( - `#9609: the comment's end stays through a later collapse of the same run (ids ${ids})`, - async () => { - // As above, and B then deletes "cc " too: the first collapse removed - // the comment's opener and kept its end, which the second collapse - // must keep although, for B, nothing opens the comment before it. + test(`#9609: the comment's end stays through a later delete next to the same run (ids ${ids})`, async () => { + // As above, and B then deletes "cc " too. While a delete collapsed the + // run it left, the first delete removed the comment's opener and kept + // its end, which the second had to keep although, for B, nothing opens + // the comment before it; a delete now keeps every format item (D86). + const t = await setup( + () => $getRoot().append($p(LINE), $p('tail line')), + { + ids, + prepare, + }, + ); + t.a.update(() => { + WORDS.forEach(([from, to], i) => { + $addMarkID(`c${i}`, $selectIn([0], from, to)); + }); + $setSelection(null); + }); + await t.sync(); + t.offline(); + t.a.update(() => $insertPlainAt(8, 'X')); + t.b.update($deleteIn(3, 13)); + t.b.update($deleteIn(3, 4)); + t.b.update($deleteIn(3, 6)); + expect(words(t.b)).toBe('xx dd'); + await t.sync(); + await expectConverged(t.peers); + expect(words(t.a)).toBe('xx Xdd'); + expect(words(t.a, 1)).toBe('tail line'); + }); + + for (const count of [1, 16, 64, 128]) { + test(`#9609: the restores of ${count} comment ids on one word end where the comments ended (ids ${ids})`, async () => { + // One write gives "Kilo" every id (one item per id at each end, more + // than an end scan of 64 structs reaches at 128). + const marks = Array.from( + {length: count}, + (_, i) => `m${i.toString().padStart(3, '0')}`, + ); const t = await setup( () => $getRoot().append($p(LINE), $p('tail line')), - { - ids, - prepare, - }, + {ids, prepare}, ); t.a.update(() => { WORDS.forEach(([from, to], i) => { - $addMarkID(`c${i}`, $selectIn([0], from, to)); + if (i === 1) { + const selection = $selectIn([0], from, to); + for (const id of marks) { + $addMarkID(id, selection); + } + } else { + $addMarkID(`c${i}`, $selectIn([0], from, to)); + } }); $setSelection(null); }); @@ -175,53 +210,11 @@ describe('@lexical/y a delete collapsing a run keeps the end a concurrent restor t.a.update(() => $insertPlainAt(8, 'X')); t.b.update($deleteIn(3, 13)); t.b.update($deleteIn(3, 4)); - t.b.update($deleteIn(3, 6)); - expect(words(t.b)).toBe('xx dd'); await t.sync(); await expectConverged(t.peers); - expect(words(t.a)).toBe('xx Xdd'); + expect(words(t.a)).toBe('xx X cc[c3] dd'); expect(words(t.a, 1)).toBe('tail line'); - }, - ); - - for (const count of [1, 16, 64, 128]) { - test.fails( - `#9609: the restores of ${count} comment ids on one word end where the comments ended (ids ${ids})`, - async () => { - // One write gives "Kilo" every id (one item per id at each end, more - // than an end scan of 64 structs reaches at 128). - const marks = Array.from( - {length: count}, - (_, i) => `m${i.toString().padStart(3, '0')}`, - ); - const t = await setup( - () => $getRoot().append($p(LINE), $p('tail line')), - {ids, prepare}, - ); - t.a.update(() => { - WORDS.forEach(([from, to], i) => { - if (i === 1) { - const selection = $selectIn([0], from, to); - for (const id of marks) { - $addMarkID(id, selection); - } - } else { - $addMarkID(`c${i}`, $selectIn([0], from, to)); - } - }); - $setSelection(null); - }); - await t.sync(); - t.offline(); - t.a.update(() => $insertPlainAt(8, 'X')); - t.b.update($deleteIn(3, 13)); - t.b.update($deleteIn(3, 4)); - await t.sync(); - await expectConverged(t.peers); - expect(words(t.a)).toBe('xx X cc[c3] dd'); - expect(words(t.a, 1)).toBe('tail line'); - }, - ); + }); } test(`a kept end leaves the value after the run as it was (ids ${ids})`, async () => { diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts index 0d1dfbc8b9b..c7c30f933e5 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts @@ -242,7 +242,7 @@ async function merged(t: Awaited>) { // of the space's bold (redundant with the run before it), the other that // run's opener, deleted with its character. The space, which nobody // touched, lost bold (with mark ids stored per member, a comment): a delete -// keeps the items a concurrent writer relies on (see Cursor#collapseGap). +// keeps the items a concurrent writer relies on (see Cursor#delete). describe('U23A-2: two concurrent deletes next to a character nobody touched', () => { for (const ids of [ [1, 2], @@ -487,7 +487,7 @@ describe('R23-B: plain text pasted over formatted runs ‖ Enter inside them', ( // word's closer, so bold spread over the text after it and the following // lines; text a peer typed at the end of the deleted word lost the word's // format. A delete keeps the format items a concurrent writer relies on -// (see Cursor#collapseGap), and a format change bounds its range with items +// (see Cursor#delete), and a format change bounds its range with items // of its own (see Cursor#format). describe('a delete racing a format change or typing at the deleted run', () => { const $node = (i: number) => @@ -702,10 +702,11 @@ describe('R24B-F1: a comment resolved ‖ plain text typed before a character it }); // F24A-1: every word of another format, comment or NodeState value typed at -// one place and deleted again left two format items live, so a document's -// items grew with its editing history. The run of deleted content and -// format items a delete leaves holds a bounded number per key (see -// Cursor#collapseGap). +// one place and deleted again leaves its two format items (its opener and +// its closer) live: a delete keeps the format items around and inside the +// content it deletes, which a concurrent writer and an undo of the delete +// rely on (D86). So the items grow by exactly two per such word, and by none +// per keystroke of plain text. describe('F24A-1: words typed and deleted again at one place', () => { const live = (peer: Peer) => { let n = 0; @@ -725,7 +726,7 @@ describe('F24A-1: words typed and deleted again at one place', () => { 'code', ] as const; for (const pattern of ['formats', 'comments'] as const) { - test(`leave a bounded number of format items (${pattern})`, async () => { + test(`leave two format items per deleted formatted word (${pattern})`, async () => { const t = await setup(() => { $getRoot().append( $createParagraphNode().append($createTextNode('start middle end')), @@ -757,9 +758,8 @@ describe('F24A-1: words typed and deleted again at one place', () => { } } expect(await merged(t)).toBe('"start middle end"'); - // As many after 60 words as after 20, and at most five per key. - expect(counts[2]).toBe(counts[0]); - expect(counts[2]).toBeLessThanOrEqual(pattern === 'formats' ? 20 : 5); + // Two per word: 40 after 20 words, 80 after 40, 120 after 60. + expect(counts).toEqual([40, 80, 120]); }); } }); @@ -768,9 +768,8 @@ describe('F24A-1: words typed and deleted again at one place', () => { // delete collapsed the format items it left, keeping a few per key, and // removed the closer of an italic word that the typist, typing plain text // after it, relied on: the typist's word, written without items of its own, -// became italic on every client. A delete keeps every item it leaves inside -// the content it deletes and at its edges; a later delete in the same run -// collapses them (see Cursor#collapseGap). +// became italic on every client. A delete keeps every item it leaves (see +// Cursor#delete). describe("S6-8: a line's text deleted ‖ a peer types a word inside it", () => { const $line = () => $getRoot().getChildAtIndex(1) as ElementNode; const $word = () => { diff --git a/packages/lexical-y/src/__tests__/unit/regressions/KeepEndsSessions.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/KeepEndsSessions.test.ts index c3d4ee3368a..a5e407f6f3d 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/KeepEndsSessions.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/KeepEndsSessions.test.ts @@ -26,9 +26,8 @@ import { type TextFormatType, type TextNode, } from 'lexical'; -import {expect, test, vi} from 'vitest'; +import {expect, test} from 'vitest'; -import {Cursor} from '../../../internal/Cursor'; import { $block, $selectIn, @@ -325,40 +324,20 @@ test('restored text takes no comment of the word before it (1861)', async () => }); // A comment over words that deletes and undos around them never touch -// stays where a document read without any collapse has it: main's -// collapse dropped c2 from "wk75" (4694). -test.fails( - 'a comment stays on text that deletes around it never touched (4694)', - async () => { - const t = await replay(session(4694)); - expect(idsOn(t.a, 1, 'wk75')).toEqual(Array(4).fill('[c2,c20]')); - t.peers.forEach(p => p.stop()); - }, -); +// stays where a document read without any collapse has it: the collapse +// dropped c2 from "wk75" (4694); a delete now keeps every format item it +// leaves (D86). +test('a comment stays on text that deletes around it never touched (4694)', async () => { + const t = await replay(session(4694)); + expect(idsOn(t.a, 1, 'wk75')).toEqual(Array(4).fill('[c2,c20]')); + t.peers.forEach(p => p.stop()); +}); -// Main's collapse dropped c9 from "wxq4xq56abcd" (3645), where a document -// read without any collapse keeps it. The drop shows only after the -// session's last undo, so this also flips without a collapse change where -// that undo takes more steps: with every undo press changing what the -// document shows (#9630), the undo also removes c9 without a collapse. -test.fails( - 'a comment is where a document without any collapse has it (3645; also flips with #9630)', - async () => { - const t = await replay(session(3645)); - const kept = chars(t.a, 0); - t.peers.forEach(p => p.stop()); - const spy = vi - .spyOn( - Cursor.prototype as unknown as {collapseGap: () => void}, - 'collapseGap', - ) - .mockImplementation(() => {}); - try { - const u = await replay(session(3645)); - expect(kept).toEqual(chars(u.a, 0)); - u.peers.forEach(p => p.stop()); - } finally { - spy.mockRestore(); - } - }, -); +// The collapse dropped c9 from "wxq4xq56abcd" (3645), where a document +// read without any collapse keeps it; a delete now keeps every format item +// it leaves (D86). +test('a comment stays where it was written (3645)', async () => { + const t = await replay(session(3645)); + expect(idsOn(t.a, 0, 'wxq4xq56abcd')).toEqual(Array(12).fill('[c9]')); + t.peers.forEach(p => p.stop()); +}); diff --git a/packages/lexical-y/src/__tests__/unit/regressions/OwnFormatItems.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/OwnFormatItems.test.ts new file mode 100644 index 00000000000..39d9275dd88 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/OwnFormatItems.test.ts @@ -0,0 +1,564 @@ +/** + * 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 + +// The issues' own repros, with garbage collection on (Yjs's default, which +// products use) and off, and both client id orders: +// - #9623: an undo reads the format items of its change after a +// collaborator deleted them; with gc on they were collected, and the +// comment came back over the rest of the document. The binding now keeps +// its own format items from collection while they are undoable. +// - #9599: a restore's scan took a client's earlier end and later opener +// (with another client's text between them) for one restored run, so its +// undo wrote no end and a format ran onto the next word. +// - #9671: a format write kept the null and the restore that the writer's +// own earlier insert left inside its range; a collaborator's typing relied +// on that restore, and undoing the insert deleted it. Like Yjs's +// formatText, the write now removes them. +// - A range end that restates another client's opener (rule 3) now also +// ends it where that opener's run ended, so undoing that client's change +// leaves the text after the range alone (#9622's family). + +import { + buildEditorFromExtensions, + getExtensionDependencyFromEditor, +} from '@lexical/extension'; +import {$isLinkNode, $toggleLink} from '@lexical/link'; +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, + $isElementNode, + $isTextNode, + $setSelection, + type AnyLexicalExtension, + configExtension, + defineExtension, + getStyleObjectFromCSS, + type LexicalEditor, + type RangeSelection, + type TextNode, + UNDO_COMMAND, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; + +import {createYDoc, type YBinding, YCollabExtension} from '../../..'; +import {readYState, seedYState} from '../../../headless'; +import { + $block, + $p, + $selectIn, + expectConverged, + setup, +} from '../../utils/harness'; + +interface Client { + binding: YBinding; + doc: Y.Doc; + editor: LexicalEditor; +} + +const IDS = [ + [1, 2], + [2, 1], +] as const; +const GC = [true, false]; + +function clients( + extensions: AnyLexicalExtension[], + seeded: Uint8Array, + ids: readonly [number, number], + gc: boolean, +): {A: Client; B: Client; sync: () => void; dispose: () => void} { + const [A, B] = ids.map((id, n) => { + const doc = createYDoc(seeded, {gc}); + doc.clientID = id; + const editor = buildEditorFromExtensions( + defineExtension({ + dependencies: [ + ...extensions, + configExtension(YCollabExtension, { + captureTimeout: 2 ** 30, + doc, + schemaVersion: 1, + synced: true, + }), + ], + name: `own-format-items-${n}`, + onError: e => { + throw e; + }, + }), + ); + const {binding} = getExtensionDependencyFromEditor( + editor, + YCollabExtension, + ).output; + return {binding, doc, editor}; + }); + const sync = () => { + 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 dispose = () => { + A.editor.dispose(); + B.editor.dispose(); + }; + return {A, B, dispose, sync}; +} + +function step(client: Client, f: () => void) { + client.editor.update(f, {discrete: true}); + client.binding.undoManager?.stopCapturing(); +} + +function undo(client: Client) { + client.editor.dispatchCommand(UNDO_COMMAND, undefined); + client.binding.undoManager?.stopCapturing(); +} + +describe('a comment its author undid stays off the rest of the document when a collaborator undoes removing it (#9623)', () => { + const CONFIG = { + extensions: [RichTextExtension, MarkStateExtension], + schemaVersion: 1, + }; + const PARAS = [ + 'zero zero zero', + 'one one one', + 'two two two', + 'three three three', + 'four four four', + ]; + const $show = () => + $getRoot() + .getChildren() + .map(b => + $isElementNode(b) + ? b + .getChildren() + .map(c => { + const ids = $isTextNode(c) + ? $getNodeMarkIDs(MarkStateExtension, c) + : []; + return `${c.getTextContent()}${ids.length ? `[${ids.join(',')}]` : ''}`; + }) + .join('|') + : '?', + ); + const $selectWord = (f: (s: RangeSelection) => void) => { + const p = $getRoot().getChildAtIndex(2); + const t = $isElementNode(p) ? p.getFirstChild() : null; + if (!$isTextNode(t)) { + throw new Error('no text'); + } + const s = $createRangeSelection(); + s.anchor.set(t.getKey(), 0, 'text'); + s.focus.set(t.getKey(), 5, 'text'); + $setSelection(s); + f(s); + $setSelection(null); + }; + const seeded = seedYState(CONFIG, () => { + $getRoot() + .clear() + .append( + ...PARAS.map(t => $createParagraphNode().append($createTextNode(t))), + ); + }); + for (const gc of GC) { + for (const ids of IDS) { + for (const removal of ['whole document', 'that word only'] as const) { + test(`removal: ${removal} (gc ${gc ? 'on' : 'off'}, ids ${ids})`, () => { + const {A, B, sync, dispose} = clients( + CONFIG.extensions, + seeded, + ids, + gc, + ); + try { + step(B, () => $selectWord(s => $addMarkID('c2', s))); + sync(); + step(A, () => + removal === 'whole document' + ? $removeMarkID('c2') + : $selectWord(s => $removeMarkID('c2', s)), + ); + sync(); + undo(B); + sync(); + undo(A); + sync(); + const a = A.editor.getEditorState().read($show); + expect(B.editor.getEditorState().read($show)).toEqual(a); + expect( + readYState(Y.encodeStateAsUpdate(A.doc), CONFIG, $show), + ).toEqual(a); + // On "two t" or nowhere; never on text nobody commented. + expect(a.slice(0, 2)).toEqual(PARAS.slice(0, 2)); + expect(a.slice(3)).toEqual(PARAS.slice(3)); + expect(['two two two', 'two t[c2]|wo two']).toContain(a[2]); + } finally { + dispose(); + } + }); + } + test(`control: without the author's undo, the comment comes back on its word only (gc ${gc ? 'on' : 'off'}, ids ${ids})`, () => { + const {A, B, sync, dispose} = clients( + CONFIG.extensions, + seeded, + ids, + gc, + ); + try { + step(B, () => $selectWord(s => $addMarkID('c2', s))); + sync(); + step(A, () => $removeMarkID('c2')); + sync(); + undo(A); + sync(); + expect(A.editor.getEditorState().read($show)).toEqual([ + ...PARAS.slice(0, 2), + 'two t[c2]|wo two', + ...PARAS.slice(3), + ]); + } finally { + dispose(); + } + }); + } + } +}); + +describe("undoing one's own formats leaves the next word alone after a collaborator typed plain text inside the first run (#9599)", () => { + const CONFIG = {extensions: [RichTextExtension], schemaVersion: 1}; + // "w0 " "w1 " "w2 " | "w3 " (strikethrough) "w4 " "w5 " + const seeded = seedYState(CONFIG, () => { + const $word = (w: string, struck = false) => { + const t = $createTextNode(w + ' '); + return struck ? t.setFormat('strikethrough') : t; + }; + $getRoot() + .clear() + .append( + $createParagraphNode().append($word('w0'), $word('w1'), $word('w2')), + $createParagraphNode().append( + $word('w3', true), + $word('w4'), + $word('w5'), + ), + ); + }); + const $find = (word: string): [TextNode, number] => { + for (const t of $getRoot().getAllTextNodes()) { + const i = t.getTextContent().indexOf(word); + if (i >= 0) { + return [t, i]; + } + } + throw new Error(word); + }; + const $select = (from: string, to: string, fromOffset = 0, toEnd = true) => { + const [a, ao] = $find(from); + const [b, bo] = $find(to); + const s = $createRangeSelection(); + s.anchor.set(a.getKey(), ao + fromOffset, 'text'); + s.focus.set(b.getKey(), toEnd ? bo + to.length : bo + fromOffset, 'text'); + $setSelection(s); + return s; + }; + const formats = (editor: LexicalEditor) => + editor.getEditorState().read(() => + $getRoot() + .getAllTextNodes() + .map(t => `${t.getTextContent()}:${t.getFormat()}`), + ); + for (const gc of GC) { + for (const ids of IDS) { + test(`B's undos leave w4 alone (gc ${gc ? 'on' : 'off'}, ids ${ids})`, () => { + const {A, B, dispose} = clients(CONFIG.extensions, seeded, ids, gc); + try { + step(B, () => void $select('w2', 'w3').formatText('italic')); + step(B, () => { + const s = $select('w3', 'w3', 2, false); + s.setFormat(0); + s.insertText(' zz'); + }); + step(B, () => void $select('w5', 'w5').formatText('italic')); + Y.applyUpdate(A.doc, Y.encodeStateAsUpdate(B.doc)); + const formatOfW4 = (editor: LexicalEditor) => + editor.getEditorState().read(() => $find('w4')[0].getFormat()); + const before = formatOfW4(A.editor); + const svA = Y.encodeStateVector(A.doc); + const svB = Y.encodeStateVector(B.doc); + // Concurrently: A types a plain "X" inside w3, B undoes its three steps. + step(A, () => { + const s = $select('w3', 'w3', 1, false); + s.setFormat(0); + s.insertText('X'); + }); + for (let i = 0; i < 3; i++) { + B.binding.undo(); + B.binding.undoManager?.stopCapturing(); + } + Y.applyUpdate(A.doc, Y.encodeStateAsUpdate(B.doc, svB)); + Y.applyUpdate(B.doc, Y.encodeStateAsUpdate(A.doc, svA)); + expect([formatOfW4(A.editor), formatOfW4(B.editor)]).toEqual([ + before, + before, + ]); + expect(formats(B.editor)).toEqual(formats(A.editor)); + } finally { + dispose(); + } + }); + } + } +}); + +describe('undoing text typed into a run and then formatted leaves the untouched rest of the run as it was (#9671)', () => { + const CONFIG = {extensions: [RichTextExtension], schemaVersion: 1}; + const $texts = (): TextNode[] => { + const p = $getRoot().getFirstChildOrThrow(); + return $isElementNode(p) ? p.getChildren().filter($isTextNode) : []; + }; + const $at = (i: number): [TextNode, number] => { + const ts = $texts(); + let acc = 0; + for (const t of ts) { + const n = t.getTextContentSize(); + if (i <= acc + n) { + return [t, i - acc]; + } + acc += n; + } + const last = ts[ts.length - 1]; + return [last, last.getTextContentSize()]; + }; + const $select = (i: number, j: number) => { + const s = $createRangeSelection(); + const [a, ao] = $at(i); + const [b, bo] = $at(j); + s.anchor.set(a.getKey(), ao, 'text'); + s.focus.set(b.getKey(), bo, 'text'); + $setSelection(s); + return s; + }; + const $typePlain = (i: number, text: string) => { + const s = $select(i, i); + s.format = 0; + s.insertText(text); + $setSelection(null); + }; + const $strike = (i: number, j: number) => { + $select(i, j).formatText('strikethrough'); + $setSelection(null); + }; + const show = (editor: LexicalEditor) => + editor.read(() => + $texts() + .map( + t => + t.getTextContent() + + (t.hasFormat('bold') ? ':b' : '') + + (t.hasFormat('strikethrough') ? ':s' : ''), + ) + .join('|'), + ); + + // "abcdef" (bold) "ghi" (strikethrough). B types a plain "Z" inside "ghi", + // then strikes "gZ" (or "fgZ"). A types a plain "1" right after Z. B undoes + // twice. The clients sync after every step. + const seeded = seedYState(CONFIG, () => { + $getRoot() + .clear() + .append( + $createParagraphNode().append( + $createTextNode('abcdef').toggleFormat('bold'), + $createTextNode('ghi').toggleFormat('strikethrough'), + ), + ); + }); + for (const gc of GC) { + for (const ids of IDS) { + for (const [range, cell] of [ + ['gZ', 'repro'], + ['fgZ', 'repro'], + ['gZ', 'control: A does not type'], + ['gZ', 'control: B undoes only the strikethrough'], + ] as const) { + test(`"hi" keeps strikethrough (B strikes ${range}, ${cell}, gc ${gc ? 'on' : 'off'}, ids ${ids})`, () => { + const {A, B, sync, dispose} = clients( + CONFIG.extensions, + seeded, + ids, + gc, + ); + try { + step(B, () => $typePlain(7, 'Z')); + sync(); + step(B, () => $strike(range === 'gZ' ? 6 : 5, 8)); + sync(); + const typed = cell !== 'control: A does not type'; + if (typed) { + step(A, () => $typePlain(8, '1')); + } + sync(); + undo(B); + sync(); + expect(show(A.editor)).toBe( + typed ? 'abcdef:b|g:s|Z1|hi:s' : 'abcdef:b|g:s|Z|hi:s', + ); + if (cell === 'control: B undoes only the strikethrough') { + return; + } + undo(B); + sync(); + const want = typed ? 'abcdef:b|g:s|1|hi:s' : 'abcdef:b|ghi:s'; + expect([show(A.editor), show(B.editor)]).toEqual([want, want]); + } finally { + dispose(); + } + }); + } + } + } + + // The mirror: "abc" (strikethrough) "def". B types a plain "Z" inside + // "abc", then strikes Z (or more). A types a plain "1" right before or + // after Z. B undoes twice. A's "1" stays plain. + const mirror = seedYState(CONFIG, () => { + $getRoot() + .clear() + .append( + $createParagraphNode().append( + $createTextNode('abc').toggleFormat('strikethrough'), + $createTextNode('def'), + ), + ); + }); + for (const gc of GC) { + for (const ids of IDS) { + for (const [where, struck] of [ + ['before Z', 'Z'], + ['after Z', 'Z'], + ['before Z', 'Zc'], + ['after Z', 'Zcd'], + ] as const) { + test(`A's plain 1 stays plain (A types ${where}, B strikes ${struck}, gc ${gc ? 'on' : 'off'}, ids ${ids})`, () => { + const {A, B, sync, dispose} = clients( + CONFIG.extensions, + mirror, + ids, + gc, + ); + try { + step(B, () => $typePlain(2, 'Z')); + sync(); + step(B, () => $strike(2, 2 + struck.length)); + sync(); + step(A, () => $typePlain(where === 'before Z' ? 2 : 3, '1')); + sync(); + undo(B); + sync(); + undo(B); + sync(); + const want = 'ab:s|1|c:s|def'; + expect([show(A.editor), show(B.editor)]).toEqual([want, want]); + } finally { + dispose(); + } + }); + } + } + } +}); + +describe("a range end that restates another client's opener ends it where that opener's run ended", () => { + // "ABC DEF GHI JKL MNO": B gives "I " and then "K" a value (two undo + // steps); while offline, B undoes the second and C gives " J", across the + // gap between them, a value of its own. Nobody gave " MNO" a value. + const LINE = 'ABC DEF GHI JKL MNO'; + const ORDERS = [ + [1, 2, 3], + [3, 2, 1], + [2, 3, 1], + [1, 3, 2], + [2, 1, 3], + [3, 1, 2], + ]; + const $give = ( + kind: 'link' | 'color', + who: 'B' | 'C', + s: number, + e: number, + ) => { + const sel = $selectIn([0], s, e); + if (kind === 'link') { + $toggleLink(who === 'B' ? 'https://b.test' : 'https://c.test'); + } else { + $patchStyleText(sel, {color: who === 'B' ? 'red' : 'blue'}); + } + $setSelection(null); + }; + // Each text node after offset 14 with any value on it. + const $valuesAfterK = () => { + const out: string[] = []; + let at = 0; + for (const t of $block([0]).getAllTextNodes()) { + const end = at + t.getTextContentSize(); + if (end > 14) { + const color = getStyleObjectFromCSS(t.getStyle()).color; + const link = t.getParent(); + if (color || $isLinkNode(link)) { + out.push( + `${t.getTextContent()} ${color ?? ''}${$isLinkNode(link) ? link.getURL() : ''}`, + ); + } + } + at = end; + } + return out; + }; + for (const gc of GC) { + for (const ids of ORDERS) { + for (const kind of ['link', 'color'] as const) { + test(`${kind} (gc ${gc ? 'on' : 'off'}, ids ${ids})`, async () => { + const t = await setup(() => $getRoot().append($p(LINE)), {gc, ids}); + const [, b, c] = t.peers; + b.update(() => $give(kind, 'B', 10, 12)); + b.binding.undoManager?.stopCapturing(); + b.update(() => $give(kind, 'B', 13, 14)); + await t.sync(); + t.offline(); + b.undo(); + c.update(() => $give(kind, 'C', 11, 13)); + await t.sync(); + await expectConverged(t.peers); + expect(t.a.editor.getEditorState().read($valuesAfterK)).toEqual([]); + }); + } + } + } +}); diff --git a/packages/lexical-y/src/__tests__/unit/regressions/OwnFormatItemsRelease.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/OwnFormatItemsRelease.test.ts new file mode 100644 index 00000000000..cd4a3b1785f --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/OwnFormatItemsRelease.test.ts @@ -0,0 +1,534 @@ +/** + * 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 binding keeps its own format items from garbage collection while they +// are undoable (D88). These check that it lets them go: when both undo +// stacks become empty (a clear, or an undo that changes nothing), when it is +// disposed or replaced, and that what a collaborator deleted meanwhile is +// collected then. A client A bolds part of "alpha"; B, with no undo, clears +// the formats; they sync. + +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $setSelection, + createEditor, + type LexicalEditor, + type TextNode, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {createYDoc, YBinding, type YDoc} from '../../..'; +import {NODES} from '../../utils/harness'; + +function bound(doc: YDoc, captureTimeout: number | null = 0) { + const editor = createEditor({ + nodes: NODES, + onError: e => { + throw e; + }, + onWarn: () => {}, + }); + const binding = new YBinding(editor, doc, {captureTimeout, schemaVersion: 1}); + const stop = binding.start(); + return {binding, doc, editor, stop}; +} + +/** Bolds "ph" of "alpha", or (`bold` false) clears every format. */ +function format(editor: LexicalEditor, bold: boolean) { + editor.update( + () => { + const nodes = $getRoot().getAllTextNodes(); + if (!bold) { + for (const n of nodes) { + n.setFormat(0); + } + return; + } + const at = (i: number): [TextNode, number] => { + for (const n of nodes) { + if (i <= n.getTextContentSize()) { + return [n, i]; + } + i -= n.getTextContentSize(); + } + const last = nodes[nodes.length - 1]; + return [last, last.getTextContentSize()]; + }; + const [a, ao] = at(2); + const [b, bo] = at(4); + const s = $createRangeSelection(); + s.anchor.set(a.getKey(), ao, 'text'); + s.focus.set(b.getKey(), bo, 'text'); + $setSelection(s); + s.formatText('bold'); + $setSelection(null); + }, + {discrete: true}, + ); +} + +function clients(gc = true) { + const a = bound(createYDoc(null, {gc})); + a.doc.clientID = 1; + a.editor.update( + () => + $getRoot().append( + $createParagraphNode().append($createTextNode('alpha')), + ), + {discrete: true}, + ); + a.binding.initialize(); + const b = bound(createYDoc(Y.encodeStateAsUpdate(a.doc), {gc}), null); + b.doc.clientID = 2; + const sync = () => { + Y.applyUpdate(b.doc, Y.encodeStateAsUpdate(a.doc)); + Y.applyUpdate(a.doc, Y.encodeStateAsUpdate(b.doc)); + }; + a.binding.undoManager!.clear(); + const dispose = () => { + a.stop(); + b.stop(); + a.doc.destroy(); + b.doc.destroy(); + }; + return {a, b, dispose, sync}; +} + +function formats(doc: Y.Doc): Y.Item[] { + return [...doc.store.clients.values()] + .flat() + .filter( + (s): s is Y.Item => + s instanceof Y.Item && s.content instanceof Y.ContentFormat, + ); +} +const deleted = (doc: Y.Doc) => formats(doc).filter(s => s.deleted); +const kept = (doc: Y.Doc) => deleted(doc).filter(s => s.keep); + +test('a deleted own format item stays readable while its change can be undone', () => { + const {a, b, sync, dispose} = clients(); + try { + format(a.editor, true); + sync(); + format(b.editor, false); + sync(); + expect(kept(a.doc).length).toBeGreaterThan(0); + // A's state carries it while A can undo, as Yjs's UndoManager does for + // the items a step deletes; a relay that loads that state collects it. + const stored = Y.encodeStateAsUpdate(a.doc); + const relay = createYDoc(stored); + expect(deleted(relay)).toEqual([]); + relay.destroy(); + } finally { + dispose(); + } +}); + +test('a clear of both stacks releases what a collaborator deleted, which a reload collects', () => { + const {a, b, sync, dispose} = clients(); + try { + format(a.editor, true); + sync(); + format(b.editor, false); + sync(); + a.binding.undoManager!.clear(); + expect(kept(a.doc)).toEqual([]); + // Released, not collected (as Yjs leaves the items an UndoManager step + // kept): A's document holds the deleted content until it reloads, and a + // relay that loads A's state collects it. + const relay = createYDoc(Y.encodeStateAsUpdate(a.doc)); + expect(deleted(relay)).toEqual([]); + relay.destroy(); + } finally { + dispose(); + } +}); + +test('repeated cycles with a clear keep no format item kept', () => { + const {a, b, sync, dispose} = clients(); + try { + for (let i = 0; i < 200; i++) { + format(a.editor, true); + sync(); + format(b.editor, false); + sync(); + a.binding.undoManager!.clear(); + } + expect(formats(a.doc).filter(s => s.keep)).toEqual([]); + } finally { + dispose(); + } +}); + +test('an undo that changes nothing and empties both stacks releases them', () => { + const {a, b, sync, dispose} = clients(); + try { + for (let i = 0; i < 50; i++) { + format(a.editor, true); + sync(); + format(b.editor, false); + sync(); + // B removed the bold, so A's undo changes nothing and drops its step. + a.binding.undo(); + } + expect(a.binding.undoManager!.undoStack.length).toBe(0); + expect(a.binding.undoManager!.redoStack.length).toBe(0); + expect(kept(a.doc)).toEqual([]); + } finally { + dispose(); + } +}); + +test('a clear of the redo stack only keeps what the undo stack still needs', () => { + const {a, b, sync, dispose} = clients(); + try { + format(a.editor, true); + sync(); + a.editor.update( + () => void $getRoot().getAllTextNodes()[0].spliceText(2, 0, 'Z'), + {discrete: true}, + ); + sync(); + a.binding.undo(); + sync(); + a.binding.undoManager!.clear(false, true); + format(b.editor, false); + sync(); + expect(kept(a.doc).length).toBeGreaterThan(0); + } finally { + dispose(); + } +}); + +test('disposing the binding releases what it kept', () => { + const {a, b, sync, dispose} = clients(); + try { + format(a.editor, true); + sync(); + a.stop(); + format(b.editor, false); + sync(); + // Released before B's delete arrives, so that delete collects them. + expect(kept(a.doc)).toEqual([]); + expect(deleted(a.doc)).toEqual([]); + } finally { + dispose(); + } +}); + +test('a new binding on the same document does not inherit the old one keeps', () => { + const {a, b, sync, dispose} = clients(); + try { + format(a.editor, true); + sync(); + a.stop(); + const reopened = bound(a.doc); + reopened.binding.undoManager!.clear(); + format(b.editor, false); + sync(); + expect(kept(a.doc)).toEqual([]); + reopened.stop(); + } finally { + dispose(); + } +}); + +test('a reload from a relay shows what the client shows', () => { + const {a, b, sync, dispose} = clients(); + try { + format(a.editor, true); + sync(); + format(b.editor, false); + sync(); + const relay = createYDoc(Y.encodeStateAsUpdate(a.doc)); + const reload = bound(createYDoc(Y.encodeStateAsUpdate(relay)), null); + expect(JSON.stringify(reload.editor.getEditorState().toJSON())).toBe( + JSON.stringify(a.editor.getEditorState().toJSON()), + ); + reload.stop(); + reload.doc.destroy(); + relay.destroy(); + } finally { + dispose(); + } +}); + +for (const gc of [true, false]) { + for (const how of ['clear', 'dispose'] as const) { + test(`another binding on the same document can still undo after this one releases (${how}, gc ${gc ? 'on' : 'off'})`, () => { + const doc = createYDoc(null, {gc}); + doc.clientID = 1; + const a = bound(doc); + a.editor.update( + () => + $getRoot().append( + $createParagraphNode().append($createTextNode('alpha')), + ), + {discrete: true}, + ); + a.binding.initialize(); + a.binding.undoManager!.clear(); + const b = bound(doc); + const bold = () => + b.editor.read(() => + $getRoot() + .getAllTextNodes() + .map(t => `${t.getTextContent()}:${t.getFormat()}`) + .join('|'), + ); + try { + format(a.editor, true); + const before = bold(); + // B's step deletes A's format items locally, on the same document. + format(b.editor, false); + expect(bold()).not.toBe(before); + if (how === 'clear') { + a.binding.undoManager!.clear(); + } else { + a.stop(); + } + b.binding.undo(); + expect(bold()).toBe(before); + } finally { + b.stop(); + if (how === 'clear') { + a.stop(); + } + doc.destroy(); + } + }); + } +} + +for (const how of ['capture filter', 'untracked origin'] as const) { + test(`writes the undo stacks do not capture keep nothing (${how})`, () => { + const {a, dispose} = clients(); + try { + const um = a.binding.undoManager!; + if (how === 'capture filter') { + um.captureTransaction = () => false; + } else { + um.removeTrackedOrigin(a.binding); + } + for (let i = 0; i < 20; i++) { + format(a.editor, true); + } + expect(um.undoStack.length + um.redoStack.length).toBe(0); + expect(formats(a.doc).filter(s => s.keep)).toEqual([]); + } finally { + dispose(); + } + }); +} + +for (const gc of [true, false]) { + test(`an app UndoManager that tracks a provider's updates can still undo after the binding releases (gc ${gc ? 'on' : 'off'})`, () => { + const {a, b, dispose} = clients(gc); + const provider = {provider: true}; + const um = new Y.UndoManager(a.doc.get('lexical-y'), { + captureTimeout: 0, + trackedOrigins: new Set([provider]), + }); + const show = () => + a.editor.read(() => + $getRoot() + .getAllTextNodes() + .map(t => `${t.getTextContent()}:${t.getFormat()}`) + .join('|'), + ); + try { + format(a.editor, true); + Y.applyUpdate(b.doc, Y.encodeStateAsUpdate(a.doc)); + const before = show(); + format(b.editor, false); + Y.applyUpdate( + a.doc, + Y.encodeStateAsUpdate(b.doc, Y.encodeStateVector(a.doc)), + provider, + ); + expect(show()).not.toBe(before); + a.binding.undoManager!.clear(); + um.undo(); + expect(show()).toBe(before); + } finally { + um.destroy(); + dispose(); + } + }); +} + +for (const gc of [true, false]) { + test(`a release from inside another UndoManager's capture leaves its step restorable (gc ${gc ? 'on' : 'off'})`, () => { + const {a, b, dispose} = clients(gc); + const provider = {provider: true}; + const um = new Y.UndoManager(a.doc.get('lexical-y'), { + captureTimeout: 0, + trackedOrigins: new Set([provider]), + }); + // The app clears the binding's history while the other UndoManager + // records its step, inside the transaction that deleted the items. + um.on('stack-item-added', () => a.binding.undoManager!.clear()); + const show = () => + a.editor.read(() => + $getRoot() + .getAllTextNodes() + .map(t => `${t.getTextContent()}:${t.getFormat()}`) + .join('|'), + ); + try { + format(a.editor, true); + Y.applyUpdate(b.doc, Y.encodeStateAsUpdate(a.doc)); + const before = show(); + format(b.editor, false); + Y.applyUpdate( + a.doc, + Y.encodeStateAsUpdate(b.doc, Y.encodeStateVector(a.doc)), + provider, + ); + expect(show()).not.toBe(before); + um.undo(); + expect(show()).toBe(before); + } finally { + um.destroy(); + dispose(); + } + }); +} + +const flagged = (doc: Y.Doc) => formats(doc).filter(s => s.keep); + +test('disposing the binding while an empty-stacks release waits releases everything', () => { + const {a, dispose} = clients(); + try { + format(a.editor, true); + const um = a.binding.undoManager!; + a.binding.undo(); + // The next captured edit clears the redo stack (a release that waits + // for the transaction to end), and its capture disposes the binding. + um.on('stack-item-added', () => { + if (um.undoStack.length > 0) { + a.stop(); + } + }); + a.editor.update( + () => { + for (const n of $getRoot().getAllTextNodes()) { + n.setFormat('italic'); + } + }, + {discrete: true}, + ); + expect(a.binding.undoManager).toBe(null); + expect(flagged(a.doc)).toEqual([]); + } finally { + dispose(); + } +}); + +for (const order of ['dispose, then destroy', 'destroy, then dispose']) { + test(`destroying the document while a release waits releases everything (${order})`, () => { + const {a, dispose} = clients(); + try { + format(a.editor, true); + const um = a.binding.undoManager!; + a.doc.transact(() => { + um.clear(); + if (order === 'dispose, then destroy') { + a.stop(); + a.doc.destroy(); + } else { + a.doc.destroy(); + a.stop(); + } + }); + expect(flagged(a.doc)).toEqual([]); + } finally { + dispose(); + } + }); +} + +test('a binding disposed during its first capture keeps nothing', () => { + const {a, dispose} = clients(); + try { + expect(flagged(a.doc)).toEqual([]); + a.binding.undoManager!.on('stack-item-added', () => a.stop()); + format(a.editor, true); + expect(a.binding.undoManager).toBe(null); + expect(flagged(a.doc)).toEqual([]); + } finally { + dispose(); + } +}); + +for (const gc of [true, false]) { + test(`a binding started again before its old release ran keeps what its new step needs (gc ${gc ? 'on' : 'off'})`, () => { + const doc = createYDoc(null, {gc}); + doc.clientID = 1; + const a = bound(doc); + a.editor.update( + () => + $getRoot().append( + $createParagraphNode().append($createTextNode('alpha')), + ), + {discrete: true}, + ); + a.binding.initialize(); + a.binding.undoManager!.clear(); + // A collaborator whose UndoManager captures A's bold as it arrives. + const other = createYDoc(Y.encodeStateAsUpdate(doc), {gc}); + other.clientID = 3; + const provider = {provider: true}; + const um = new Y.UndoManager(other.get('lexical-y'), { + captureTimeout: 0, + trackedOrigins: new Set([provider]), + }); + let stop = a.stop; + try { + format(a.editor, true); + Y.applyUpdate( + other, + Y.encodeStateAsUpdate(doc, Y.encodeStateVector(other)), + provider, + ); + // Inside one transaction: dispose (its release waits), start again, + // and remove the bold, a step of the new lifetime. + doc.transact(() => { + a.stop(); + stop = a.binding.start(); + format(a.editor, false); + }, a.binding); + expect(a.binding.undoManager!.undoStack.length).toBe(1); + um.undo(); + Y.applyUpdate( + doc, + Y.encodeStateAsUpdate(other, Y.encodeStateVector(doc)), + ); + a.binding.undo(); + expect( + a.editor.read(() => + $getRoot() + .getAllTextNodes() + .map(n => `${n.getTextContent()}:${n.getFormat()}`) + .join('|'), + ), + ).toBe('al:0|ph:1|a:0'); + } finally { + stop(); + um.destroy(); + doc.destroy(); + other.destroy(); + } + }); +} diff --git a/packages/lexical-y/src/__tests__/unit/regressions/ReplacedLineCaret.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/ReplacedLineCaret.test.ts index 826b7a4d359..be6ecc76385 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/ReplacedLineCaret.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/ReplacedLineCaret.test.ts @@ -270,7 +270,7 @@ for (const ids of [ } // A styled line replaced: the delete of its text leaves the format items that -// bounded its runs live (see Cursor#collapseGap), and those show nothing. The +// bounded its runs live (see Cursor#delete), and those show nothing. The // caret took them for what the line still showed and went to the start of // the new line. for (const ids of [ diff --git a/packages/lexical-y/src/__tests__/unit/regressions/TypedTextAfterDeletedSpace.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/TypedTextAfterDeletedSpace.test.ts index c5415d662a0..269f40cbf0e 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/TypedTextAfterDeletedSpace.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/TypedTextAfterDeletedSpace.test.ts @@ -7,7 +7,8 @@ */ // @vitest-environment node -// After type-and-delete cycles at one place and a collapse, a client types +// After type-and-delete cycles at one place (which a delete collapsed before +// D86; it now keeps every format item it leaves), a client types // plain "X21" where a space was deleted; the space's deleter undoes. "X21" // takes no comment: it never had one. A collapse that kept a run end there // (#9617, parked) moved where the insert lands among the deleted items, and @@ -51,7 +52,7 @@ function $plain(l: number, off: number, text: string) { } } -test('text typed after a collapse takes no comment when a deleted space next to it comes back', async () => { +test('text typed after type-and-delete cycles takes no comment when a deleted space next to it comes back', async () => { const t = await setup( () => { words.forEach((ws, l) => { diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts index 8cd3cd31acd..ad79cb1aeb7 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoCommentRemovalSpread.test.ts @@ -155,55 +155,53 @@ function onRange( describe('undoing the removal of a comment after its author undid adding it, with a second comment and typing', () => { for (const typed of [true, false]) { for (const second of [true, false]) { - // Fails on main (#9623): pinned until the fix lands. - it.fails( - `keeps the comment off the paragraphs after it (second comment: ${second}, typed at its start: ${typed})`, - () => { - const {A, B, sync} = clients(); - onRange(A.editor, 0, 8, s => $addMarkID('c1', s)); - if (second) onRange(A.editor, 0, 3, s => $addMarkID('c2', s)); - if (typed) - onRange(A.editor, 0, 0, s => { - s.format = 0; - s.insertText(' new '); - }); - // A's comments and typing are one undo step. - A.step(); - sync(); - B.editor.update(() => $removeMarkID('c1'), {discrete: true}); - B.step(); - sync(); - A.editor.dispatchCommand(UNDO_COMMAND, undefined); - sync(); - B.editor.dispatchCommand(UNDO_COMMAND, undefined); - sync(); - const a = A.editor.getEditorState().read($show); - const b = B.editor.getEditorState().read($show); - const fresh = readYState(Y.encodeStateAsUpdate(A.doc), CONFIG, $show); - expect(b).toEqual(a); - expect(fresh).toEqual(a); - expect(a.slice(0, 2)).toEqual(PARAS.slice(0, 2)); - expect(a.slice(3)).toEqual(PARAS.slice(3)); - // And c1 is on exactly its words or nowhere: the comment's author - // undid it, and whether B's undo of its removal brings it back is - // undecided (it must not cover less, nor more). - const withC1 = A.editor.getEditorState().read(() => { - const block = $getRoot().getChildAtIndex(2); - return $isElementNode(block) - ? block - .getAllTextNodes() - .filter(n => - $getNodeMarkIDs(MarkStateExtension, n).includes('c1'), - ) - .map(n => n.getTextContent()) - .join('') - : '?'; + // Spread with garbage collection on (#9623) until the binding kept its + // own format items from collection while they are undoable. + it(`keeps the comment off the paragraphs after it (second comment: ${second}, typed at its start: ${typed})`, () => { + const {A, B, sync} = clients(); + onRange(A.editor, 0, 8, s => $addMarkID('c1', s)); + if (second) onRange(A.editor, 0, 3, s => $addMarkID('c2', s)); + if (typed) + onRange(A.editor, 0, 0, s => { + s.format = 0; + s.insertText(' new '); }); - expect(['', 'two two '], 'c1 on its words or nowhere').toContain( - withC1, - ); - }, - ); + // A's comments and typing are one undo step. + A.step(); + sync(); + B.editor.update(() => $removeMarkID('c1'), {discrete: true}); + B.step(); + sync(); + A.editor.dispatchCommand(UNDO_COMMAND, undefined); + sync(); + B.editor.dispatchCommand(UNDO_COMMAND, undefined); + sync(); + const a = A.editor.getEditorState().read($show); + const b = B.editor.getEditorState().read($show); + const fresh = readYState(Y.encodeStateAsUpdate(A.doc), CONFIG, $show); + expect(b).toEqual(a); + expect(fresh).toEqual(a); + expect(a.slice(0, 2)).toEqual(PARAS.slice(0, 2)); + expect(a.slice(3)).toEqual(PARAS.slice(3)); + // And c1 is on exactly its words or nowhere: the comment's author + // undid it, and whether B's undo of its removal brings it back is + // undecided (it must not cover less, nor more). + const withC1 = A.editor.getEditorState().read(() => { + const block = $getRoot().getChildAtIndex(2); + return $isElementNode(block) + ? block + .getAllTextNodes() + .filter(n => + $getNodeMarkIDs(MarkStateExtension, n).includes('c1'), + ) + .map(n => n.getTextContent()) + .join('') + : '?'; + }); + expect(['', 'two two '], 'c1 on its words or nowhere').toContain( + withC1, + ); + }); } } }); diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts index 8148401a6f8..7804e030ceb 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEndExtents.test.ts @@ -12,10 +12,11 @@ // back a value its author undid is undecided, #9623); it never covers more: // a collaborator removes a value, its author undoes it (and may write the // same key later), a third client may delete text around the value's end, -// and the collaborator then undoes the removal. On main, with garbage -// collection on, the restored value runs on past its end (#9623), and with -// many comment ids or a concurrent undo of the end's change too (#9620): -// those cells are pinned as test.fails until the fix lands. +// and the collaborator then undoes the removal. With garbage collection on, +// the restored value ran on past its end (#9623), and with many comment ids +// or a concurrent undo of the end's change too (#9620): a collaborator's +// delete collected the format items the undo reads, until the binding kept +// its own format items from collection while they are undoable. import {$toggleLink, LinkNode} from '@lexical/link'; import {$addMarkID, $getNodeMarkIDs, $removeMarkID} from '@lexical/mark'; import {$patchStyleText} from '@lexical/selection'; @@ -41,15 +42,15 @@ import { } from '../../utils/harness'; import {markState, prepareMarkState} from '../../utils/sizeOps'; -/** The cells main fails (#9623, #9620), pinned until the fix lands. */ -function failsOnMain(name: string): boolean { - return ( - (name.includes('gc=true') && !name.includes('author keeps')) || - name.startsWith('concurrent') - ); -} +/** + * Every cell runs with garbage collection on and off. The gc=true cells + * failed (#9623) until the binding kept its own format items from collection + * while they are undoable; the concurrent cells failed while a delete + * collapsed the run it left (it dropped the end the restore relied on), and a + * delete now keeps every format item it leaves (D86). + */ function testOnMain(name: string, fn: () => Promise) { - (failsOnMain(name) ? test.fails : test)(name, fn); + test(name, fn); } const STATE = markState('review9633'); diff --git a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts index 6d3d3c9bb48..06278d8321a 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoRestoredRunEnds.test.ts @@ -281,16 +281,17 @@ const REMOVALS: Removal[] = [ ]; /** - * The cells main fails, pinned as test.fails until the fixes land: with - * garbage collection on, the restored value runs on past its end (#9623); + * The cells that still fail, pinned as test.fails until their fix lands: * where the removal took part of the word, the author's undo leaves the rest * of the word with the value the removal restated after that part (D64, - * #9560), and the removal's undo gives the whole word back. + * #9560), and the removal's undo gives the whole word back. With garbage + * collection on, the restored value ran on past its end (#9623) until the + * binding kept its own format items from collection while they are undoable. */ function testOnMain(name: string, fn: () => void) { const fails = name.includes('its author undoes it first') && - (name.includes('gc on') || name.includes('part of the word')); + name.includes('part of the word'); (fails ? test.fails : test)(name, fn); } @@ -427,40 +428,39 @@ describe('an undo that brings back a value while its end goes at the same time ( [1, 2], [2, 1], ]) { - // Fails on main (#9620): pinned until the fix lands. - test.fails( - `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"'); - }, - ); + // Failed while a delete collapsed the run it left (#9620: the collapse + // dropped the comment's end); a delete now keeps every format item it + // leaves (D86). + 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/__tests__/unit/regressions/UndoSpreadSession2810.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/UndoSpreadSession2810.test.ts new file mode 100644 index 00000000000..985c6bbfff4 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/UndoSpreadSession2810.test.ts @@ -0,0 +1,222 @@ +/** + * 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 downstream app's undo fuzz (two clients, one comment id per text node, +// sync after every edit), session 2810: with garbage collection on, a +// comment came back over four paragraphs after the two clients' undos once +// a delete kept every format item it leaves (D86), until the binding kept +// its own format items from collection while they are undoable (D88). The +// session's own ops, replayed: no comment may end up on two paragraphs. + +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 CONFIG = { + extensions: [RichTextExtension, MarkStateExtension], + schemaVersion: 1, +}; +const WORDS = ['alpha', 'beta', 'gamma', 'delta', 'note', 'run']; + +type Op = {e: 0 | 1} & ( + | {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'} +); + +// The session up to the op after which the comment spread. +const OPS: Op[] = [ + {a: 0, b: 4, e: 1, f: 'bold', k: 'format', t: 7}, + {a: 0, e: 1, k: 'type', s: 'yy', t: 5}, + {a: 14, b: 23, e: 0, id: 'c1', k: 'add', t: 10}, + {a: 8, b: 14, e: 1, id: 'c2', k: 'add', t: 4}, + {a: 2, b: 3, e: 1, k: 'del', t: 4}, + {e: 0, id: 'c2', k: 'remove'}, + {a: 21, b: 28, e: 0, f: 'italic', k: 'format', t: 2}, + {a: 0, b: 2, e: 0, k: 'del', t: 17}, + {a: 0, b: 1, e: 0, id: 'c3', k: 'add', t: 6}, + {a: 0, e: 1, k: 'type', s: 'x', t: 6}, + {a: 13, b: 14, e: 0, k: 'del', t: 7}, + {a: 1, b: 7, e: 0, k: 'del', t: 17}, + {a: 24, b: 35, e: 1, id: 'c4', k: 'add', t: 3}, + {e: 1, k: 'undo'}, + {a: 2, b: 5, e: 1, k: 'del', t: 6}, + {e: 0, 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 on more than one paragraph. */ +function spread(editor: LexicalEditor): string[] { + return editor.getEditorState().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, s]) => `${id}:${s.size}`); + }); +} + +for (const gc of [true, false]) { + for (const ids of [ + [1, 2], + [2, 1], + ]) { + test(`no comment spreads over paragraphs (gc ${gc ? 'on' : 'off'}, ids ${ids})`, () => { + const seeded = seedYState(CONFIG, () => { + const root = $getRoot().clear(); + for (let i = 0; i < 6; i++) { + root.append( + $createParagraphNode().append( + $createTextNode(`${WORDS[i]} one two three four five ${i}`), + ), + ); + } + }); + const peers = ids.map((id, n) => { + const doc = createYDoc(seeded, {gc}); + doc.clientID = id; + const editor = buildEditorFromExtensions( + defineExtension({ + dependencies: [ + ...CONFIG.extensions, + configExtension(YCollabExtension, { + // Every edit of a client until its undo is one step, as in + // the session (it ran well inside the capture timeout). + captureTimeout: 2 ** 30, + doc, + schemaVersion: 1, + synced: true, + }), + ], + name: `undo-spread-2810-${n}`, + onError: e => { + throw e; + }, + }), + ); + 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)), + ); + }; + try { + for (const op of OPS) { + const {editor} = peers[op.e]; + if (op.k === 'undo') { + editor.dispatchCommand(UNDO_COMMAND, undefined); + } else { + editor.update(() => $apply(op), {discrete: true}); + } + sync(); + expect(peers.map(p => spread(p.editor))).toEqual([[], []]); + } + } finally { + for (const p of peers) { + p.editor.dispose(); + } + } + }); + } +} diff --git a/packages/lexical-y/src/__tests__/utils/harness.ts b/packages/lexical-y/src/__tests__/utils/harness.ts index 084fe42b605..247cfc03a91 100644 --- a/packages/lexical-y/src/__tests__/utils/harness.ts +++ b/packages/lexical-y/src/__tests__/utils/harness.ts @@ -230,8 +230,10 @@ export class Peer { /** Runs on the editor before the binding starts. */ readonly prepare?: (editor: LexicalEditor) => void, readonly withBlockIds: boolean = WITH_BLOCK_IDS, + /** Yjs garbage collection (products use Yjs's default, on). */ + readonly gc: boolean = WITH_GC, ) { - this.doc = createYDoc(null, {gc: false}); + this.doc = createYDoc(null, {gc}); this.doc.clientID = clientID; const bound = createBoundEditor(this.doc, { namespace: 'test', @@ -356,6 +358,7 @@ export async function setup( nodes?: Klass[]; prepare?: (editor: LexicalEditor) => void; withBlockIds?: boolean; + gc?: boolean; } = {}, ): Promise { const net: Network = { @@ -367,7 +370,14 @@ export async function setup( const ids = options.ids ?? [1, 2]; const peers = ids.map( id => - new Peer(id, net, options.nodes, options.prepare, options.withBlockIds), + new Peer( + id, + net, + options.nodes, + options.prepare, + options.withBlockIds, + options.gc, + ), ); const [a] = peers; a.update(() => { @@ -549,6 +559,11 @@ export function mainNormalizes(peer: Peer): string | null { * YCollabExtension and BlockIdExtension, as an app that uses both does. */ export const WITH_BLOCK_IDS = process.env.LEXICAL_Y_BLOCK_ID === '1'; +/** + * Peers' Yjs garbage collection: off unless a test asks for it, or + * `LEXICAL_Y_GC=1` runs every harness test with it on (as products run). + */ +export const WITH_GC = process.env.LEXICAL_Y_GC === '1'; /** * An editor bound to `doc`: `createEditor` and a `YBinding` (started by diff --git a/packages/lexical-y/src/internal/Binding.ts b/packages/lexical-y/src/internal/Binding.ts index 873e6911da4..74dedd91d95 100644 --- a/packages/lexical-y/src/internal/Binding.ts +++ b/packages/lexical-y/src/internal/Binding.ts @@ -306,6 +306,17 @@ const MAX_PENDING_COMMITS = 16; */ const MAX_REGISTRATION_RUNS = 100; +/** + * The bindings that keep an item from garbage collection (D88), per item, and + * whether it was kept already when the first of them did (by an UndoManager's + * step): the item is released when the last of them releases it, and only + * if nothing else kept it before. + */ +const keepOwners = new WeakMap< + Item, + {external: boolean; owners: Set} +>(); + /** What an update listener gets for a commit (see YBinding#onUpdate). */ type UpdatePayload = { dirtyElements: Map; @@ -1040,7 +1051,7 @@ export class YBinding { // Yjs cleans up formats after a remote transaction by rescanning every // type that has formats (whole containers here, so O(document) per remote // bold). The binding keeps format items a write leaves redundant that - // a concurrent write may rely on (see Cursor#collapseGap), so its + // a concurrent write may rely on (see Cursor#delete), so its // containers opt out: they are marked as having no formats before Yjs // decides whether to clean up. A document createYDoc made never cleans // up; this keeps one that does (read only, see unsupportedConfig) from @@ -1110,6 +1121,164 @@ export class YBinding { trackedOrigins: new Set([this]), }); this._undoManager = um; + // Undo reads the format items of the changes on its stacks, and the + // end each of them gave a run it deleted (see isKeptByUndo, + // closeRestoredRuns, changeEnd), after a collaborator deleted them: + // keep those from garbage collection (Item.keep, as UndoManager keeps + // the items a change deletes) so a collected item's key and value + // stay readable (#9623). The flag is local; while an item is kept, + // this document still holds its content (D88). + // What this binding keeps is released when both stacks are empty + // (after a clear, an undo that leaves nothing to redo, or a write the + // stacks did not capture) and when the binding is disposed. Another + // binding on the same document, or an UndoManager, may keep the same + // item: it stays kept until every binding that kept it released it. + // Releasing collects nothing: an item deleted while kept keeps its + // content until the document reloads, as Yjs leaves the items an + // UndoManager step kept (any UndoManager on the document may still + // restore it). + const kept = new Set(); + // This listening lifetime: a binding started again after a dispose + // owns its keeps anew, apart from a release the old one queued. + const owner = {}; + const keep = (item: Item | null) => { + for (let it = item; it !== null; it = (it.parent as YNode)._item) { + let entry = keepOwners.get(it); + if (entry === undefined) { + // `external`: kept already (by an UndoManager's step), which + // this leaves to whoever kept it. + entry = {external: it.keep, owners: new Set()}; + keepOwners.set(it, entry); + } else if (entry.owners.has(owner)) { + // Its parents are this lifetime's already. + break; + } + entry.owners.add(owner); + it.keep = true; + kept.add(it); + } + }; + // Released only between transactions: inside one (another + // UndoManager's stack event, a listener) the transaction's own + // collection, which runs after its listeners, would still collect an + // item it deleted that another UndoManager has just kept. + // A queued release keeps the stronger request (dispose releases + // whatever the stacks hold), and a document destroyed meanwhile + // releases at once (its listeners, the queued one too, are gone). + let queued: 'empty' | 'always' | null = null; + let disposed = false; + const inTransaction = () => + this._doc._transaction !== null || + this._doc._transactionCleanups.length > 0; + const releaseNow = () => { + for (const it of kept) { + const entry = keepOwners.get(it); + if (entry !== undefined) { + entry.owners.delete(owner); + if (entry.owners.size > 0) { + continue; + } + keepOwners.delete(it); + if (entry.external) { + continue; + } + } + it.keep = false; + } + kept.clear(); + if (disposed) { + this._doc.off('destroy', onDestroy); + } + }; + const release = (onlyWhenEmpty: boolean) => { + if (kept.size === 0) { + if (disposed) { + this._doc.off('destroy', onDestroy); + } + return; + } + if ( + onlyWhenEmpty && + (um.undoStack.length > 0 || um.redoStack.length > 0) + ) { + return; + } + if (inTransaction() && !this._doc.isDestroyed) { + if (queued === null) { + this._doc.once('afterAllTransactions', () => { + const always = queued === 'always'; + queued = null; + release(!always); + }); + } + if (queued !== 'always') { + queued = onlyWhenEmpty ? 'empty' : 'always'; + } + return; + } + releaseNow(); + }; + const onDestroy = () => { + disposed = true; + queued = null; + releaseNow(); + }; + this._doc.on('destroy', onDestroy); + const releaseWhenEmpty = () => release(true); + um.on('stack-cleared', () => { + releaseWhenEmpty(); + // A clear of one stack makes the UndoManager release what that + // stack's steps kept, parents included: keep what this binding + // still relies on. + for (const it of kept) { + if (!it.keep) { + it.keep = true; + } + } + }); + this.disposers.push(() => { + disposed = true; + release(false); + }); + const keepFormats = (tr: Transaction) => { + // A listener call already under way when the binding was disposed + // keeps nothing. + if (!tr.local || disposed) { + return; + } + if (tr.origin === um) { + // An undo or redo that leaves both stacks empty (one that changes + // nothing pops its step without an event). + releaseWhenEmpty(); + } + if (tr.origin !== this && tr.origin !== um) { + return; + } + const ends: Item[] = []; + iterateStructsByIdSet(tr, tr.deleteSet, s => { + if ( + s instanceof Item && + s.content instanceof ContentFormat && + s.content.value !== null + ) { + const end = changeEnd(this._doc, s); + if (end !== null) { + ends.push(end); + } + } + }); + ends.forEach(keep); + iterateStructsByIdSet(tr, tr.insertSet, s => { + if (s instanceof Item && s.content instanceof ContentFormat) { + keep(s); + } + }); + // A write the stacks did not capture (a capture filter, an origin no + // longer tracked) leaves nothing an undo could read. + releaseWhenEmpty(); + }; + this._doc.on('afterTransaction', keepFormats); + this.disposers.push(() => this._doc.off('afterTransaction', keepFormats)); // 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 diff --git a/packages/lexical-y/src/internal/Cursor.ts b/packages/lexical-y/src/internal/Cursor.ts index cdc638f1cfc..abb390322fd 100644 --- a/packages/lexical-y/src/internal/Cursor.ts +++ b/packages/lexical-y/src/internal/Cursor.ts @@ -44,13 +44,13 @@ import {deleteItem, isGone} from './Seen'; * licensed), which are not exported. Only the parts this binding needs are * implemented: no renderer (attribution) support. A sequence of edits shows * the same content and formats as through Yjs (Cursor.test checks it against - * Y.Node#applyDelta), but the items differ: a delete keeps, in the run of - * deleted content and format items it leaves, a bounded number of the format - * items Yjs removes as redundant, which concurrent writers rely on (see - * collapseGap), and a format change keeps the items that bound the content - * around its range, opens its value next to its content and closes it where - * the change that gave the range its old value ends (see format), all per - * family of keys (see formatFamily); an undo keeps some of the closers a + * Y.Node#applyDelta), but the items differ: a delete keeps every format item + * in the run of deleted content and format items it leaves, which Yjs removes + * as redundant and concurrent writers rely on (see delete), and a format + * change keeps the items that bound the content around its range, opens its + * value next to its content and closes it where the change that gave the + * range its old value ends (see format), all per family of keys (see + * formatFamily); an undo keeps some of the closers a * format change wrote (see YBinding#isKeptByUndo). */ @@ -245,6 +245,13 @@ export function runOpener(doc: Doc, item: Item): Item { if (!it.deleted && isFormat(it) && keyOf(it) === key) { break; } + // A restore follows text its own client inserted inside the run, + // after the null (see insertNegatedFormats): content of another + // client between them means `current` opens a run of its own (its + // client's earlier end and later opener, #9599). + if (!isFormat(it) && it.id.client !== current.id.client) { + break; + } } const source = found ? liveItemBefore(negation, key) : null; if (source === null || !equalFormatValue(valueOf(source), value)) { @@ -304,19 +311,6 @@ export class Cursor { * wherever the cursor looks for format items. */ hidden: Hidden | null = null; - /** - * Where the run of items without live content that the cursor is in - * starts (see collapseGap): after this item, or at the parent's start for - * null. The live content before it, or the item the cursor was made after - * when it has passed no live content since (and that item is not live). - */ - private gapStart: Item | null; - /** - * The value each key had at gapStart, for the keys whose format items the - * cursor passed since; any other key still has the value it has at the - * cursor. - */ - private readonly gapFormats: Formats = new Map(); /** * The live format item that gave each key its value at the cursor, for * the keys whose format items the cursor passed. @@ -337,7 +331,6 @@ export class Cursor { this.left = left; this.right = left === null ? parent._start : left.right; this.formats = formats; - this.gapStart = left; // Index search markers would not follow these item-level edits. if (parent._searchMarker !== null) { parent._searchMarker.length = 0; @@ -357,10 +350,8 @@ export class Cursor { this.right = item.right; return; } - if (isLiveContent(item)) { - this.gapStart = item; - this.gapFormats.clear(); - } else if ( + if ( + !isLiveContent(item) && this.passed !== null && isFormat(item) && !this.tr.insertSet.hasId(item.id) @@ -374,9 +365,6 @@ export class Cursor { } if (!isGone(item) && isFormat(item)) { const {key, value} = item.content as ContentFormat; - if (!this.gapFormats.has(key)) { - this.gapFormats.set(key, this.formats.get(key) ?? null); - } this.sources.set(key, item); if (value === null) { this.formats.delete(key); @@ -863,12 +851,11 @@ export class Cursor { } /** - * Delete `n` content units, and collapse the format items of the run of - * items without live content that the delete leaves (see collapseGap). + * Delete `n` content units. The format items around and inside them stay: + * a concurrent writer that has not seen the delete relies on them, and an + * undo of the delete brings the content back between them (D24, D86). */ delete(n: number): void { - let first: Item | null = null; - let last: Item | null = null; while (n > 0 && this.right !== null) { const item = this.right; if (this.counts(item)) { @@ -877,274 +864,34 @@ export class Cursor { } n -= item.length; deleteItem(item, this.tr); - first = first ?? item; - last = item; } this.forward(); } - if (first !== null && last !== null) { - this.collapseGap(first, last); - } } /** - * Collapse the format items of the run of items without live content (deleted - * content and format items) that a delete of the content from `first` to - * `last` leaves the cursor in. Per family (see formatFamily), keep the units - * (see unitFrom) this delete leaves: those inside the content it deletes, the - * last before it and the first after it; and of the others, which earlier - * deletes left, the first that changes the family's value the content before - * the run has, and the last two. Where a unit is kept, also keep, of each key - * of its family, the last item before the end of that place (a deleted item - * or content ends it) where without it the key would have another value - * there: a format change keeps the item before its opener at a place (see - * format), and with several keys a whole value's item there gives every key - * its value. Delete the items of the rest, except, of each key, the last item - * where without it the key would have another value after the run, and, - * where the key's last item is kept, the last item before it of that key with - * the same value (no other value between them) that another client wrote. - * Invariant: - * such a run holds, besides the units of the last delete in it, at most five - * units per family, the items that give a family its value at the end of the - * places of those, and items that change a key's value between the content - * before the run and the content after it, whatever was written and deleted - * there before. Why: a delete that leaves the format items of the content it - * deletes live grows the items a document stores with its editing history - * (each word of another format, comment or NodeState value typed at one place - * and deleted again left two more: 2,000 words left 4,000; with a set stored - * one key per member, each new comment a key of its own). And why those: a - * concurrent writer that has not seen the delete relies on them. The units of - * the content a delete removes give text a peer types inside it, which writes - * no item where its value matches, its value (a line deleted while a peer - * typed plain text after an italic word in it made that text italic, when the - * word's closer went); the first unit ends the value of the content before - * the run (a peer's text typed after that content without it, or a peer's - * format change that ends its range there); the units around the deleted - * content give text a peer typed at either of its edges, with its value or - * without it, that value or the one after it; the last unit gives the content - * after the run its value (two peers deleting the characters before a space - * at once each left the space's own item, which the other relied on); and the - * one before it gives that content the value it falls back to where a - * concurrent format change of it removes its item (a comment resolved on a - * character took the comment of the text deleted before it). The other - * client's item with the key's last value stays as each client's undo deletes - * its own items only: a run's end and the null another client wrote right - * after it (see format, rule 5) each end the run where the other's writer - * removes theirs (a paste over plain text typed after a word two comments - * cover kept only the typist's null of one comment, and the typist's - * concurrent undo left that comment on to the end of the document). The - * items a kept - * unit's place holds for the other keys of its family give content restored - * there, as an undo of its delete restores it, the value it had: a word two - * clients gave a comment each, deleted, kept one comment's opener once a - * peer's later delete next to it collapsed the run, so the undo brought it - * back with one comment (each client's own writes are undone last first, so - * its own later delete is undone before its earlier one). Yjs' deleteText - * keeps the last item only, and not even that one where it repeats the value - * before the run (its cleanupFormattingGap), which lost the value in each of - * those cases. What the bound gives up: text a peer types inside content an - * earlier delete in the run removed, once a later delete collapsed it, takes - * the value that content had at its start where its value changed more than - * once along it, and text typed at the edges of such content can take the - * value of content deleted later; content an undo restores there, of a delete - * a peer's later delete collapsed the run of, can take those values too. + * Whether format item `item` is one this client wrote, before this + * transaction, right next to text of its own in the same insert: the null + * right before the text (`before`: the text's origin is the null) or the + * restore right after it (the restore's origin is the text's last unit). */ - private collapseGap(first: Item, last: Item): void { - const start = - this.gapStart === null ? this.parent._start : this.gapStart.right; - const byFamily = new Map(); - // The format items left of the cursor (their values are in formats). - const passed = new Set(); - // Per family, the units this delete leaves: those inside the deleted - // content, the last before it and the first after it. - const around = new Set(); - const lastBefore = new Map(); - const firstAfter = new Set(); - let leftOfCursor = true; - let phase = 0; - let prev: Item | null = null; - let prevUnit: Item[] | null = null; - // The place of each unit: a deleted item or content ends a place. - const placeOf = new Map(); - let place = 0; - for (let it = start; it !== null && !isLiveContent(it); it = it.right) { - if (it === this.right) { - leftOfCursor = false; - } - if (this.isHeldFormat(it)) { - continue; - } - if (it === first) { - phase = 1; - for (const kept of lastBefore.values()) { - around.add(kept); - } - } - let unit: Item[] | null = null; - if (isGone(it) || !isFormat(it)) { - place++; - } else { - const family = formatFamily(keyOf(it)); - if ( - prev !== null && - prevUnit !== null && - continuesUnit(prevUnit, prev, it) - ) { - unit = prevUnit; - unit.push(it); - } else { - unit = [it]; - placeOf.set(unit, place); - const units = byFamily.get(family); - if (units === undefined) { - byFamily.set(family, [unit]); - } else { - units.push(unit); - } - } - if (leftOfCursor) { - passed.add(it); - } - if (phase === 0) { - lastBefore.set(family, unit); - } else if (phase === 1) { - around.add(unit); - } else if (!firstAfter.has(family)) { - firstAfter.add(family); - around.add(unit); - } - } - prev = it; - prevUnit = unit; - if (it === last) { - phase = 2; - } + private isOwnInsertFormat(item: Item, before: boolean): boolean { + const doc = this.tr.doc; + const client = doc.clientID; + if (item.id.client !== client || this.tr.insertSet.hasId(item.id)) { + return false; } - const atStart = (key: string): unknown => - this.gapFormats.has(key) - ? (this.gapFormats.get(key) ?? null) - : (this.formats.get(key) ?? null); - for (const units of byFamily.values()) { - if (units.length < 3) { - continue; - } - const keep = new Set(); - const keepUnit = (unit: Item[] | undefined) => { - for (const it of unit ?? []) { - keep.add(it); - } - }; - keepUnit( - units.find(unit => - unit.some(it => !equalFormatValue(valueOf(it), atStart(keyOf(it)))), - ), - ); - for (const unit of units) { - if (around.has(unit)) { - keepUnit(unit); - } - } - keepUnit(units[units.length - 2]); - keepUnit(units[units.length - 1]); - // Where a unit is kept, the value its family's keys have at the end of - // that place stays (a format change keeps the item before its opener - // there, and with several keys a whole value's item there gives every - // key its value): those keys' last items before that end are kept too. - { - const lastAt = new Map(); - const keptValue = new Map(); - for (let i = 0; i < units.length; ) { - let kept = false; - let j = i; - for ( - ; - j < units.length && placeOf.get(units[j]) === placeOf.get(units[i]); - j++ - ) { - for (const it of units[j]) { - lastAt.set(keyOf(it), it); - if (keep.has(it)) { - kept = true; - keptValue.set(keyOf(it), valueOf(it)); - } - } - } - if (kept) { - for (const [key, it] of lastAt) { - const value = keptValue.has(key) - ? keptValue.get(key) - : atStart(key); - if (!equalFormatValue(value, valueOf(it))) { - keep.add(it); - keptValue.set(key, valueOf(it)); - } - } - } - i = j; - } - } - // The value of each key after the run stays; where the key's last item - // stays, so does the last item before it with the same value that - // another client wrote. - const lastOf = new Map(); - const lastKept = new Map(); - const otherClient = new Map(); - for (const unit of units) { - for (const it of unit) { - const key = keyOf(it); - const before = lastOf.get(key); - if (before !== undefined) { - if (!equalFormatValue(valueOf(before), valueOf(it))) { - otherClient.delete(key); - } else if (before.id.client !== it.id.client) { - otherClient.set(key, before); - } - } - lastOf.set(key, it); - if (keep.has(it)) { - lastKept.set(key, it); - } - } - } - for (const [key, it] of lastOf) { - const kept = lastKept.get(key); - if ( - !equalFormatValue( - kept === undefined ? atStart(key) : valueOf(kept), - valueOf(it), - ) - ) { - keep.add(it); - } - const other = otherClient.get(key); - if (keep.has(it) && other !== undefined) { - keep.add(other); - } - } - const values = new Map(); - const changed = new Set(); - for (const unit of units) { - for (const it of unit) { - if (!keep.has(it)) { - deleteItem(it, this.tr); - if (passed.has(it)) { - changed.add(keyOf(it)); - } - } else if (passed.has(it)) { - values.set(keyOf(it), valueOf(it)); - } - } - } - // The value at the cursor: the last item left of it that stays. - for (const key of changed) { - const value = values.has(key) ? values.get(key) : atStart(key); - if (value === null) { - this.formats.delete(key); - } else { - this.formats.set(key, value); - } - } + const clock = before ? item.id.clock + item.length : item.id.clock - 1; + if (clock < 0 || clock >= doc.store.getClock(client)) { + return false; + } + const text = doc.store.getItem(createID(client, clock)); + if (!(text instanceof Item) || text.content instanceof ContentFormat) { + return false; } + const origin = before ? text.origin : item.origin; + const want = before ? item.id.clock + item.length - 1 : clock; + return origin !== null && origin.client === client && origin.clock === want; } /** @@ -1240,8 +987,8 @@ export class Cursor { * is that change's own end of the run (see changeEnd), it writes a null * of its own right after that end. Why: that end can go while the * restore stays, as the undo of that change deletes it and a concurrent - * delete may drop it (it keeps a bounded number of the items it leaves, - * see collapseGap), and the restored value ran on to the end of the + * delete could drop it (before deletes kept every item they leave, see + * delete), and the restored value ran on to the end of the * document (a format undone while a collaborator typed plain text inside * it, pasted a list there or removed it from part of the range; a * comment undone while a collaborator typed right after it where another @@ -1333,6 +1080,23 @@ export class Cursor { this.forward(); continue; } + if ( + !inRange && + n > 0 && + !opened.has(key) && + prev !== null && + equalFormatValue(prev, attr) && + this.isOwnInsertFormat(r, true) + ) { + // The null this client's own earlier insert wrote before its + // text, which the range covers with the value before it: + // removed, as Yjs' formatText does, so the run goes on over + // the range and no one relies on it (#9671). + deleteItem(r, this.tr); + negated.set(key, value); + this.forward(); + continue; + } if ( !inRange && !opened.has(key) && @@ -1350,6 +1114,15 @@ export class Cursor { if (equalFormatValue(attr, value)) { if (n === 0) { if (attr !== null) { + if (this.isOwnInsertFormat(r, false)) { + // The restore this client's own earlier insert wrote + // after its text, with the range's value: removed, as + // Yjs' formatText does, the runs joined (#9671). + negated.delete(key); + deleteItem(r, this.tr); + this.forward(); + continue; + } // Rule 3. The opener of the content after the range: kept, // and the range closed before it. break loop; @@ -1431,6 +1204,7 @@ export class Cursor { // at all where that is the range's value, or with that item itself // where its value is the value from before (the restore reuses it). const ahead = new Map(); + const copiedEnds: Item[] = []; for ( let it = this.right; it !== null && !isLiveContent(it); @@ -1462,10 +1236,29 @@ export class Cursor { formatClosers.add(this.right); } this.forward(); + // Rule 5 for this copy of another client's opener: it restates + // that opener's run, whose change's undo deletes the opener and + // the run's end (see changeEnd) but not the copy, which would run + // on over the text after it (#9622). End it right after that end + // too, with a null of this write's own. + if (value !== null && next.id.client !== this.tr.doc.clientID) { + const end = changeEnd(this.tr.doc, next); + if (end !== null && !end.deleted && valueOf(end) === null) { + copiedEnds.push(end); + } + } } } } this.passed = null; this.insertNegatedFormats(negated, true); + for (const end of copiedEnds) { + formatClosers.add( + this.insertAfter( + end, + new ContentFormat(keyOf(end), null as unknown as object), + ), + ); + } } } diff --git a/packages/lexical-y/src/internal/Schema.ts b/packages/lexical-y/src/internal/Schema.ts index 1e589c5db52..c11e4ec5947 100644 --- a/packages/lexical-y/src/internal/Schema.ts +++ b/packages/lexical-y/src/internal/Schema.ts @@ -96,7 +96,7 @@ export const WRAPPER_PREFIX = 'w:'; /** * The family of a format key: the keys whose items together give content * one attribute, which the write cursor bounds and keeps at boundaries as - * one (see Cursor#collapseGap and Cursor#format). A set-valued state's + * one (see Cursor#format). A set-valued state's * member entries (`s:\u001f`, see setMemberEntry) are the * family of the set's key (`s:`), with an array an earlier build * stored whole under that key (see joinSetMembers): together they give diff --git a/packages/lexical-y/src/internal/Write.ts b/packages/lexical-y/src/internal/Write.ts index bd1feb21e37..9dec2ec4a72 100644 --- a/packages/lexical-y/src/internal/Write.ts +++ b/packages/lexical-y/src/internal/Write.ts @@ -2456,10 +2456,10 @@ export class Writer { * write computes the format items it writes from the formats it starts * from; from the start of a unit after a deleted block node (inside a * format run around that node), a delete, which then removed the format - * items it found redundant (see Cursor#collapseGap), removed the opener - * of a run that goes on after the deleted content, and every reader - * showed that text without the format while the author kept it (also on - * a follow-up's delete). + * items it found redundant (before deletes kept them, see Cursor#delete), + * removed the opener of a run that goes on after the deleted content, and + * every reader showed that text without the format while the author + * kept it (also on a follow-up's delete). */ private $deleteUnit(old: Unit[], idx: number): void { const u = old[idx]; diff --git a/packages/lexical-y/src/internal/YDoc.ts b/packages/lexical-y/src/internal/YDoc.ts index f779b9b0b35..aaf6c8a0c62 100644 --- a/packages/lexical-y/src/internal/YDoc.ts +++ b/packages/lexical-y/src/internal/YDoc.ts @@ -11,7 +11,7 @@ // Invariant: a document the binding writes never runs Yjs's format cleanup, // from its creation on. Why: after a remote change, the cleanup deletes the // format items it finds redundant, among them the ones left where text was -// deleted. The binding keeps those (see Cursor#collapseGap): text a +// deleted. The binding keeps those (see Cursor#delete): text a // collaborator types into the deleted text at the same time keeps the // formats it was typed with only while they are there. The cleanup is also // a write of this client's that reaches every peer. A document that ran it diff --git a/packages/lexical-y/src/internal/YjsContract.ts b/packages/lexical-y/src/internal/YjsContract.ts index 3fe542c4f33..4aa4f73784b 100644 --- a/packages/lexical-y/src/internal/YjsContract.ts +++ b/packages/lexical-y/src/internal/YjsContract.ts @@ -159,8 +159,10 @@ export function yjsMembers() { clientID: (d: Doc) => isNumber(d.clientID), destroy: (d: Doc) => isFunction(d.destroy), get: (d: Doc) => isFunction(d.get), + isDestroyed: (d: Doc) => isBoolean(d.isDestroyed), off: (d: Doc) => isFunction(d.off), on: (d: Doc) => isFunction(d.on), + once: (d: Doc) => isFunction(d.once), share: (d: Doc) => isMap(d.share), store: (d: Doc) => isObject(d.store), transact: (d: Doc) => isFunction(d.transact), @@ -194,6 +196,7 @@ export function yjsMembers() { deleted: (it: Item) => isBoolean(it.deleted), id: (it: Item) => isObject(it.id), integrate: (it: Item) => isFunction(it.integrate), + keep: (it: Item) => isBoolean(it.keep), lastId: (it: Item) => isObject(it.lastId), left: (it: Item) => isSet(it.left), length: (it: Item) => isNumber(it.length), @@ -230,6 +233,7 @@ export function yjsMembers() { deleteSet: (tr: Transaction) => isObject(tr.deleteSet), doc: (tr: Transaction) => isObject(tr.doc), insertSet: (tr: Transaction) => isObject(tr.insertSet), + local: (tr: Transaction) => isBoolean(tr.local), origin: (tr: Transaction) => isSet(tr.origin), }, UndoManager: { @@ -282,6 +286,7 @@ export const YJS_EVENTS = { 'afterTransaction', 'beforeObserverCalls', 'beforeTransaction', + 'destroy', 'update', ], UndoManager: ['stack-cleared', 'stack-item-added', 'stack-item-popped'],