From 8bcae9b205cceb82f7858269c4e38ef742712c31 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 20:08:54 -0700 Subject: [PATCH 1/7] [lexical-y] Bug Fix: Read an inline element around blocks only, in a shadow root, as those blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #9571. A writer other than `@lexical/y` (a plain Yjs client that does not clean up formats) can leave a link run open over the root sequence, from before a shadow-root quote's paragraph to after the next quote's paragraph. The reader put that link around each quote's paragraph, and since a shadow root holds blocks only, wrapped the link in a paragraph of its own: `quote(paragraph(link(paragraph("q2 two"))))`. A client that then deleted the first quote rewrote the second quote's paragraph (deleted it and inserted it again), so a collaborator's concurrent typing there was lost and its delete undone. A shadow root's content now reads an inline element that holds blocks only as those blocks (`$inShadowRoot`): the quotes show `quote(paragraph("q2 two"))`, and the delete deletes the first quote's text only. The write self-check does not count a stored wrapper over a block in a shadow root, which the reader does not show. The stored link stays; nothing is written to remove it. This extends how a shadow root already shows its content (inline runs in paragraphs of their own), so it is a second case of D70's exception: recorded as DECISIONS.md D88, `OPEN` for review (the number may shift at landing). `@lexical/y`'s own writers leave no such runs. `regressions/QuotesAndSlots.test.ts`, S4-8: an outside link run from the title or the first quote's paragraph to the end of the second quote's, or inside the first quote only (control); then a client deletes the first quote while a collaborator does nothing, empties the second quote or types in it. Each checks that the delete writes only the first quote's text, that every reader shows the quotes' paragraphs as paragraphs, and the text of the second quote on every client and a fresh read. The 6 runs that reach the second quote were pinned as failing; they pass now. Mutations: without the reader rule, all 9 fail (the new "shown as paragraphs" check included); without the self-check rule, the 6 fail on a self-check warning. main (2e2acb76), the S4-8 cells as plain tests: ``` Tests 6 failed | 3 passed (9) × S4-8 run: title to second quote end; peer: nothing: only the first quote's text is deleted expected { del: 'q1 oneq2 two', ins: 'q2 two' } to deeply equal { del: 'q1 one', ins: '' } ``` ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/QuotesAndSlots.test.ts Tests 9 passed (9) ``` Unit tests of lexical-y, lexical-mark, lexical, lexical-selection, lexical-list, lexical-table, lexical-html, lexical-extension and lexical-playground: 24,125 passed, 182 expected fail, 0 failed. `pnpm run ci-check` passes. Soak against main with the same seeds: L1B (mix profile) 300 sessions and L2 (mix and code profiles) 150 each: every session on the same op path, identical flag classes, none only on this branch. --- .changeset/y-shadow-root-link-over-blocks.md | 7 + packages/lexical-y/DECISIONS.md | 2 + .../unit/regressions/QuotesAndSlots.test.ts | 200 +++++++++--------- packages/lexical-y/src/internal/Read.ts | 24 ++- packages/lexical-y/src/internal/SelfCheck.ts | 15 ++ 5 files changed, 145 insertions(+), 103 deletions(-) create mode 100644 .changeset/y-shadow-root-link-over-blocks.md diff --git a/.changeset/y-shadow-root-link-over-blocks.md b/.changeset/y-shadow-root-link-over-blocks.md new file mode 100644 index 00000000000..4d46fbfab55 --- /dev/null +++ b/.changeset/y-shadow-root-link-over-blocks.md @@ -0,0 +1,7 @@ +--- +'@lexical/y': patch +--- + +[fix] A link run another writer left open over a shadow-root quote's paragraph no longer makes a delete of the quote before it rewrite that paragraph + +Verify: with a plain Yjs client, format the root sequence with a link from inside one shadow-root quote's paragraph to past the next quote's paragraph; delete the first quote while a collaborator types in the second: the collaborator's text stays, and every client shows the quotes' paragraphs as paragraphs. diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index b4623f3433e..ac001ebb2ee 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -137,6 +137,7 @@ issues and docs that carry it. | 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] | +| D88 | **An inline element around blocks only, in a shadow root, reads as those blocks (proposed).** Another writer's link run left open over a shadow-root quote's paragraph read as `link(paragraph)` inside a paragraph of its own, a shape no editor keeps; it now reads as the paragraph, and the self-check does not count the stored link there. | A delete of the quote before it rewrote the next quote's paragraph (deleted and inserted it again), losing a collaborator's concurrent typing there ([#9571]). It extends how a shadow root already shows its content (inline runs in paragraphs of their own, `$inShadowRoot`), so it is a second case of D70's "text left without a line marker shows as a paragraph": the stored link stays and the next local edit there does not remove it. `@lexical/y`'s own writers leave no such runs. For Gerard against D70. | `OPEN` | 10-30 | [#9571] | [root-agents]: ../../AGENTS.md [limits]: ../lexical-website/docs/collaboration/y.md#limits @@ -204,3 +205,4 @@ issues and docs that carry it. [#9635]: https://github.com/meta/lexical/issues/9635 [#9636]: https://github.com/meta/lexical/issues/9636 [#9606]: https://github.com/meta/lexical/issues/9606 +[#9571]: https://github.com/meta/lexical/issues/9571 diff --git a/packages/lexical-y/src/__tests__/unit/regressions/QuotesAndSlots.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/QuotesAndSlots.test.ts index 531eb9885b0..c0212c7318a 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/QuotesAndSlots.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/QuotesAndSlots.test.ts @@ -29,9 +29,13 @@ import {expectConverged, freshReader, setup} from '../../utils/harness'; // paragraph and closes after the next quote's paragraph. A client then // deletes the first quote: its write must delete only that quote's text. It // deleted and inserted the second quote's paragraph again, so a peer's -// concurrent typing there was lost and its delete undone. Open for the -// runs that reach the second quote (#9571; `@lexical/y`'s own writers do -// not leave such runs); the run inside the first quote is a control. +// concurrent typing there was lost and its delete undone (#9571): the link +// run was read around the quotes' paragraphs (link(paragraph) in a +// paragraph of its own), a shape the editor could not keep. A shadow root +// holds blocks only, so an inline element around blocks only is read as +// those blocks (see $inShadowRoot), and the quotes show their paragraphs +// without the link. (`@lexical/y`'s own writers do not leave such runs; the +// run inside the first quote, and the title's link, are controls.) describe('S4-8: an outside link run over two quotes, then a client deletes the first quote', () => { const runs = [ 'title to second quote end', @@ -42,105 +46,109 @@ describe('S4-8: an outside link run over two quotes, then a client deletes the f for (const run of runs) { for (const peerEdit of peerEdits) { const open = run !== 'first quote paragraph only'; - (open ? test.fails : test)( - `S4-8 run: ${run}; peer: ${peerEdit}: only the first quote's text is deleted`, - async () => { - const t = await setup(() => { - $getRoot().append( - $createParagraphNode().append($createTextNode('Title')), - $createQuoteNode({shadowRoot: true}).append( - $createParagraphNode().append($createTextNode('q1 one')), - ), - $createQuoteNode({shadowRoot: true}).append( - $createParagraphNode().append($createTextNode('q2 two')), - ), - $createParagraphNode().append($createTextNode('end')), - ); - }); - const outside = new Y.Doc(); - outside.cleanupFormatting = false; - Y.applyUpdate(outside, Y.encodeStateAsUpdate(t.a.doc)); - const sv = Y.encodeStateVector(outside); - const root = outside.get('lexical-y'); - // The indices of the quotes' paragraph items in the root sequence. - const paragraphs: number[] = []; - let index = 0; - for (let item = root._start; item !== null; item = item.right) { - if (item.deleted || !item.countable) { - continue; - } - if ( - item.content instanceof Y.ContentType && - String(item.content.type.name).startsWith('i:') - ) { - paragraphs.push(index); - } - index += item.length; - } - const from = run === 'title to second quote end' ? 2 : paragraphs[0]; - const to = (open ? paragraphs[1] : paragraphs[0]) + 1; - outside.transact(() => - root.format(from, to - from, { - 'w:link': {url: 'https://o.example'}, - }), + test(`S4-8 run: ${run}; peer: ${peerEdit}: only the first quote's text is deleted`, async () => { + const t = await setup(() => { + $getRoot().append( + $createParagraphNode().append($createTextNode('Title')), + $createQuoteNode({shadowRoot: true}).append( + $createParagraphNode().append($createTextNode('q1 one')), + ), + $createQuoteNode({shadowRoot: true}).append( + $createParagraphNode().append($createTextNode('q2 two')), + ), + $createParagraphNode().append($createTextNode('end')), ); - const update = Y.encodeStateAsUpdate(outside, sv); - Y.applyUpdate(t.a.doc, update, 'net'); - Y.applyUpdate(t.b.doc, update, 'net'); - await t.sync(); + }); + const outside = new Y.Doc(); + outside.cleanupFormatting = false; + Y.applyUpdate(outside, Y.encodeStateAsUpdate(t.a.doc)); + const sv = Y.encodeStateVector(outside); + const root = outside.get('lexical-y'); + // The indices of the quotes' paragraph items in the root sequence. + const paragraphs: number[] = []; + let index = 0; + for (let item = root._start; item !== null; item = item.right) { + if (item.deleted || !item.countable) { + continue; + } + if ( + item.content instanceof Y.ContentType && + String(item.content.type.name).startsWith('i:') + ) { + paragraphs.push(index); + } + index += item.length; + } + const from = run === 'title to second quote end' ? 2 : paragraphs[0]; + const to = (open ? paragraphs[1] : paragraphs[0]) + 1; + outside.transact(() => + root.format(from, to - from, { + 'w:link': {url: 'https://o.example'}, + }), + ); + const update = Y.encodeStateAsUpdate(outside, sv); + Y.applyUpdate(t.a.doc, update, 'net'); + Y.applyUpdate(t.b.doc, update, 'net'); + await t.sync(); + // Every reader shows the quotes' paragraphs as paragraphs. + for (const p of [t.a, t.b]) { + expect(p.describe().split(' / ').slice(1, 3)).toEqual([ + 'quote(paragraph("q1 one"))', + 'quote(paragraph("q2 two"))', + ]); + } - const wrote = {del: '', ins: ''}; - t.b.doc.on('afterTransaction', (tr: Y.Transaction) => { - if (!tr.local) { - return; + const wrote = {del: '', ins: ''}; + t.b.doc.on('afterTransaction', (tr: Y.Transaction) => { + if (!tr.local) { + return; + } + Y.iterateStructsByIdSet(tr, tr.insertSet, s => { + if (s instanceof Y.Item && s.content instanceof Y.ContentString) { + wrote.ins += s.content.str; + } + }); + Y.iterateStructsByIdSet(tr, tr.deleteSet, s => { + if (s instanceof Y.Item && s.content instanceof Y.ContentString) { + wrote.del += s.content.str; } - Y.iterateStructsByIdSet(tr, tr.insertSet, s => { - if (s instanceof Y.Item && s.content instanceof Y.ContentString) { - wrote.ins += s.content.str; - } - }); - Y.iterateStructsByIdSet(tr, tr.deleteSet, s => { - if (s instanceof Y.Item && s.content instanceof Y.ContentString) { - wrote.del += s.content.str; - } - }); }); - t.offline(); - t.b.update(() => { - $getRoot().getChildAtIndex(1)!.remove(); + }); + t.offline(); + t.b.update(() => { + $getRoot().getChildAtIndex(1)!.remove(); + $setSelection(null); + }); + const $q2 = () => + ( + $getRoot().getChildAtIndex(2) as ElementNode + ).getFirstChild() as ElementNode; + if (peerEdit === 'empties q2') { + t.a.update(() => { + $q2().clear(); $setSelection(null); }); - const $q2 = () => - ( - $getRoot().getChildAtIndex(2) as ElementNode - ).getFirstChild() as ElementNode; - if (peerEdit === 'empties q2') { - t.a.update(() => { - $q2().clear(); - $setSelection(null); - }); - } else if (peerEdit === 'types in q2') { - t.a.update(() => { - $q2().selectEnd().insertText(' X'); - }); - } - await t.sync(); - expect(wrote).toEqual({del: 'q1 one', ins: ''}); - const expected = - peerEdit === 'empties q2' - ? '' - : peerEdit === 'types in q2' - ? 'q2 two X' - : 'q2 two'; - const fresh = await freshReader(t.a); - for (const p of [t.a, t.b, fresh]) { - expect( - p.read(() => $getRoot().getChildAtIndex(1)!.getTextContent()), - ).toBe(expected); - } - await expectConverged(t.peers); - }, - ); + } else if (peerEdit === 'types in q2') { + t.a.update(() => { + $q2().selectEnd().insertText(' X'); + }); + } + await t.sync(); + expect(wrote).toEqual({del: 'q1 one', ins: ''}); + const expected = + peerEdit === 'empties q2' + ? '' + : peerEdit === 'types in q2' + ? 'q2 two X' + : 'q2 two'; + const fresh = await freshReader(t.a); + for (const p of [t.a, t.b, fresh]) { + expect( + p.read(() => $getRoot().getChildAtIndex(1)!.getTextContent()), + ).toBe(expected); + } + await expectConverged(t.peers); + }); } } }); diff --git a/packages/lexical-y/src/internal/Read.ts b/packages/lexical-y/src/internal/Read.ts index b94916c3856..8a9d05a7733 100644 --- a/packages/lexical-y/src/internal/Read.ts +++ b/packages/lexical-y/src/internal/Read.ts @@ -3546,15 +3546,25 @@ function $inShadowRoot( run = []; } }; + const isBlock = (c: LexicalNode) => + ($isElementNode(c) || $isDecoratorNode(c)) && !c.isInline(); children.forEach((c, i) => { - if (($isElementNode(c) || $isDecoratorNode(c)) && !c.isInline()) { - flush(); - out.push(c); - } else { - if (run.length === 0) { - first = items[i]; + // An inline element around blocks only (a link run another writer left + // open over the shadow root's blocks): its blocks, without it. + const held = + $isElementNode(c) && c.isInline() && c.getChildrenSize() > 0 + ? c.getChildren() + : null; + for (const node of held !== null && held.every(isBlock) ? held : [c]) { + if (isBlock(node)) { + flush(); + out.push(node); + } else { + if (run.length === 0) { + first = items[i]; + } + run.push(node); } - run.push(c); } }); flush(); diff --git a/packages/lexical-y/src/internal/SelfCheck.ts b/packages/lexical-y/src/internal/SelfCheck.ts index e54bcfdef63..f487a249964 100644 --- a/packages/lexical-y/src/internal/SelfCheck.ts +++ b/packages/lexical-y/src/internal/SelfCheck.ts @@ -15,6 +15,7 @@ import { $getRoot, $getSlot, $getSlotNames, + $isDecoratorNode, $isElementNode, $parseSerializedNode, createEditor, @@ -218,6 +219,20 @@ export function $checkWrite(writer: Writer): string[] { if (k === SEGMENT_KEY && a[k] === undefined && !wrappersOnly) { continue; } + // A wrapper (a link run another writer left open) over a block in + // a shadow root: the reader shows the block without it (see + // $inShadowRoot), so the editor holds none. + const special = want.specials.get(i); + if ( + k.startsWith(WRAPPER_PREFIX) && + special !== undefined && + special.kind === 'inline' && + ($isElementNode(special.node) || $isDecoratorNode(special.node)) && + !special.node.isInline() && + a[k] === undefined + ) { + continue; + } // A wrapper's textFormat/textStyle at a character: Lexical's // reconciler derives them from the wrapper's text (see // stripGroupText). From 0480c5f91e7c43edfe05d7425e73d27a7e4ce2a2 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 20:18:51 -0700 Subject: [PATCH 2/7] [lexical-y] Chore: Renumber the shadow-root decision to D89 ## Description D88 is taken by another open draft. Number may shift at landing. ## Test plan Docs only. --- packages/lexical-y/DECISIONS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index ac001ebb2ee..1c67e570715 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -137,7 +137,7 @@ issues and docs that carry it. | 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] | -| D88 | **An inline element around blocks only, in a shadow root, reads as those blocks (proposed).** Another writer's link run left open over a shadow-root quote's paragraph read as `link(paragraph)` inside a paragraph of its own, a shape no editor keeps; it now reads as the paragraph, and the self-check does not count the stored link there. | A delete of the quote before it rewrote the next quote's paragraph (deleted and inserted it again), losing a collaborator's concurrent typing there ([#9571]). It extends how a shadow root already shows its content (inline runs in paragraphs of their own, `$inShadowRoot`), so it is a second case of D70's "text left without a line marker shows as a paragraph": the stored link stays and the next local edit there does not remove it. `@lexical/y`'s own writers leave no such runs. For Gerard against D70. | `OPEN` | 10-30 | [#9571] | +| D89 | **An inline element around blocks only, in a shadow root, reads as those blocks (proposed).** Another writer's link run left open over a shadow-root quote's paragraph read as `link(paragraph)` inside a paragraph of its own, a shape no editor keeps; it now reads as the paragraph, and the self-check does not count the stored link there. | A delete of the quote before it rewrote the next quote's paragraph (deleted and inserted it again), losing a collaborator's concurrent typing there ([#9571]). It extends how a shadow root already shows its content (inline runs in paragraphs of their own, `$inShadowRoot`), so it is a second case of D70's "text left without a line marker shows as a paragraph": the stored link stays and the next local edit there does not remove it. `@lexical/y`'s own writers leave no such runs. For Gerard against D70. | `OPEN` | 10-30 | [#9571] | [root-agents]: ../../AGENTS.md [limits]: ../lexical-website/docs/collaboration/y.md#limits From edb03e66ad884c4a16149f554aab3c8cc17fc250 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 20:39:15 -0700 Subject: [PATCH 3/7] [lexical-y] Bug Fix: Take blocks out of format wrappers at any depth, and only out of format wrappers ## Description Review of the first commit found three gaps: - The reader rule handled one wrapper around blocks only: two nested link runs, or a link that also holds text before or after the quote's paragraph, still lost a collaborator's typing. A format wrapper that holds a block, at any depth of such wrappers, is now taken apart: the blocks show on their own, in order, and inline content the wrapper also holds stays in it (itself around the first run, a copy around each later one), in a paragraph of its own. - The rule also unwrapped an inline element stored as a node of its own (with a slot): the node and its slot were dropped on read. It now applies only to wrappers the read makes from format items. - The self-check's exemption applied outside shadow roots too, and its per-run de-duplication skipped an inline node after a block: a writer that failed to remove a visible link there passed. The exemption now applies to a block whose parent is a shadow root only, and such a block is checked apart from what follows it. With text inside the link, the write self-check still reports the quote's line, as it does not model the paragraph a shadow root shows around inline content; what clients show and store is right. Pinned as that report, #9656. ## Test plan - `Review9654ReadSafety.test.ts` (none, one link, nested links, link with text before or after the paragraph; gc on/off; id orders; causal and sender delivery; collaborator does nothing, types, empties): 120 pass; on the previous head 72 failed. - `Review9654OwnInline.test.ts`: an inline node with a paragraph and a slot keeps both through the read and an unrelated delete, 4/4 (the previous head dropped it in all 4). - `Review9654SelfCheck.test.ts`: a missing visible link is reported for inline-only, block-only, block-then-inline and inline-then-block content outside a shadow root, 4/4 (previous head: 2 missed). Units of lexical-y, lexical-mark, lexical, lexical-selection, lexical-list, lexical-table, lexical-html, lexical-extension and lexical-playground: 24,253 passed, 182 expected fail, 0 failed. `pnpm run ci-check` passes. --- packages/lexical-y/DECISIONS.md | 2 +- .../unit/regressions/Review9654Fixtures.ts | 154 ++++++++++++++++++ .../regressions/Review9654OwnInline.test.ts | 89 ++++++++++ .../regressions/Review9654ReadSafety.test.ts | 90 ++++++++++ .../regressions/Review9654SelfCheck.test.ts | 90 ++++++++++ packages/lexical-y/src/internal/Read.ts | 88 ++++++++-- packages/lexical-y/src/internal/SelfCheck.ts | 38 +++-- 7 files changed, 526 insertions(+), 25 deletions(-) create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Review9654Fixtures.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Review9654OwnInline.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Review9654ReadSafety.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Review9654SelfCheck.test.ts diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index 1c67e570715..8c30baf0282 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -137,7 +137,7 @@ issues and docs that carry it. | 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] | -| D89 | **An inline element around blocks only, in a shadow root, reads as those blocks (proposed).** Another writer's link run left open over a shadow-root quote's paragraph read as `link(paragraph)` inside a paragraph of its own, a shape no editor keeps; it now reads as the paragraph, and the self-check does not count the stored link there. | A delete of the quote before it rewrote the next quote's paragraph (deleted and inserted it again), losing a collaborator's concurrent typing there ([#9571]). It extends how a shadow root already shows its content (inline runs in paragraphs of their own, `$inShadowRoot`), so it is a second case of D70's "text left without a line marker shows as a paragraph": the stored link stays and the next local edit there does not remove it. `@lexical/y`'s own writers leave no such runs. For Gerard against D70. | `OPEN` | 10-30 | [#9571] | +| D89 | **Inline elements around blocks, in a shadow root, read as the blocks out of them (proposed).** Another writer's link run left open over a shadow-root quote's paragraph read as `link(paragraph)` inside a paragraph of its own, a shape no editor keeps; now the paragraph shows on its own, at any depth of such elements, and inline content they also hold stays in them, in a paragraph of its own. The write self-check does not count a stored wrapper over a block in a shadow root (only there). | A delete of the quote before it rewrote the next quote's paragraph (deleted and inserted it again), losing a collaborator's concurrent typing there ([#9571]). It extends how a shadow root already shows its content (inline runs in paragraphs of their own, `$inShadowRoot`), so it is a second case of D70's "text left without a line marker shows as a paragraph": the stored link stays and the next local edit there does not remove it. `@lexical/y`'s own writers leave no such runs. For Gerard against D70. | `OPEN` | 10-30 | [#9571] | [root-agents]: ../../AGENTS.md [limits]: ../lexical-website/docs/collaboration/y.md#limits diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654Fixtures.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654Fixtures.ts new file mode 100644 index 00000000000..1d6555b00f9 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654Fixtures.ts @@ -0,0 +1,154 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ +import {LinkNode} from '@lexical/link'; +import {$createQuoteNode} from '@lexical/rich-text'; +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createTextNode, + $getRoot, + $getSlot, + $getSlotNames, + $isElementNode, + type ElementNode, + type LexicalNode, +} from 'lexical'; +import {expect, onTestFinished} from 'vitest'; + +import {readYState} from '../../../headless'; +import {NODES, type Peer, setup} from '../../utils/harness'; +/** A second link type, for two link wrappers nested around blocks. */ +export class AuditLink extends LinkNode { + $config() { + return this.config('auditlink', {extends: LinkNode}); + } +} +export const $p = (text: string) => + $createParagraphNode().append($createTextNode(text)); +/** @deprecated renamed to {@link $p} by @lexical/eslint-plugin rules-of-lexical */ +export const p = $p; +export function $deep(node: LexicalNode): unknown { + const result: Record = {...node.exportJSON()}; + if ($isElementNode(node)) result.children = node.getChildren().map($deep); + const names = $getSlotNames(node); + if (names.length) { + const slots: Record = {}; + for (const name of names) slots[name] = $deep($getSlot(node, name)!); + result.$slots = slots; + } + return result; +} +/** @deprecated renamed to {@link $deep} by @lexical/eslint-plugin rules-of-lexical */ +export const deep = $deep; +export function equalFresh(q: Peer) { + expect(q.doc.store.pendingStructs).toBeNull(); + expect(q.doc.store.pendingDs).toBeNull(); + const want = readYState( + Y.encodeStateAsUpdate(q.doc), + { + nodes: q.nodes, + schemaVersion: 1, + }, + () => $deep($getRoot()), + ); + expect(q.read(() => $deep($getRoot()))).toEqual(want); +} +export function noLinkOn(q: Peer, text = 'q2 two') { + q.read(() => { + for (const n of $getRoot().getAllTextNodes()) + if (n.getTextContent().includes(text)) { + for ( + let ancestor = n.getParent(); + ancestor; + ancestor = ancestor.getParent() + ) + expect( + ancestor instanceof LinkNode, + `${text} in ${q.describe()}`, + ).toBe(false); + } + }); +} +export async function fixture( + opts: { + ids?: number[]; + delivery?: 'causal' | 'sender'; + gc?: boolean; + withBlockIds?: boolean; + container?: () => ElementNode; + nodes?: typeof NODES; + variant?: string; + } = {}, +) { + const t = await setup( + () => + $getRoot().append( + $p('Title'), + $createQuoteNode({shadowRoot: true}).append($p('q1 one')), + $createQuoteNode({shadowRoot: true}).append($p('q2 two')), + $p('end'), + ), + { + delivery: opts.delivery, + ids: opts.ids, + nodes: [...NODES, AuditLink, ...(opts.nodes ?? [])], + withBlockIds: opts.withBlockIds, + }, + ); + onTestFinished(() => + t.peers.forEach(q => { + q.stop(); + q.doc.destroy(); + }), + ); + for (const q of t.peers) q.doc.gc = opts.gc ?? false; + const outside = new Y.Doc({gc: opts.gc ?? false}); + outside.cleanupFormatting = false; + onTestFinished(() => outside.destroy()); + Y.applyUpdate(outside, Y.encodeStateAsUpdate(t.a.doc)); + const paras = () => { + const out: {at: number; node: Y.Node}[] = []; + let at = 0; + const root = outside.get('lexical-y'); + for (let it = root._start; it; it = it.right) { + if (it.deleted || !it.countable) continue; + if ( + it.content instanceof Y.ContentType && + String(it.content.type.name).startsWith('i:') + ) + out.push({at, node: it.content.type}); + at += it.length; + } + return out; + }; + const apply = async (fn: (root: Y.Node) => void) => { + const sv = Y.encodeStateVector(outside); + outside.transact(() => fn(outside.get('lexical-y'))); + const upd = Y.encodeStateAsUpdate(outside, sv); + for (const q of t.peers) Y.applyUpdate(q.doc, upd, 'net'); + await t.sync(); + }; + const variant = opts.variant ?? 'single'; + await apply(root => { + const [a, b] = paras(); + const attrs: Record = { + 'w:link': {url: 'https://o.example'}, + }; + if (variant === 'nested') attrs['w:auditlink'] = {url: 'https://n.example'}; + if (variant !== 'none') root.format(a.at, b.at + 1 - a.at, attrs); + if (variant.startsWith('mixed')) + root.insert(b.at + (variant === 'mixed-after' ? 1 : 0), ' RAW', attrs); + }); + return {...t, apply, outside, paras}; +} +export const $q2 = () => + $getRoot() + .getChildren() + .find(n => n.getTextContent().includes('q2 two')) as ElementNode; +/** @deprecated renamed to {@link $q2} by @lexical/eslint-plugin rules-of-lexical */ +export const q2 = $q2; diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654OwnInline.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654OwnInline.test.ts new file mode 100644 index 00000000000..36fb3e4f4a3 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654OwnInline.test.ts @@ -0,0 +1,89 @@ +/** + * 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 * as Y from '@y/y'; +import { + $getRoot, + $getSlot, + $getSlotNames, + $setSelection, + ElementNode, + type LexicalNode, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {fixture} from './Review9654Fixtures'; + +// #9571: the reader takes blocks out of format wrappers only (link runs made +// from format items), never out of an inline element stored as a node of its +// own: such a node, holding a paragraph and a slot, stays with both. +class AuditOwnInline extends ElementNode { + $config() { + return this.config('auditowninline', {extends: ElementNode}); + } + isInline(): true { + return true; + } + createDOM(): HTMLElement { + throw new Error('headless'); + } + updateDOM(): false { + return false; + } +} +function $slotTexts(n: LexicalNode): string[] { + const out: string[] = []; + for (const name of $getSlotNames(n)) { + const slot = $getSlot(n, name)!; + out.push(slot.getTextContent()); + out.push(...$slotTexts(slot)); + } + if (n instanceof ElementNode) + for (const c of n.getChildren()) out.push(...$slotTexts(c)); + return out; +} +for (const gc of [false, true]) + for (const ids of [ + [1, 2], + [2, 1], + ]) { + test(`own inline block-child slot survives read and unrelated delete gc=${gc} ids=${ids}`, async () => { + const t = await fixture({ + gc, + ids, + nodes: [AuditOwnInline], + variant: 'none', + }); + await t.apply(root => { + const at = t.paras()[1].at; + root.delete(at, 1); + const holder = new Y.Node('i:auditowninline'); + root.insert(at, [holder]); + holder.setAttr('c', 'i'); + const body = new Y.Node('i:paragraph'); + holder.insert(0, [body]); + body.setAttr('c', 'i'); + body.insert(0, 'q2 two'); + const slot = new Y.Node('b:paragraph'); + holder.setAttr('slot:note', slot); + slot.setAttr('c', 'i'); + slot.insert(0, 'SLOT KEEP'); + }); + for (const q of t.peers) + expect + .soft(q.read(() => $slotTexts($getRoot()))) + .toEqual(['SLOT KEEP']); + t.b.update(() => { + $getRoot().getChildAtIndex(1)!.remove(); + $setSelection(null); + }); + await t.sync(); + for (const q of t.peers) + expect(q.read(() => $slotTexts($getRoot()))).toEqual(['SLOT KEEP']); + }); + } diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654ReadSafety.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654ReadSafety.test.ts new file mode 100644 index 00000000000..dca377ceffb --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654ReadSafety.test.ts @@ -0,0 +1,90 @@ +/** + * 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 * as Y from '@y/y'; +import {$getRoot, $setSelection, type ElementNode} from 'lexical'; +import {expect, test} from 'vitest'; + +import {equalFresh, fixture} from './Review9654Fixtures'; +// #9571: an outside link run over two shadow-root quotes' paragraphs (one +// link, two nested links, or a link that also holds text before or after the +// second quote's paragraph), then a client deletes the first quote while a +// collaborator does nothing, types in the second quote or empties it: the +// delete writes only the first quote's text, every client shows the second +// quote's text (and the outside text, in a paragraph of its own), as a fresh +// read does. With text in the link (mixed), the write self-check reports the +// quote's line, as it does not model the paragraph a shadow root shows around +// inline content (#9656): pinned as that report. +for (const variant of [ + 'none', + 'single', + 'nested', + 'mixed-before', + 'mixed-after', +]) + for (const gc of [false, true]) + for (const ids of [ + [1, 2], + [2, 1], + ]) + for (const delivery of ['causal', 'sender'] as const) + for (const peerEdit of ['nothing', 'types', 'empties']) { + test(`${variant} gc=${gc} ids=${ids} delivery=${delivery} peer=${peerEdit}`, async () => { + const t = await fixture({delivery, gc, ids, variant}); + t.peers.forEach(equalFresh); + const wrote = {del: '', ins: ''}; + t.b.doc.on('afterTransaction', (tr: Y.Transaction) => { + if (!tr.local) return; + Y.iterateStructsByIdSet(tr, tr.insertSet, s => { + if (s instanceof Y.Item && s.content instanceof Y.ContentString) + wrote.ins += s.content.str; + }); + Y.iterateStructsByIdSet(tr, tr.deleteSet, s => { + if (s instanceof Y.Item && s.content instanceof Y.ContentString) + wrote.del += s.content.str; + }); + }); + t.offline(); + t.b.update(() => { + $getRoot().getChildAtIndex(1)!.remove(); + $setSelection(null); + }); + if (peerEdit !== 'nothing') + t.a.update(() => { + const q = $getRoot().getChildAtIndex(2) as ElementNode; + const text = q + .getAllTextNodes() + .find(n => n.getTextContent() === 'q2 two')!; + if (peerEdit === 'types') text.selectEnd().insertText(' X'); + else text.setTextContent(''); + $setSelection(null); + }); + await t.sync(); + expect.soft(wrote).toEqual({del: 'q1 one', ins: ''}); + const raw = variant.startsWith('mixed') ? ' RAW' : ''; + const text = + peerEdit === 'empties' + ? '' + : `q2 two${peerEdit === 'types' ? ' X' : ''}`; + for (const q of t.peers) { + const shown = q.read(() => + ($getRoot().getChildAtIndex(1) as ElementNode).getTextContent(), + ); + expect + .soft(shown.replace(/\n/g, '')) + .toBe(variant === 'mixed-before' ? raw + text : text + raw); + equalFresh(q); + const reports = q.warnings.filter(w => w.includes('self-check')); + if (variant.startsWith('mixed')) + for (const w of reports) + expect.soft(w).toMatch(/: text "( RAW| RAW)" != ""$/); + else expect.soft(reports).toEqual([]); + } + expect(t.a.json()).toEqual(t.b.json()); + }); + } diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654SelfCheck.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654SelfCheck.test.ts new file mode 100644 index 00000000000..03f5338a803 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654SelfCheck.test.ts @@ -0,0 +1,90 @@ +/** + * 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 * as Y from '@y/y'; +import {$create, $createParagraphNode, $getRoot, createEditor} from 'lexical'; +import {expect, test} from 'vitest'; + +import {RUN_END} from '../../../internal/Schema'; +import {$checkWrite} from '../../../internal/SelfCheck'; +import {type Writer} from '../../../internal/Write'; +import {EquationNode, NODES, WidgetNode} from '../../utils/harness'; +// #9571: the self-check skips a stored wrapper over a block only in a shadow +// root (the reader shows such a block without it); elsewhere a wrapper the +// writer failed to remove is still reported, also after a block. +for (const nodes of [ + 'inline-only', + 'block-only', + 'block-then-inline', + 'inline-then-block', +]) { + test(`self-check must reject a missing visible wrapper: ${nodes}`, () => { + const editor = createEditor({ + nodes: NODES, + onError: e => { + throw e; + }, + }); + const doc = new Y.Doc({gc: false}); + doc.cleanupFormatting = false; + const root = doc.get('audit'); + const names = + nodes === 'inline-only' + ? ['test_equation'] + : nodes === 'block-only' + ? ['test_widget'] + : nodes === 'block-then-inline' + ? ['test_widget', 'test_equation'] + : ['test_equation', 'test_widget']; + let end: Y.Item; + doc.transact(() => { + for (const [at, name] of names.entries()) + root.insert(at, [new Y.Node('i:' + name)], { + 'w:link': {url: 'https://should-be-visible.example'}, + }); + root.insert(names.length, [new Y.Node(RUN_END)]); + for (let it = root._start; it; it = it.right) + if ( + it.content instanceof Y.ContentType && + it.content.type.name === RUN_END + ) + end = it; + }); + editor.update( + () => { + const ns = names.map(name => + name === 'test_widget' ? $create(WidgetNode) : $create(EquationNode), + ); + // Deliberate fault injection: a writer fails to persist the author's + // removal of a visible link. No shadow root in this editor/unit. + $getRoot().append($createParagraphNode().append(...ns)); + const writer = { + $ctx: () => ({ + cursor: null, + cursorAt: -1, + segments: {create: () => null, get: () => undefined}, + }), + formatAsStored: (_key: string, value: unknown) => value, + state: {editor, hidden: null, segments: new Map()}, + textFields: () => new Set(), + touched: [ + { + d: {kind: 'run', nodes: ns}, + unit: {cs: {y: root}, end: end!, key: 'audit', start: new Map()}, + }, + ], + wrapperFields: () => new Set(['url']), + } as unknown as Writer; + const problems = $checkWrite(writer); + expect(problems.some(p => p.includes('format w:link'))).toBe(true); + }, + {discrete: true}, + ); + doc.destroy(); + }); +} diff --git a/packages/lexical-y/src/internal/Read.ts b/packages/lexical-y/src/internal/Read.ts index 8a9d05a7733..03eda8c3d61 100644 --- a/packages/lexical-y/src/internal/Read.ts +++ b/packages/lexical-y/src/internal/Read.ts @@ -16,6 +16,7 @@ import { type Node as YNode, } from '@y/y'; import { + $copyNode, $create, $createLineBreakNode, $createParagraphNode, @@ -2658,6 +2659,8 @@ export class Reader { const top: LexicalNode[] = []; // The item each of `top` was read from first (see $inShadowRoot). const topItems: Item[] = []; + // The wrappers this read makes from format items (see $inShadowRoot). + const formatWrappers = new Set(); type Frame = { value: WrapperValue; node: ElementNode; @@ -2702,6 +2705,7 @@ export class Reader { ); for (const {w} of missing) { const node = this.$wrapperNode(w, wrappersPool); + formatWrappers.add(node.getKey()); if (stack.length > 0) { stack[stack.length - 1].children.push(node); } else { @@ -2834,7 +2838,7 @@ export class Reader { state.refresh.add(holder.getKey()); } if (el !== null && el.isShadowRoot()) { - return $inShadowRoot(top, topItems, shown, this.settler); + return $inShadowRoot(top, topItems, shown, this.settler, formatWrappers); } for (const p of shown) { // No longer a paragraph around inline content in a shadow root. @@ -3501,6 +3505,7 @@ function $inShadowRoot( items: Item[], shown: ElementNode[], settler: BlockIdSettler, + formatWrappers: ReadonlySet, ): LexicalNode[] { const out: LexicalNode[] = []; let run: LexicalNode[] = []; @@ -3546,17 +3551,13 @@ function $inShadowRoot( run = []; } }; - const isBlock = (c: LexicalNode) => - ($isElementNode(c) || $isDecoratorNode(c)) && !c.isInline(); children.forEach((c, i) => { - // An inline element around blocks only (a link run another writer left - // open over the shadow root's blocks): its blocks, without it. - const held = - $isElementNode(c) && c.isInline() && c.getChildrenSize() > 0 - ? c.getChildren() - : null; - for (const node of held !== null && held.every(isBlock) ? held : [c]) { - if (isBlock(node)) { + // Inline elements around blocks (a link run another writer left open + // over the shadow root's blocks): the blocks, out of them. + for (const node of $holdsBlock(c, formatWrappers) + ? $splitAroundBlocks(c, formatWrappers) + : [c]) { + if ($isShownBlock(node)) { flush(); out.push(node); } else { @@ -3574,6 +3575,71 @@ function $inShadowRoot( return out; } +function $isShownBlock(node: LexicalNode): boolean { + return ($isElementNode(node) || $isDecoratorNode(node)) && !node.isInline(); +} + +/** + * Whether `node` is a wrapper this read made from format items (a link run, + * see Reader#$wrapperNode; never an inline element stored as a node of its + * own) that holds a block, at any depth of such wrappers. + */ +function $holdsBlock( + node: LexicalNode, + formatWrappers: ReadonlySet, +): node is ElementNode { + return ( + $isElementNode(node) && + formatWrappers.has(node.getKey()) && + node + .getChildren() + .some(k => $isShownBlock(k) || $holdsBlock(k, formatWrappers)) + ); +} + +/** + * A format wrapper that holds blocks (see $holdsBlock), taken apart: the + * blocks, in order, and between them the element around the inline content + * it holds there (itself around the first such run, a copy around each later + * one), at any depth. Why: a shadow root holds blocks only, and a block + * inside an inline element is a shape the editor cannot keep (it was read + * as `link(paragraph)` in a paragraph of its own, and a delete next to it + * rewrote the paragraph: #9571). + */ +function $splitAroundBlocks( + el: ElementNode, + formatWrappers: ReadonlySet, +): LexicalNode[] { + const out: LexicalNode[] = []; + const runs: [ElementNode, LexicalNode[]][] = []; + let run: LexicalNode[] | null = null; + for (const k of el.getChildren()) { + for (const part of $holdsBlock(k, formatWrappers) + ? $splitAroundBlocks(k, formatWrappers) + : [k]) { + if ($isShownBlock(part)) { + run = null; + out.push(part); + } else { + if (run === null) { + const wrapper = runs.length === 0 ? el : $copyNode(el); + run = []; + runs.push([wrapper, run]); + out.push(wrapper); + } + run.push(part); + } + } + } + if (runs.length === 0 || runs[0][0] !== el) { + el.clear(); + } + for (const [wrapper, kids] of runs) { + wrapper.splice(0, wrapper.getChildrenSize(), kids); + } + return out; +} + /** * The group nodes (and level wrappers) under a group read before, for the * read to reuse. An item whose only child is a group stored as its own node diff --git a/packages/lexical-y/src/internal/SelfCheck.ts b/packages/lexical-y/src/internal/SelfCheck.ts index f487a249964..abc2b1e460e 100644 --- a/packages/lexical-y/src/internal/SelfCheck.ts +++ b/packages/lexical-y/src/internal/SelfCheck.ts @@ -108,6 +108,15 @@ function $shownText(writer: Writer, atoms: Atoms): string { return out; } +/** A block (not inline) whose parent is a shadow root. */ +function $isShadowRootBlock(node: LexicalNode): boolean { + if (!($isElementNode(node) || $isDecoratorNode(node)) || node.isInline()) { + return false; + } + const parent = node.getParent(); + return parent !== null && parent.isShadowRoot(); +} + function hasLiveContent(y: YNode): boolean { for (let it = y._start; it !== null; it = it.right) { if (!it.deleted && it.countable) { @@ -204,8 +213,22 @@ export function $checkWrite(writer: Writer): string[] { const b = rb < have.runs.length ? have.runs[rb].attrs : {}; const isBreak = !want.specials.has(i) && want.text.charCodeAt(i) === 10; const wrappersOnly = want.specials.has(i) || isBreak; + const special = want.specials.get(i); + // A wrapper over a block in a shadow root (another writer's run left + // open over its blocks): the reader shows the block without it (see + // $inShadowRoot), so the editor holds none. + const shownBare = + special !== undefined && + special.kind === 'inline' && + $isShadowRootBlock(special.node); const key = - ra + '/' + rb + '/' + (wrappersOnly ? 1 : 0) + (isBreak ? i : ''); + ra + + '/' + + rb + + '/' + + (wrappersOnly ? 1 : 0) + + (shownBare ? 'b' : '') + + (isBreak ? i : ''); if (key === lastKey) { continue; } @@ -219,18 +242,7 @@ export function $checkWrite(writer: Writer): string[] { if (k === SEGMENT_KEY && a[k] === undefined && !wrappersOnly) { continue; } - // A wrapper (a link run another writer left open) over a block in - // a shadow root: the reader shows the block without it (see - // $inShadowRoot), so the editor holds none. - const special = want.specials.get(i); - if ( - k.startsWith(WRAPPER_PREFIX) && - special !== undefined && - special.kind === 'inline' && - ($isElementNode(special.node) || $isDecoratorNode(special.node)) && - !special.node.isInline() && - a[k] === undefined - ) { + if (shownBare && k.startsWith(WRAPPER_PREFIX) && a[k] === undefined) { continue; } // A wrapper's textFormat/textStyle at a character: Lexical's From b67d43c0a71f6b1e76517956f396e07fcdf1e5a8 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 20:47:14 -0700 Subject: [PATCH 4/7] [lexical-y] Bug Fix: Know once per read whether a format wrapper holds a block ## Description Taking blocks out of nested format wrappers asked, at each level, whether each wrapper below held a block, so the cost grew with the square of the nesting depth (128 nested link types: 8,393 child visits). Whether a wrapper holds a block is now kept per read, so each wrapper's children are looked at once. Also adds the review's coverage for #9571: local edits and undo after the paragraph shows out of the link (and the outside writer adding linked text next to it, which no longer brings the link back), every catalog shape as a quote's block, the unchanged reading outside shadow roots, and the cost of one wide wrapper. ## Test plan - `Review9654DepthCost.test.ts`: child visits for 8 to 128 nested wrappers grow linearly (fails on the previous head: 8,393 at 128). - `Review9654Lifetime.test.ts` 136, `Review9654Catalog.test.ts` 244, `Review9654Outside.test.ts` 4, `Review9654Cost.test.ts` 1: pass. Units of lexical-y, lexical-mark, lexical, lexical-selection, lexical-list, lexical-table, lexical-html, lexical-extension and lexical-playground: 24,639 passed, 182 expected fail, 0 failed. `pnpm run ci-check` passes. --- .../regressions/Review9654Catalog.test.ts | 100 ++++++++++ .../unit/regressions/Review9654Cost.test.ts | 64 +++++++ .../regressions/Review9654DepthCost.test.ts | 64 +++++++ .../unit/regressions/Review9654Fixtures.ts | 20 +- .../regressions/Review9654Lifetime.test.ts | 178 ++++++++++++++++++ .../regressions/Review9654Outside.test.ts | 45 +++++ packages/lexical-y/src/internal/Read.ts | 28 ++- 7 files changed, 484 insertions(+), 15 deletions(-) create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Review9654Catalog.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Review9654Cost.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Review9654DepthCost.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Review9654Lifetime.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Review9654Outside.test.ts diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654Catalog.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654Catalog.test.ts new file mode 100644 index 00000000000..41c97d559e5 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654Catalog.test.ts @@ -0,0 +1,100 @@ +/** + * 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 +// #9571: every catalog shape as a quote's block under an outside link run +// keeps its payload when another quote is deleted. +import {$createQuoteNode} from '@lexical/rich-text'; +import * as Y from '@y/y'; +import {$getRoot} from 'lexical'; +import {expect, onTestFinished, test} from 'vitest'; + +import {CATALOG_NODES, NODE_SHAPES} from '../../catalog/nodes'; +import {setup} from '../../utils/harness'; +import {deep, equalFresh, p} from './Review9654Fixtures'; + +const mask = (value: unknown) => + JSON.parse(JSON.stringify(value), (k, v) => + k === 'editorState' ? 'nested-editor' : v, + ); +for (const shape of NODE_SHAPES) + for (const gc of [false, true]) + for (const ids of [ + [1, 2], + [2, 1], + ]) { + test(`catalog ${shape.name} gc=${gc} ids=${ids}`, async () => { + const t = await setup( + () => + $getRoot().append( + p('Title'), + $createQuoteNode({shadowRoot: true}).append(p('q1 one')), + $createQuoteNode({shadowRoot: true}).append(shape.$block()), + p('end'), + ), + { + ids, + nodes: [...CATALOG_NODES], + withBlockIds: shape.tags.includes('nested-editor'), + }, + ); + onTestFinished(() => + t.peers.forEach(q => { + q.stop(); + q.doc.destroy(); + }), + ); + t.peers.forEach(q => (q.doc.gc = gc)); + const before = mask( + t.a.read(() => deep($getRoot().getChildAtIndex(2)!)), + ); + const d = new Y.Doc({gc}); + d.cleanupFormatting = false; + onTestFinished(() => d.destroy()); + Y.applyUpdate(d, Y.encodeStateAsUpdate(t.a.doc)); + const sv = Y.encodeStateVector(d); + const root = d.get('lexical-y'); + const atoms: number[] = []; + let at = 0; + for (let it = root._start; it; it = it.right) { + if (it.deleted || !it.countable) continue; + if ( + it.content instanceof Y.ContentType && + String(it.content.type.name).startsWith('i:') + ) + atoms.push(at); + at += it.length; + } + expect(atoms.length).toBeGreaterThanOrEqual(2); + d.transact(() => + root.format(atoms[0], atoms[atoms.length - 1] + 1 - atoms[0], { + 'w:link': {url: 'https://outside.example'}, + }), + ); + const update = Y.encodeStateAsUpdate(d, sv); + for (const q of t.peers) Y.applyUpdate(q.doc, update, 'net'); + await t.sync(); + for (const q of t.peers) + expect( + mask(q.read(() => deep($getRoot().getChildAtIndex(2)!))), + ).toEqual(before); + // Removing only the other quote cannot change any payload in this one. + t.b.update(() => $getRoot().getChildAtIndex(1)!.remove()); + await t.sync(); + for (const q of t.peers) { + expect( + mask(q.read(() => deep($getRoot().getChildAtIndex(1)!))), + ).toEqual(before); + if (!shape.tags.includes('nested-editor')) equalFresh(q); + expect( + q.warnings.filter( + w => w.includes('self-check') && !w.includes('.editorState.'), + ), + ).toEqual([]); + } + }); + } diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654Cost.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654Cost.test.ts new file mode 100644 index 00000000000..6ce7f2eb119 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654Cost.test.ts @@ -0,0 +1,64 @@ +/** + * 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 +// #9571: taking blocks out of a format wrapper looks at the wrapper's +// children a bounded number of times (linear in the blocks). +import {$createQuoteNode} from '@lexical/rich-text'; +import * as Y from '@y/y'; +import {$getRoot, ElementNode} from 'lexical'; +import {expect, test, vi} from 'vitest'; + +import {readYState, seedYState} from '../../../headless'; +import {NODES} from '../../utils/harness'; +import {deep, p} from './Review9654Fixtures'; + +test('total child enumeration is linear for one wrapper over 64/128/256 blocks', () => { + const counts: number[] = []; + const cfg = { + namespace: 'cost', + nodes: NODES, + onError: (e: Error) => { + throw e; + }, + schemaVersion: 1, + }; + for (const n of [64, 128, 256]) { + const state = seedYState(cfg, () => { + $getRoot().append( + $createQuoteNode({shadowRoot: true}).append( + ...Array.from({length: n}, (_, i) => p('p' + i)), + ), + ); + }); + const doc = new Y.Doc({gc: false}); + doc.cleanupFormatting = false; + Y.applyUpdate(doc, state); + const root = doc.get('lexical-y'); + root.format(0, n, {'w:link': {url: 'https://outside.example'}}); + let visits = 0; + const orig = ElementNode.prototype.getChildren; + const spy = vi + .spyOn(ElementNode.prototype, 'getChildren') + .mockImplementation(function (this: ElementNode) { + const result = orig.call(this); + visits += result.length; + return result; + }); + try { + readYState(Y.encodeStateAsUpdate(doc), cfg, () => deep($getRoot())); + } finally { + spy.mockRestore(); + doc.destroy(); + } + counts.push(visits); + } + if (process.env.REVIEW9654_VERBOSE === '1') + console.warn('CHILD_VISITS', counts); + expect(counts[1]).toBeLessThan(counts[0] * 2.5 + 256); + expect(counts[2]).toBeLessThan(counts[1] * 2.5 + 256); +}); diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654DepthCost.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654DepthCost.test.ts new file mode 100644 index 00000000000..b52f765af8d --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654DepthCost.test.ts @@ -0,0 +1,64 @@ +/** + * 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 +// #9571: taking blocks out of nested format wrappers looks at each wrapper's +// children once per read (linear in the nesting depth). +import {LinkNode} from '@lexical/link'; +import {$createQuoteNode} from '@lexical/rich-text'; +import * as Y from '@y/y'; +import {$getRoot, ElementNode} from 'lexical'; +import {expect, test, vi} from 'vitest'; + +import {readYState, seedYState} from '../../../headless'; +import {NODES} from '../../utils/harness'; +import {deep, p} from './Review9654Fixtures'; + +test('enumerating nested format wrappers is linear in nesting depth', () => { + const counts: number[] = []; + for (const depth of [8, 16, 32, 64, 128]) { + const nodes = Array.from({length: depth}, (_, i) => { + const name = `audit_depth_${i}`; + return class DepthLink extends LinkNode { + $config() { + return this.config(name, {extends: LinkNode}); + } + }; + }); + const config = {nodes: [...NODES, ...nodes], schemaVersion: 1}; + const state = seedYState(config, () => { + $getRoot().append($createQuoteNode({shadowRoot: true}).append(p('body'))); + }); + const doc = new Y.Doc({gc: false}); + doc.cleanupFormatting = false; + Y.applyUpdate(doc, state); + const attrs = Object.fromEntries( + nodes.map((_, i) => [ + `w:audit_depth_${i}`, + {url: `https://wrapper${i}.example`}, + ]), + ); + doc.get('lexical-y').format(0, 1, attrs); + let visits = 0; + const original = ElementNode.prototype.getChildren; + const spy = vi + .spyOn(ElementNode.prototype, 'getChildren') + .mockImplementation(function (this: ElementNode) { + const result = original.call(this); + visits += result.length; + return result; + }); + try { + readYState(Y.encodeStateAsUpdate(doc), config, () => deep($getRoot())); + } finally { + spy.mockRestore(); + doc.destroy(); + } + counts.push(visits); + } + expect(counts[4]).toBeLessThan(counts[3] * 2.5 + 128); +}); diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654Fixtures.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654Fixtures.ts index 1d6555b00f9..07f501cd29c 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/Review9654Fixtures.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654Fixtures.ts @@ -50,18 +50,17 @@ export function equalFresh(q: Peer) { expect(q.doc.store.pendingDs).toBeNull(); const want = readYState( Y.encodeStateAsUpdate(q.doc), - { - nodes: q.nodes, - schemaVersion: 1, - }, + {nodes: q.nodes, schemaVersion: 1}, () => $deep($getRoot()), ); expect(q.read(() => $deep($getRoot()))).toEqual(want); } export function noLinkOn(q: Peer, text = 'q2 two') { + let found = 0; q.read(() => { for (const n of $getRoot().getAllTextNodes()) if (n.getTextContent().includes(text)) { + found++; for ( let ancestor = n.getParent(); ancestor; @@ -73,6 +72,17 @@ export function noLinkOn(q: Peer, text = 'q2 two') { ).toBe(false); } }); + expect(found, `missing sentinel ${text}`).toBeGreaterThan(0); +} +export function noLinks(q: Peer) { + q.read(() => { + const walk = (n: LexicalNode) => { + expect(n instanceof LinkNode, q.describe()).toBe(false); + if ($isElementNode(n)) n.getChildren().forEach(walk); + for (const name of $getSlotNames(n)) walk($getSlot(n, name)!); + }; + walk($getRoot()); + }); } export async function fixture( opts: { @@ -81,7 +91,7 @@ export async function fixture( gc?: boolean; withBlockIds?: boolean; container?: () => ElementNode; - nodes?: typeof NODES; + nodes?: Readonly; variant?: string; } = {}, ) { diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654Lifetime.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654Lifetime.test.ts new file mode 100644 index 00000000000..64ca289f765 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654Lifetime.test.ts @@ -0,0 +1,178 @@ +/** + * 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 +// #9571: after the reader shows a quote's paragraph out of an outside link +// run, local edits (typing, Enter, moves into containers, collapse, copy and +// paste) and undo/redo keep every client equal to a fresh read with no link +// shown; the outside writer adding linked text next to the paragraph does not +// bring the link back around it. +import type {QuoteNode} from '@lexical/rich-text'; + +import { + $generateJSONFromSelectedNodes, + $generateNodesFromSerializedNodes, +} from '@lexical/clipboard'; +import { + $createTableCellNode, + $createTableNode, + $createTableRowNode, +} from '@lexical/table'; +import { + $createNodeSelection, + $getEditor, + $getRoot, + $setSelection, + type ElementNode, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {$createLayoutContainerNode} from '../../../../../lexical-playground/src/nodes/LayoutContainerNode'; +import {$createLayoutItemNode} from '../../../../../lexical-playground/src/nodes/LayoutItemNode'; +import {$createCollapsibleContainerNode} from '../../../../../lexical-playground/src/plugins/CollapsibleExtension/CollapsibleContainerNode'; +import {$createCollapsibleContentNode} from '../../../../../lexical-playground/src/plugins/CollapsibleExtension/CollapsibleContentNode'; +import {$createCollapsibleTitleNode} from '../../../../../lexical-playground/src/plugins/CollapsibleExtension/CollapsibleTitleNode'; +import {$createDownstreamComponent} from '../../catalog/downstream'; +import {CATALOG_NODES} from '../../catalog/nodes'; +import {$createBoxNode} from '../../utils/harness'; +import { + equalFresh, + fixture, + noLinkOn, + noLinks, + p, + q2, +} from './Review9654Fixtures'; + +const ops: Record void> = { + after: () => q2().getAllTextNodes()[0].selectEnd().insertText('X'), + 'append-paragraph': () => q2().append(p('X')), + before: () => q2().getAllTextNodes()[0].select(0, 0).insertText('X'), + collapse: () => { + (q2() as QuoteNode).collapseAtStart(); + }, + 'copy-paste': () => { + const q = q2(); + const s = $createNodeSelection(); + s.add(q.getKey()); + $setSelection(s); + const data = $generateJSONFromSelectedNodes($getEditor(), s); + const ns = $generateNodesFromSerializedNodes(data.nodes); + $getRoot().append(...ns); + $setSelection(null); + }, + 'direct-shadow-text': () => q2().selectEnd().insertText('X'), + enter: () => q2().getAllTextNodes()[0].select(2, 2).insertParagraph(), + inside: () => q2().getAllTextNodes()[0].select(2, 2).insertText('X'), + 'into-box': () => { + const q = q2(); + const box = $createBoxNode('audit'); + q.insertBefore(box); + box.append(q); + }, + 'into-cell': () => { + const q = q2(); + const table = $createTableNode(); + const cell = $createTableCellNode(); + table.append($createTableRowNode().append(cell)); + q.insertBefore(table); + cell.append(q); + }, + 'into-collapsible': () => { + const q = q2(); + const next = q.getNextSibling()!; + q.remove(); + next.insertBefore( + $createCollapsibleContainerNode(true).append( + $createCollapsibleTitleNode().append(p('Title')), + $createCollapsibleContentNode().append(q), + ), + ); + }, + 'into-layout': () => { + const q = q2(); + const next = q.getNextSibling()!; + q.remove(); + next.insertBefore( + $createLayoutContainerNode('1fr').append( + $createLayoutItemNode().append(q), + ), + ); + }, + 'into-slot': () => { + const q = q2(); + const next = q.getNextSibling()!; + q.remove(); + next.insertBefore($createDownstreamComponent('callout', {body: [q]})); + }, + 'lift-block': () => { + const q = q2(); + q.insertAfter(q.getFirstChild()!); + q.remove(); + }, + 'toggle-shadow-off': () => { + (q2() as QuoteNode).setIsShadowRoot(false); + }, + 'type-outside': () => { + ($getRoot().getFirstChild() as ElementNode) + .getFirstChild()! + .selectEnd() + .insertText('X'); + }, +}; +for (const gc of [false, true]) + for (const ids of [ + [1, 2], + [2, 1], + ]) + for (const delivery of ['causal', 'sender'] as const) + for (const op of Object.keys(ops)) { + test(`${op} gc=${gc} ids=${ids} delivery=${delivery}`, async () => { + const t = await fixture({delivery, gc, ids, nodes: CATALOG_NODES}); + t.peers.forEach(equalFresh); + t.a.update(ops[op]); + equalFresh(t.a); + await t.sync(); + t.peers.forEach(equalFresh); + const state = t.a.read(() => $getRoot().getTextContent()); + expect(state).toContain('q2'); + // Never let a hidden link start applying to untouched text. + t.peers.forEach(q => noLinks(q)); + t.a.undo(); + await t.sync(); + t.peers.forEach(equalFresh); + t.peers.forEach(q => noLinkOn(q)); + t.a.binding.redo(); + await t.sync(); + t.peers.forEach(equalFresh); + t.peers.forEach(q => noLinks(q)); + for (const q of t.peers) + expect(q.warnings.filter(w => w.includes('self-check'))).toEqual( + [], + ); + }); + } +for (const side of ['before', 'after']) + for (const gc of [false, true]) + for (const ids of [ + [1, 2], + [2, 1], + ]) { + test(`outside writer adds linked text ${side} gc=${gc} ids=${ids}`, async () => { + const t = await fixture({gc, ids}); + t.peers.forEach(equalFresh); + t.peers.forEach(q => noLinkOn(q)); + await t.apply(root => { + const b = t.paras()[1]; + root.insert(b.at + (side === 'after' ? 1 : 0), ' RAW', { + 'w:link': {url: 'https://o.example'}, + }); + }); + t.peers.forEach(equalFresh); + t.peers.forEach(q => noLinkOn(q)); + }); + } diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654Outside.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654Outside.test.ts new file mode 100644 index 00000000000..0ea93d3862f --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654Outside.test.ts @@ -0,0 +1,45 @@ +/** + * 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 +// #9571: outside a shadow root, the reader keeps a link run around a +// quote's paragraph as it is (the rule is for shadow roots only). +import * as Y from '@y/y'; +import {expect, test} from 'vitest'; + +import {equalFresh, fixture} from './Review9654Fixtures'; + +for (const gc of [false, true]) { + for (const ids of [ + [1, 2], + [2, 1], + ]) { + test(`the same stored shape outside shadow roots stays unchanged gc=${gc} ids=${ids}`, async () => { + const t = await fixture({gc, ids}); + await t.apply(root => { + let changed = 0; + for (let it = root._start; it; it = it.right) { + if ( + !it.deleted && + it.content instanceof Y.ContentType && + it.content.type.getAttr('t') === 'quote' + ) { + it.content.type.setAttr('s:shadowRoot', false); + changed++; + } + } + expect(changed).toBe(2); + }); + for (const q of t.peers) { + expect(q.describe()).toBe( + 'paragraph("Title") / quote(link(paragraph("q1 one"))) / quote(link(paragraph("q2 two"))) / paragraph("end")', + ); + equalFresh(q); + } + }); + } +} diff --git a/packages/lexical-y/src/internal/Read.ts b/packages/lexical-y/src/internal/Read.ts index 03eda8c3d61..51841d26ad5 100644 --- a/packages/lexical-y/src/internal/Read.ts +++ b/packages/lexical-y/src/internal/Read.ts @@ -3507,6 +3507,8 @@ function $inShadowRoot( settler: BlockIdSettler, formatWrappers: ReadonlySet, ): LexicalNode[] { + // Whether each format wrapper holds a block (see $holdsBlock), once each. + const holds = new Map(); const out: LexicalNode[] = []; let run: LexicalNode[] = []; let first: Item | null = null; @@ -3554,8 +3556,8 @@ function $inShadowRoot( children.forEach((c, i) => { // Inline elements around blocks (a link run another writer left open // over the shadow root's blocks): the blocks, out of them. - for (const node of $holdsBlock(c, formatWrappers) - ? $splitAroundBlocks(c, formatWrappers) + for (const node of $holdsBlock(c, formatWrappers, holds) + ? $splitAroundBlocks(c, formatWrappers, holds) : [c]) { if ($isShownBlock(node)) { flush(); @@ -3587,14 +3589,19 @@ function $isShownBlock(node: LexicalNode): boolean { function $holdsBlock( node: LexicalNode, formatWrappers: ReadonlySet, + holds: Map, ): node is ElementNode { - return ( - $isElementNode(node) && - formatWrappers.has(node.getKey()) && - node + if (!$isElementNode(node) || !formatWrappers.has(node.getKey())) { + return false; + } + let known = holds.get(node.getKey()); + if (known === undefined) { + known = node .getChildren() - .some(k => $isShownBlock(k) || $holdsBlock(k, formatWrappers)) - ); + .some(k => $isShownBlock(k) || $holdsBlock(k, formatWrappers, holds)); + holds.set(node.getKey(), known); + } + return known; } /** @@ -3609,13 +3616,14 @@ function $holdsBlock( function $splitAroundBlocks( el: ElementNode, formatWrappers: ReadonlySet, + holds: Map, ): LexicalNode[] { const out: LexicalNode[] = []; const runs: [ElementNode, LexicalNode[]][] = []; let run: LexicalNode[] | null = null; for (const k of el.getChildren()) { - for (const part of $holdsBlock(k, formatWrappers) - ? $splitAroundBlocks(k, formatWrappers) + for (const part of $holdsBlock(k, formatWrappers, holds) + ? $splitAroundBlocks(k, formatWrappers, holds) : [k]) { if ($isShownBlock(part)) { run = null; From bc11722d010740ed2343a60e08e7ab7d69ae9703 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 20:54:53 -0700 Subject: [PATCH 5/7] [lexical-y] Bug Fix: Take format wrappers apart in one walk ## Description Taking blocks out of nested format wrappers copied and rechecked the blocks found below at every level: n wrappers around n paragraphs cost n squared isInline calls (64: 4,163). One walk now pushes each block once to the result and each run of inline content to the wrappers open at its depth (opening, or copying, the chain only after a block closed it). ## Test plan `Review9654DepthWidthCost.test.ts`: 8 to 64 wrappers around as many paragraphs: linear (fails on the previous head: 4,163 at 64). All Review9654 suites and QuotesAndSlots pass. Units of lexical-y, lexical-mark, lexical, lexical-selection, lexical-list, lexical-table, lexical-html, lexical-extension and lexical-playground: 24,640 passed, 182 expected fail, 0 failed. `pnpm run ci-check` passes. --- .../Review9654DepthWidthCost.test.ts | 75 +++++++++++++++++ packages/lexical-y/src/internal/Read.ts | 84 ++++++++++++------- 2 files changed, 131 insertions(+), 28 deletions(-) create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Review9654DepthWidthCost.test.ts diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654DepthWidthCost.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654DepthWidthCost.test.ts new file mode 100644 index 00000000000..0daef9b90aa --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654DepthWidthCost.test.ts @@ -0,0 +1,75 @@ +/** + * 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 +// #9571: n format wrappers nested around n paragraphs are taken apart in +// one walk (linear in wrappers and blocks). +import {LinkNode} from '@lexical/link'; +import {$createQuoteNode} from '@lexical/rich-text'; +import * as Y from '@y/y'; +import {$getRoot, ParagraphNode} from 'lexical'; +import {expect, test, vi} from 'vitest'; + +import {readYState, seedYState} from '../../../headless'; +import {NODES} from '../../utils/harness'; +import {deep, p} from './Review9654Fixtures'; + +test('splitting nested wrappers visits each emitted block a bounded number of times', () => { + const counts: number[] = []; + for (const n of [8, 16, 32, 64]) { + const wrappers = Array.from({length: n}, (_, i) => { + const type = `audit_width_depth_${i}`; + return class ShapeLink extends LinkNode { + $config() { + return this.config(type, {extends: LinkNode}); + } + }; + }); + const config = {nodes: [...NODES, ...wrappers], schemaVersion: 1}; + const seed = seedYState(config, () => + $getRoot().append( + $createQuoteNode({shadowRoot: true}).append( + ...Array.from({length: n}, (_, i) => p(`body${i}`)), + ), + ), + ); + const doc = new Y.Doc({gc: false}); + doc.cleanupFormatting = false; + Y.applyUpdate(doc, seed); + doc + .get('lexical-y') + .format( + 0, + n, + Object.fromEntries( + wrappers.map((_, i) => [ + `w:audit_width_depth_${i}`, + {url: `https://wrapper${i}.example`}, + ]), + ), + ); + let visits = 0; + const original = ParagraphNode.prototype.isInline; + const spy = vi + .spyOn(ParagraphNode.prototype, 'isInline') + .mockImplementation(function (this: ParagraphNode) { + visits++; + return original.call(this); + }); + try { + const shown = readYState(Y.encodeStateAsUpdate(doc), config, () => + deep($getRoot()), + ); + expect(JSON.stringify(shown)).toContain(`"text":"body${n - 1}"`); + } finally { + spy.mockRestore(); + doc.destroy(); + } + counts.push(visits); + } + expect(counts[3]).toBeLessThan(counts[2] * 2.5 + 128); +}); diff --git a/packages/lexical-y/src/internal/Read.ts b/packages/lexical-y/src/internal/Read.ts index 51841d26ad5..8bcabc1d3e5 100644 --- a/packages/lexical-y/src/internal/Read.ts +++ b/packages/lexical-y/src/internal/Read.ts @@ -3605,45 +3605,73 @@ function $holdsBlock( } /** - * A format wrapper that holds blocks (see $holdsBlock), taken apart: the - * blocks, in order, and between them the element around the inline content - * it holds there (itself around the first such run, a copy around each later - * one), at any depth. Why: a shadow root holds blocks only, and a block - * inside an inline element is a shape the editor cannot keep (it was read - * as `link(paragraph)` in a paragraph of its own, and a delete next to it + * A format wrapper that holds blocks (see $holdsBlock), taken apart in one + * walk: the blocks, in order, and between them the wrappers around the + * inline content they hold there (each wrapper itself around its first such + * run, a copy around each later one), at any depth. Each node is looked at + * once. Why: a shadow root holds blocks only, and a block inside an inline + * element is a shape the editor cannot keep (it was read as + * `link(paragraph)` in a paragraph of its own, and a delete next to it * rewrote the paragraph: #9571). */ function $splitAroundBlocks( - el: ElementNode, + top: ElementNode, formatWrappers: ReadonlySet, holds: Map, ): LexicalNode[] { const out: LexicalNode[] = []; - const runs: [ElementNode, LexicalNode[]][] = []; - let run: LexicalNode[] | null = null; - for (const k of el.getChildren()) { - for (const part of $holdsBlock(k, formatWrappers, holds) - ? $splitAroundBlocks(k, formatWrappers, holds) - : [k]) { - if ($isShownBlock(part)) { - run = null; - out.push(part); + // The wrappers walked into, outermost first. + const chain: ElementNode[] = []; + // The wrappers open for inline content now (a prefix of `chain`, as + // written), and the children each gets. + const open: {src: ElementNode; node: ElementNode; kids: LexicalNode[]}[] = []; + const made: [ElementNode, LexicalNode[]][] = []; + const walked: ElementNode[] = []; + const used = new Set(); + const target = (): LexicalNode[] => { + const depth = chain.length; + if (open.length < depth || open[depth - 1].src !== chain[depth - 1]) { + let d = 0; + while (d < open.length && d < depth && open[d].src === chain[d]) { + d++; + } + open.length = d; + for (; d < depth; d++) { + const src = chain[d]; + const node = used.has(src.getKey()) ? $copyNode(src) : src; + used.add(src.getKey()); + const kids: LexicalNode[] = []; + made.push([node, kids]); + (d === 0 ? out : open[d - 1].kids).push(node); + open.push({kids, node, src}); + } + } + open.length = depth; + return open[depth - 1].kids; + }; + const walk = (el: ElementNode) => { + chain.push(el); + walked.push(el); + for (const k of el.getChildren()) { + if ($isShownBlock(k)) { + open.length = 0; + out.push(k); + } else if ($holdsBlock(k, formatWrappers, holds)) { + walk(k); } else { - if (run === null) { - const wrapper = runs.length === 0 ? el : $copyNode(el); - run = []; - runs.push([wrapper, run]); - out.push(wrapper); - } - run.push(part); + target().push(k); } } + chain.pop(); + }; + walk(top); + for (const el of walked) { + if (!used.has(el.getKey())) { + el.clear(); + } } - if (runs.length === 0 || runs[0][0] !== el) { - el.clear(); - } - for (const [wrapper, kids] of runs) { - wrapper.splice(0, wrapper.getChildrenSize(), kids); + for (const [node, kids] of made) { + node.splice(0, node.getChildrenSize(), kids); } return out; } From d2e9c90e3ad352e70b27c99b62cc940cae39dd5a Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 21:09:24 -0700 Subject: [PATCH 6/7] [lexical-y] Bug Fix: Open only the wrappers past the open prefix ## Description Each descent into a format wrapper compared the open wrappers with the walked ones from the outermost again: inline text at every depth cost the square of the depth. The open wrappers are now kept a prefix of the walked ones (a block empties them, leaving a wrapper shortens them), so only the wrappers past them are opened. ## Test plan `Review9654OpenChainCost.test.ts`: depth 8 to 64 with inline text at every depth and one innermost block: linear (fails on the previous head: 2,079 at 64). All Review9654 suites and QuotesAndSlots pass. Units of lexical-y, lexical-mark, lexical, lexical-selection, lexical-list, lexical-table, lexical-html, lexical-extension and lexical-playground: 25,047 passed, 418 expected fail, 0 failed. `pnpm run ci-check` passes. --- .../Review9654OpenChainCost.test.ts | 109 ++++++++++++++++++ packages/lexical-y/src/internal/Read.ts | 37 +++--- 2 files changed, 125 insertions(+), 21 deletions(-) create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Review9654OpenChainCost.test.ts diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654OpenChainCost.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654OpenChainCost.test.ts new file mode 100644 index 00000000000..a4f8673ace2 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654OpenChainCost.test.ts @@ -0,0 +1,109 @@ +/** + * 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 +// #9571: inline text at every depth of nested format wrappers opens each +// wrapper once (linear in depth). +import {LinkNode} from '@lexical/link'; +import {$createQuoteNode} from '@lexical/rich-text'; +import * as Y from '@y/y'; +import {$getRoot, ElementNode} from 'lexical'; +import {expect, test} from 'vitest'; + +import {readYState, seedYState} from '../../../headless'; +import {NODES} from '../../utils/harness'; +import {deep, p} from './Review9654Fixtures'; + +test('opening an unchanged wrapper-chain prefix is linear in depth', () => { + const counts: number[] = []; + for (const depth of [8, 16, 32, 64]) { + const wrappers = Array.from({length: depth}, (_, i) => { + const type = `audit_open_${i}`; + return class OpenLink extends LinkNode { + $config() { + return this.config(type, {extends: LinkNode}); + } + }; + }); + const config = {nodes: [...NODES, ...wrappers], schemaVersion: 1}; + const doc = new Y.Doc({gc: false}); + doc.cleanupFormatting = false; + Y.applyUpdate( + doc, + seedYState(config, () => + $getRoot().append( + $createQuoteNode({shadowRoot: true}).append(p('BODY')), + ), + ), + ); + const root = doc.get('lexical-y'); + const attrs = (n: number) => + Object.fromEntries( + Array.from({length: n}, (_, i) => [ + `w:audit_open_${i}`, + {url: `https://w${i}.example`}, + ]), + ); + doc.transact(() => { + for (let i = 0; i < depth; i++) root.insert(i, 'L', attrs(i + 1)); + root.format(depth, 1, attrs(depth)); + for (let i = 0; i < depth; i++) + root.insert(depth + 1 + i, 'R', attrs(depth - i)); + }); + let reads = 0; + const original = Array.prototype.push; + // eslint-disable-next-line no-extend-native + Array.prototype.push = function (this: unknown[], ...items: unknown[]) { + for (const value of items) { + if (value !== null && typeof value === 'object') { + const frame = value as { + src?: unknown; + node?: unknown; + kids?: unknown; + }; + if ( + frame.src instanceof ElementNode && + frame.node instanceof ElementNode && + Array.isArray(frame.kids) + ) { + const src = frame.src; + Object.defineProperty(frame, 'src', { + configurable: true, + get: () => { + reads++; + return src; + }, + }); + } + } + } + return original.apply(this, items); + }; + try { + const shown = readYState(Y.encodeStateAsUpdate(doc), config, () => + deep($getRoot()), + ); + const texts: string[] = []; + const collect = (value: unknown): void => { + if (typeof value !== 'object' || value === null) return; + const node = value as {text?: unknown; children?: unknown[]}; + if (typeof node.text === 'string') texts.push(node.text); + for (const child of node.children ?? []) collect(child); + }; + collect(shown); + expect(texts.join('')).toBe( + 'L'.repeat(depth) + 'BODY' + 'R'.repeat(depth), + ); + } finally { + // eslint-disable-next-line no-extend-native + Array.prototype.push = original; + doc.destroy(); + } + counts.push(reads); + } + expect(counts[3]).toBeLessThan(counts[2] * 2.5 + 128); +}); diff --git a/packages/lexical-y/src/internal/Read.ts b/packages/lexical-y/src/internal/Read.ts index 8bcabc1d3e5..2bc68a0bdb6 100644 --- a/packages/lexical-y/src/internal/Read.ts +++ b/packages/lexical-y/src/internal/Read.ts @@ -3622,32 +3622,24 @@ function $splitAroundBlocks( const out: LexicalNode[] = []; // The wrappers walked into, outermost first. const chain: ElementNode[] = []; - // The wrappers open for inline content now (a prefix of `chain`, as - // written), and the children each gets. + // The wrappers open for inline content now, and the children each gets. const open: {src: ElementNode; node: ElementNode; kids: LexicalNode[]}[] = []; const made: [ElementNode, LexicalNode[]][] = []; const walked: ElementNode[] = []; const used = new Set(); + // Invariant: `open` is a prefix of `chain` (a block empties it, leaving a + // wrapper shortens it), so only the wrappers past it are opened. const target = (): LexicalNode[] => { - const depth = chain.length; - if (open.length < depth || open[depth - 1].src !== chain[depth - 1]) { - let d = 0; - while (d < open.length && d < depth && open[d].src === chain[d]) { - d++; - } - open.length = d; - for (; d < depth; d++) { - const src = chain[d]; - const node = used.has(src.getKey()) ? $copyNode(src) : src; - used.add(src.getKey()); - const kids: LexicalNode[] = []; - made.push([node, kids]); - (d === 0 ? out : open[d - 1].kids).push(node); - open.push({kids, node, src}); - } - } - open.length = depth; - return open[depth - 1].kids; + for (let d = open.length; d < chain.length; d++) { + const src = chain[d]; + const node = used.has(src.getKey()) ? $copyNode(src) : src; + used.add(src.getKey()); + const kids: LexicalNode[] = []; + made.push([node, kids]); + (d === 0 ? out : open[d - 1].kids).push(node); + open.push({kids, node, src}); + } + return open[chain.length - 1].kids; }; const walk = (el: ElementNode) => { chain.push(el); @@ -3663,6 +3655,9 @@ function $splitAroundBlocks( } } chain.pop(); + if (open.length > chain.length) { + open.length = chain.length; + } }; walk(top); for (const el of walked) { From 1cdc66b00994644fa1813a8eebe06069c9358b2d Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 21:59:04 -0700 Subject: [PATCH 7/7] [lexical-y] Chore: Document which local edits close the outside run, and count a real cost in the open-chain test ## Description - D89: the read writes nothing; the stored link stays until a local edit writes that quote's content again (Enter in its paragraph, a paragraph appended to it), which stores a null of the link before what it writes and closes the run there. Nothing is lost and every reader shows the same. No behaviour change. - `Review9654OpenChainCost.test.ts` counted reads of a field the walk no longer has, so it could not fail; it now counts children enumerated during the read (and checks that count is not zero). - `Review9654EnterClosesRun.test.ts`: Enter in the second quote's paragraph stores a null of the link from the local client before it; every client shows the text with no link, as a fresh read does (gc on and off). - Changeset: the link run starts just before the paragraph. ## Test plan Units of lexical-y, lexical-mark, lexical, lexical-selection, lexical-list, lexical-table, lexical-html, lexical-extension and lexical-playground: 25,049 passed, 418 expected fail, 0 failed. `pnpm run ci-check` passes. --- .changeset/y-shadow-root-link-over-blocks.md | 2 +- packages/lexical-y/DECISIONS.md | 2 +- .../Review9654EnterClosesRun.test.ts | 54 +++++++++++++++++++ .../Review9654OpenChainCost.test.ts | 45 +++++----------- 4 files changed, 69 insertions(+), 34 deletions(-) create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/Review9654EnterClosesRun.test.ts diff --git a/.changeset/y-shadow-root-link-over-blocks.md b/.changeset/y-shadow-root-link-over-blocks.md index 4d46fbfab55..88aa4dd2d9f 100644 --- a/.changeset/y-shadow-root-link-over-blocks.md +++ b/.changeset/y-shadow-root-link-over-blocks.md @@ -4,4 +4,4 @@ [fix] A link run another writer left open over a shadow-root quote's paragraph no longer makes a delete of the quote before it rewrite that paragraph -Verify: with a plain Yjs client, format the root sequence with a link from inside one shadow-root quote's paragraph to past the next quote's paragraph; delete the first quote while a collaborator types in the second: the collaborator's text stays, and every client shows the quotes' paragraphs as paragraphs. +Verify: with a plain Yjs client, format the root sequence with a link from just before one shadow-root quote's paragraph to past the next quote's paragraph; delete the first quote while a collaborator types in the second: the collaborator's text stays, and every client shows the quotes' paragraphs as paragraphs. diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index 8c30baf0282..1dd18589fc8 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -137,7 +137,7 @@ issues and docs that carry it. | 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] | -| D89 | **Inline elements around blocks, in a shadow root, read as the blocks out of them (proposed).** Another writer's link run left open over a shadow-root quote's paragraph read as `link(paragraph)` inside a paragraph of its own, a shape no editor keeps; now the paragraph shows on its own, at any depth of such elements, and inline content they also hold stays in them, in a paragraph of its own. The write self-check does not count a stored wrapper over a block in a shadow root (only there). | A delete of the quote before it rewrote the next quote's paragraph (deleted and inserted it again), losing a collaborator's concurrent typing there ([#9571]). It extends how a shadow root already shows its content (inline runs in paragraphs of their own, `$inShadowRoot`), so it is a second case of D70's "text left without a line marker shows as a paragraph": the stored link stays and the next local edit there does not remove it. `@lexical/y`'s own writers leave no such runs. For Gerard against D70. | `OPEN` | 10-30 | [#9571] | +| D89 | **Inline elements around blocks, in a shadow root, read as the blocks out of them (proposed).** Another writer's link run left open over a shadow-root quote's paragraph read as `link(paragraph)` inside a paragraph of its own, a shape no editor keeps; now the paragraph shows on its own, at any depth of such elements, and inline content they also hold stays in them, in a paragraph of its own. The write self-check does not count a stored wrapper over a block in a shadow root (only there). | A delete of the quote before it rewrote the next quote's paragraph (deleted and inserted it again), losing a collaborator's concurrent typing there ([#9571]). It extends how a shadow root already shows its content (inline runs in paragraphs of their own, `$inShadowRoot`), so it is a second case of D70's "text left without a line marker shows as a paragraph": the read writes nothing, and the stored link stays until a local edit writes that quote's content again: Enter in its paragraph or a paragraph appended to it stores a null of the link before what it writes, which closes the run there (nothing is lost, and every reader shows the same). `@lexical/y`'s own writers leave no such runs. For Gerard against D70. | `OPEN` | 10-30 | [#9571] | [root-agents]: ../../AGENTS.md [limits]: ../lexical-website/docs/collaboration/y.md#limits diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654EnterClosesRun.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654EnterClosesRun.test.ts new file mode 100644 index 00000000000..99a19539007 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654EnterClosesRun.test.ts @@ -0,0 +1,54 @@ +/** + * 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 +// #9571 (D89): a local edit that writes the second quote's content again +// (Enter inside its paragraph) closes the outside link run there: it stores +// a null of the link before the paragraph it writes. Nothing is lost, and +// every reader shows the same content, with no link around the quote's +// paragraphs. +import * as Y from '@y/y'; +import {$getRoot} from 'lexical'; +import {expect, test} from 'vitest'; + +import {equalFresh, fixture, noLinks, q2} from './Review9654Fixtures'; + +/** The `w:link` format items in the root sequence, as `client:value`. */ +function linkItems(doc: Y.Doc): string[] { + const out: string[] = []; + for (let it = doc.get('lexical-y')._start; it !== null; it = it.right) { + if ( + !it.deleted && + it.content instanceof Y.ContentFormat && + it.content.key === 'w:link' + ) { + out.push( + `${it.id.client === doc.clientID ? 'own' : 'other'}:${JSON.stringify(it.content.value)}`, + ); + } + } + return out; +} + +for (const gc of [false, true]) { + test(`Enter in the second quote's paragraph stores a null of the link before it (gc=${gc})`, async () => { + const t = await fixture({gc, variant: 'single'}); + expect(linkItems(t.a.doc).filter(s => s.startsWith('own'))).toEqual([]); + t.a.update(() => { + q2().getAllTextNodes()[0].select(2, 2).insertParagraph(); + }); + await t.sync(); + expect(linkItems(t.a.doc)).toContain('own:null'); + for (const peer of t.peers) { + expect( + peer.read(() => $getRoot().getChildAtIndex(2)!.getTextContent()), + ).toBe('q2\n\n two'); + noLinks(peer); + equalFresh(peer); + } + }); +} diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Review9654OpenChainCost.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Review9654OpenChainCost.test.ts index a4f8673ace2..fdbe0dd5241 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/Review9654OpenChainCost.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/Review9654OpenChainCost.test.ts @@ -12,13 +12,13 @@ import {LinkNode} from '@lexical/link'; import {$createQuoteNode} from '@lexical/rich-text'; import * as Y from '@y/y'; import {$getRoot, ElementNode} from 'lexical'; -import {expect, test} from 'vitest'; +import {expect, test, vi} from 'vitest'; import {readYState, seedYState} from '../../../headless'; import {NODES} from '../../utils/harness'; import {deep, p} from './Review9654Fixtures'; -test('opening an unchanged wrapper-chain prefix is linear in depth', () => { +test('taking apart wrappers with inline text at every depth is linear in depth', () => { const counts: number[] = []; for (const depth of [8, 16, 32, 64]) { const wrappers = Array.from({length: depth}, (_, i) => { @@ -54,35 +54,16 @@ test('opening an unchanged wrapper-chain prefix is linear in depth', () => { for (let i = 0; i < depth; i++) root.insert(depth + 1 + i, 'R', attrs(depth - i)); }); + // Children enumerated during the read (each wrapper's once when it is + // taken apart). let reads = 0; - const original = Array.prototype.push; - // eslint-disable-next-line no-extend-native - Array.prototype.push = function (this: unknown[], ...items: unknown[]) { - for (const value of items) { - if (value !== null && typeof value === 'object') { - const frame = value as { - src?: unknown; - node?: unknown; - kids?: unknown; - }; - if ( - frame.src instanceof ElementNode && - frame.node instanceof ElementNode && - Array.isArray(frame.kids) - ) { - const src = frame.src; - Object.defineProperty(frame, 'src', { - configurable: true, - get: () => { - reads++; - return src; - }, - }); - } - } - } - return original.apply(this, items); - }; + const original = ElementNode.prototype.getChildren; + const spy = vi + .spyOn(ElementNode.prototype, 'getChildren') + .mockImplementation(function (this: ElementNode) { + reads++; + return original.call(this); + }); try { const shown = readYState(Y.encodeStateAsUpdate(doc), config, () => deep($getRoot()), @@ -99,11 +80,11 @@ test('opening an unchanged wrapper-chain prefix is linear in depth', () => { 'L'.repeat(depth) + 'BODY' + 'R'.repeat(depth), ); } finally { - // eslint-disable-next-line no-extend-native - Array.prototype.push = original; + spy.mockRestore(); doc.destroy(); } counts.push(reads); } + expect(counts[0]).toBeGreaterThan(0); expect(counts[3]).toBeLessThan(counts[2] * 2.5 + 128); });