From 3c1859db11d975ae2d44e0a1c626c56a2263dea4 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 12:50:19 -0400 Subject: [PATCH 01/18] [lexical-y] Bug Fix: Keep a link a collaborator removes removed when another collaborator formats text covering it ## Description A removes a link while B, at the same time, makes a range covering the whole link code (#9597): after the sync the link was back on every client. B's format also sets the link's textFormat (`RangeSelection#formatText` sets it on element nodes the selection covers), and the binding stored a wrapper's textFormat and textStyle in the value its text carries (`w:link`). B's write of the new value went over the link's text as a new opener and a restore of the old value after it, and A's concurrent removal, which deleted the opener B had seen, did not reach B's new opener: the link came back. A wrapper's textFormat and textStyle are now stored only while it has no text of its own (`shouldSerializeTextStyles`), as Lexical serializes them: with text, they follow it. A format change over a link then writes no link value. Documents that hold them in a link's value still read as before. `regressions/Formats.test.ts` S7-26 asserted the old result of the same mechanism (B links "t7z t8z t9z" and undoes it while A formats a range over the link's start, and the link stayed): it now expects the link gone on every client, as B undid it and A did not touch the link. Closes #9597 ## Test plan `packages/lexical-y/src/__tests__/unit/LinkRemovedWhileFormatted.test.ts` (new): the issue's repro, both client id orders. ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/LinkRemovedWhileFormatted.test.ts Tests 2 failed (2) AssertionError: expected [ 't29z t30z' ] to deeply equal [] ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/LinkRemovedWhileFormatted.test.ts Tests 2 passed (2) $ pnpm exec vitest run --project unit packages/lexical-y packages/lexical-mark Test Files 183 passed | 2 skipped (185) Tests 8688 passed | 46 expected fail | 4 skipped (8738) ``` `pnpm run ci-check` passes. --- .changeset/lexical-y-link-textformat.md | 9 ++ .../unit/LinkRemovedWhileFormatted.test.ts | 135 ++++++++++++++++++ .../unit/regressions/Formats.test.ts | 8 +- packages/lexical-y/src/internal/Schema.ts | 9 +- 4 files changed, 157 insertions(+), 4 deletions(-) create mode 100644 .changeset/lexical-y-link-textformat.md create mode 100644 packages/lexical-y/src/__tests__/unit/LinkRemovedWhileFormatted.test.ts diff --git a/.changeset/lexical-y-link-textformat.md b/.changeset/lexical-y-link-textformat.md new file mode 100644 index 00000000000..a72884fd112 --- /dev/null +++ b/.changeset/lexical-y-link-textformat.md @@ -0,0 +1,9 @@ +--- +'@lexical/y': patch +--- + +[fix] Keep a link a collaborator removes removed when another collaborator formats text covering it at the same time + +A format change over a link also sets the link's textFormat (`RangeSelection#formatText`), and the binding stored a link's textFormat and textStyle in the value its text carries: the format change wrote the link's whole value again over its text, and that write brought a concurrently removed link back for everyone. A wrapper's textFormat and textStyle are now stored only while it has no text of its own, as Lexical serializes them; with text, they follow it. + +Verify: with two clients on a paragraph that holds a link, remove the link on one while the other makes a range covering the whole link code, sync, and check that the link stays removed on both. diff --git a/packages/lexical-y/src/__tests__/unit/LinkRemovedWhileFormatted.test.ts b/packages/lexical-y/src/__tests__/unit/LinkRemovedWhileFormatted.test.ts new file mode 100644 index 00000000000..4e997271692 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/LinkRemovedWhileFormatted.test.ts @@ -0,0 +1,135 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +// @vitest-environment node + +// A removes a link while B, at the same time, formats a range covering the +// whole link (code). B's format sets the link's textFormat too +// (RangeSelection#formatText); with the link's textFormat stored in its +// value, B wrote the link's value again over its text, and that write +// brought the removed link back for everyone. The link stays removed. + +import {buildEditorFromExtensions} from '@lexical/extension'; +import { + $createLinkNode, + $isLinkNode, + $toggleLink, + LinkExtension, +} from '@lexical/link'; +import {RichTextExtension} from '@lexical/rich-text'; +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $isElementNode, + $setSelection, + configExtension, + defineExtension, + type LexicalEditor, + type TextNode, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {YCollabExtension} from '../..'; +import {createYDoc, seedYState} from '../../headless'; + +const extensions = [RichTextExtension, LinkExtension]; +const config = {extensions, onWarn: () => {}, schemaVersion: 1}; + +// "t28z " + link(https://c.test/100: "t29z t30z") + " t31z" +function $build() { + $getRoot() + .clear() + .append( + $createParagraphNode().append( + $createTextNode('t28z '), + $createLinkNode('https://c.test/100').append( + $createTextNode('t29z t30z'), + ), + $createTextNode(' t31z'), + ), + ); +} + +function client(seed: Uint8Array, id: number) { + const doc = createYDoc(seed); + doc.clientID = id; + const editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + dependencies: [ + ...extensions, + configExtension(YCollabExtension, {doc, schemaVersion: 1}), + ], + name: 'client' + id, + }), + ); + return {doc, editor}; +} + +function $select(from: string, to: string) { + const texts = $getRoot().getAllTextNodes(); + const find = (word: string, end: boolean): [TextNode, number] => { + for (const t of texts) { + const i = t.getTextContent().indexOf(word); + if (i >= 0) return [t, end ? i + word.length : i]; + } + throw new Error(word); + }; + const [a, ao] = find(from, false); + const [b, bo] = find(to, true); + const s = $createRangeSelection(); + s.anchor.set(a.getKey(), ao, 'text'); + s.focus.set(b.getKey(), bo, 'text'); + $setSelection(s); + return s; +} + +const links = (editor: LexicalEditor) => + editor.getEditorState().read(() => + $getRoot() + .getChildren() + .flatMap(p => ($isElementNode(p) ? p.getChildren() : [])) + .filter($isLinkNode) + .map(l => l.getTextContent()), + ); + +for (const ids of [ + [1, 2], + [2, 1], +]) { + test(`a removed link stays removed when a collaborator formats over it at the same time (ids ${ids})`, async () => { + const seed = seedYState(config, $build); + const a = client(seed, ids[0]); + const b = client(seed, ids[1]); + await Promise.resolve(); + // A removes the link; B, at the same time, makes "t28z ... t30z" code + a.editor.update( + () => { + $select('t29z', 't30z'); + $toggleLink(null); + }, + {discrete: true}, + ); + b.editor.update( + () => { + $select('t28z', 't30z').formatText('code'); + }, + {discrete: true}, + ); + const ua = Y.encodeStateAsUpdate(a.doc); + const ub = Y.encodeStateAsUpdate(b.doc); + Y.applyUpdate(a.doc, ub); + Y.applyUpdate(b.doc, ua); + await Promise.resolve(); + expect(links(a.editor)).toEqual([]); + expect(links(b.editor)).toEqual([]); + }); +} diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts index c2a94e1efa3..0d1dfbc8b9b 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts @@ -1697,8 +1697,10 @@ describe('a value one client restates ‖ the undo of the change that gave it', ); }); // S7-26: a link's undo ‖ a format change over the link's start, which - // also sets the link's textFormat (RangeSelection#formatText): that - // write restates the link over its text, which keeps it there. + // also sets the link's textFormat (RangeSelection#formatText). That + // write restated the link over its text and kept it there; a link's + // textFormat is no longer stored while it has text (#9597), so the + // format change writes no link and B's undo removes it on every peer. for (const format of ['italic', 'bold'] as const) { test(`S7-26: B links "t7z t8z t9z" and undoes it ‖ A makes "t5z t6z t7z" ${format} (ids ${ids})`, async () => { const t = await setup( @@ -1717,7 +1719,7 @@ describe('a value one client restates ‖ the undo of the change that gave it', await merged(t); const fresh = await freshReader(t.a); for (const p of [t.a, t.b, fresh]) { - expect(p.read($links)).toBe('t7z t8z t9z / '); + expect(p.read($links)).toBe(' / '); } }); } diff --git a/packages/lexical-y/src/internal/Schema.ts b/packages/lexical-y/src/internal/Schema.ts index da5d7f9b89f..342f2634ae8 100644 --- a/packages/lexical-y/src/internal/Schema.ts +++ b/packages/lexical-y/src/internal/Schema.ts @@ -1995,7 +1995,14 @@ export function $wrapperAttrs( putValue(own, f, json[f], `${node.getType()}.${f}`); } $stateAttrs(node.getLatest(), own); - $putTextFields(node.getLatest(), own); + // Only while the wrapper has no text of its own, as Lexical serializes + // them: with text, they follow its text (a format of that text sets + // them), and a write of them alone would write the whole value again over + // the wrapper's text, which a collaborator's concurrent removal of the + // wrapper does not delete (the removed link came back). + if (node.getLatest().shouldSerializeTextStyles()) { + $putTextFields(node.getLatest(), own); + } for (const k in own) { const v = own[k]; // Yjs compares format values one level deep: a value that is not a From 52ec6126925188c414d1532619aeb24a0da43914 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 13:33:45 -0400 Subject: [PATCH 02/18] [lexical-y] Bug Fix: Keep a removed link removed also when its text sits in an inline element inside it ## Description Follow-up to the previous commit (store a wrapper's textFormat and textStyle only while it has no text). That checked Lexical's `shouldSerializeTextStyles`, which looks at the wrapper's direct children only: a link whose text sits in a mark (link > mark > text) still stored them, B's format change over it still wrote the link's whole value again, and A's concurrent removal of the link was lost the same way. A wrapper now stores them only when it holds no text node anywhere inside it, through inline elements too. Reported in review. ## Test plan `LinkWithMarkRemovedWhileFormatted.test.ts` (new, from the review): the issue's steps with the link's text inside a mark, both client id orders. ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/LinkWithMarkRemovedWhileFormatted.test.ts Tests 2 failed (2) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/LinkWithMarkRemovedWhileFormatted.test.ts packages/lexical-y/src/__tests__/unit/LinkRemovedWhileFormatted.test.ts Tests 4 passed (4) $ pnpm exec vitest run --project unit packages/lexical-y packages/lexical-mark packages/lexical-link Test Files 196 passed | 2 skipped (198) Tests 8977 passed | 46 expected fail | 4 skipped (9027) ``` `pnpm run ci-check` passes. --- .../LinkWithMarkRemovedWhileFormatted.test.ts | 144 ++++++++++++++++++ packages/lexical-y/src/internal/Schema.ts | 27 +++- 2 files changed, 165 insertions(+), 6 deletions(-) create mode 100644 packages/lexical-y/src/__tests__/unit/LinkWithMarkRemovedWhileFormatted.test.ts diff --git a/packages/lexical-y/src/__tests__/unit/LinkWithMarkRemovedWhileFormatted.test.ts b/packages/lexical-y/src/__tests__/unit/LinkWithMarkRemovedWhileFormatted.test.ts new file mode 100644 index 00000000000..19169527a37 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/LinkWithMarkRemovedWhileFormatted.test.ts @@ -0,0 +1,144 @@ +/** + * 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 + +// As LinkRemovedWhileFormatted, with the link's text inside a mark: the +// link holds its text through an inline element, and B's format still +// writes no link value, so A's removal of the link stays. + +import {buildEditorFromExtensions} from '@lexical/extension'; +import { + $createLinkNode, + $isLinkNode, + $toggleLink, + LinkExtension, +} from '@lexical/link'; +import {$createMarkNode, MarkExtension} from '@lexical/mark'; +import {RichTextExtension} from '@lexical/rich-text'; +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getRoot, + $isElementNode, + $setSelection, + configExtension, + defineExtension, + type LexicalEditor, + type TextNode, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {YCollabExtension} from '../..'; +import {createYDoc, seedYState} from '../../headless'; + +const extensions = [RichTextExtension, LinkExtension, MarkExtension]; +const config = {extensions, schemaVersion: 1}; + +function $build(): void { + $getRoot() + .clear() + .append( + $createParagraphNode().append( + $createTextNode('before '), + $createLinkNode('https://nested.test').append( + $createMarkNode(['c1']).append($createTextNode('linked words')), + ), + $createTextNode(' after'), + ), + ); +} + +function client(seed: Uint8Array, id: number) { + const doc = createYDoc(seed); + doc.clientID = id; + const editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + dependencies: [ + ...extensions, + configExtension(YCollabExtension, {doc, schemaVersion: 1}), + ], + name: `nested-${id}`, + }), + ); + return {doc, editor}; +} + +function $select(from: string, to: string) { + const find = (word: string, end: boolean): [TextNode, number] => { + for (const text of $getRoot().getAllTextNodes() as TextNode[]) { + const offset = text.getTextContent().indexOf(word); + if (offset >= 0) { + return [text, end ? offset + word.length : offset]; + } + } + throw new Error(`missing ${word}`); + }; + const [a, ao] = find(from, false); + const [b, bo] = find(to, true); + const selection = $createRangeSelection(); + selection.anchor.set(a.getKey(), ao, 'text'); + selection.focus.set(b.getKey(), bo, 'text'); + $setSelection(selection); + return selection; +} + +function links(editor: LexicalEditor): string[] { + return editor.read(() => { + const out: string[] = []; + const visit = (node: ReturnType) => { + for (const child of node.getChildren()) { + if ($isLinkNode(child)) { + out.push(child.getTextContent()); + } + if ($isElementNode(child)) { + visit(child as ReturnType); + } + } + }; + visit($getRoot()); + return out; + }); +} + +for (const ids of [ + [1, 2], + [2, 1], +]) { + test(`a removed link containing a mark stays removed when formatted (ids ${ids})`, async () => { + const seed = seedYState(config, $build); + const a = client(seed, ids[0]); + const b = client(seed, ids[1]); + await Promise.resolve(); + a.editor.update( + () => { + $select('linked', 'words'); + $toggleLink(null); + }, + {discrete: true}, + ); + expect(links(a.editor)).toEqual([]); + b.editor.update( + () => { + $select('before', 'words').formatText('code'); + }, + {discrete: true}, + ); + expect(links(b.editor)).toEqual(['linked words']); + const ua = Y.encodeStateAsUpdate(a.doc); + const ub = Y.encodeStateAsUpdate(b.doc); + Y.applyUpdate(a.doc, ub); + Y.applyUpdate(b.doc, ua); + await Promise.resolve(); + expect(links(a.editor)).toEqual([]); + expect(links(b.editor)).toEqual([]); + }); +} diff --git a/packages/lexical-y/src/internal/Schema.ts b/packages/lexical-y/src/internal/Schema.ts index 342f2634ae8..224ce16f98e 100644 --- a/packages/lexical-y/src/internal/Schema.ts +++ b/packages/lexical-y/src/internal/Schema.ts @@ -1995,12 +1995,13 @@ export function $wrapperAttrs( putValue(own, f, json[f], `${node.getType()}.${f}`); } $stateAttrs(node.getLatest(), own); - // Only while the wrapper has no text of its own, as Lexical serializes - // them: with text, they follow its text (a format of that text sets - // them), and a write of them alone would write the whole value again over - // the wrapper's text, which a collaborator's concurrent removal of the - // wrapper does not delete (the removed link came back). - if (node.getLatest().shouldSerializeTextStyles()) { + // Only while the wrapper holds no text, directly or through inline + // elements inside it (a mark in a link): with text, they follow that text + // (a format of it sets them), and a write of them alone would write the + // whole value again over the wrapper's text, which a collaborator's + // concurrent removal of the wrapper does not delete (the removed link + // came back). + if (!$holdsText(node.getLatest())) { $putTextFields(node.getLatest(), own); } for (const k in own) { @@ -2146,6 +2147,20 @@ export function $lineAttrs( return attrs; } +/** Whether `node` holds a text node, directly or inside its descendants. */ +function $holdsText(node: ElementNode): boolean { + for (let child = node.getFirstChild(); child !== null; ) { + if ($isTextNode(child)) { + return true; + } + if ($isElementNode(child) && $holdsText(child)) { + return true; + } + child = child.getNextSibling(); + } + return false; +} + /** * An element's textFormat/textStyle, which Lexical serializes only while * the element has no text: every peer must hold the same values, so groups From 4072e4fbeedcaf10057e139e3e7737ab5df3f9bf Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 16:22:12 -0700 Subject: [PATCH 03/18] [lexical][lexical-website][lexical-y] Bug Fix: Give a link whose text sits in a mark the same JSON for its author and a fresh reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Follow-up to the previous commit, which stores a wrapper's textFormat and textStyle only while it holds no text anywhere inside it, so that a link whose text sits in a mark (link > mark > text) stays removed under a concurrent format. Lexical's `ElementNode#shouldSerializeTextStyles` looked at direct TextNode children only, so after an ordinary format the author's link still wrote `textFormat` while the binding no longer stored it: the author's JSON and a fresh reader's differed. `shouldSerializeTextStyles` now checks an inline element (a link, an autolink, a mark) for a TextNode anywhere inside it. A reconcile already derives an element's textFormat and textStyle from its first text at any depth, and the JSON of an inline element now follows the same rule. A block keeps the direct-children check, so its JSON is unchanged. The binding stores a wrapper's textFormat and textStyle exactly when `shouldSerializeTextStyles` is true, so the two agree by construction (including for a subclass that overrides it). Exported JSON changes only for a link, autolink or mark whose children are all elements: it no longer writes those two fields. JSON that has them still reads into the same state. Recorded as D81 in `packages/lexical-y/DECISIONS.md`. A link that holds no text at all (only an inline decorator or a line break) still has to store them, and still comes back in the same race: #9627, marked `test.fails` in the new matrix. ## Test plan `packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts` (new): link, autolink, mark (two ids) and overflow nested one to two deep in every order (three with `MATRIX_FULL=1`), around plain, bold, styled text, two texts, a line break, text next to a nested mark or an inline decorator, a decorator alone and a line break alone, with and without text fields of their own. It checks that the author's JSON and a peer's equal a fresh reader's after a format or style change over the paragraph or the wrapped text. It also checks that every wrapper one client removes stays removed while another formats over it, in both client id orders. `packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts` (new): which elements persist the two fields, and stored JSON that has them. ### Before On the previous commit: ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 42 failed | 1848 passed (1890) an author and a fresh reader have the same JSON > link>mark>text AssertionError: format-all: author: expected { root: { …(8) } } to deeply equal { root: { …(8) } } ``` On main (before this pull request): ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 196 failed | 1694 passed (1890) a wrapper one client removes stays removed while another formats over it > link>text: remove link, ids 1,2 AssertionError: a: expected 1 to be +0 // Object.is equality $ pnpm exec vitest run --project unit packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts Tests 2 failed | 4 passed (6) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 1890 passed (1890) $ MATRIX_FULL=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 7182 passed (7182) $ pnpm exec vitest run --project unit packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts Tests 6 passed (6) $ pnpm exec vitest run --project unit --project scripts-unit Test Files 639 passed | 4 skipped (643) Tests 27462 passed | 246 expected fail | 135 skipped (27843) ``` --- .../lexical-inline-element-text-fields.md | 9 + .changeset/lexical-y-link-textformat.md | 2 +- .../docs/serialization/serialization.md | 5 +- packages/lexical-y/DECISIONS.md | 4 + .../regressions/WrapperTextFields.test.ts | 440 ++++++++++++++++++ packages/lexical-y/src/internal/Schema.ts | 29 +- .../unit/InlineElementTextFields.test.ts | 178 +++++++ .../lexical/src/nodes/LexicalElementNode.ts | 51 +- 8 files changed, 676 insertions(+), 42 deletions(-) create mode 100644 .changeset/lexical-inline-element-text-fields.md create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts create mode 100644 packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts diff --git a/.changeset/lexical-inline-element-text-fields.md b/.changeset/lexical-inline-element-text-fields.md new file mode 100644 index 00000000000..ea9707725d6 --- /dev/null +++ b/.changeset/lexical-inline-element-text-fields.md @@ -0,0 +1,9 @@ +--- +'lexical': patch +--- + +[fix] Stop serializing the textFormat and textStyle of an inline element whose text sits in another element inside it + +An element persists its textFormat and textStyle only when it has no text to derive them from (#7968), and a reconcile derives them from its first text at any depth. The JSON of an inline element (a link, an autolink, a mark) now follows the same rule: a link whose text sits in a mark no longer writes them, as a link with text of its own already did not. JSON that holds them still reads the same, and a block keeps the direct-children check, so the JSON of a paragraph whose text is all inside links, and of every other node, is unchanged. + +Detect: snapshots of exported JSON that hold `textFormat` or `textStyle` on a `link`, `autolink` or `mark` whose children are all elements. diff --git a/.changeset/lexical-y-link-textformat.md b/.changeset/lexical-y-link-textformat.md index a72884fd112..460d6f99bc3 100644 --- a/.changeset/lexical-y-link-textformat.md +++ b/.changeset/lexical-y-link-textformat.md @@ -4,6 +4,6 @@ [fix] Keep a link a collaborator removes removed when another collaborator formats text covering it at the same time -A format change over a link also sets the link's textFormat (`RangeSelection#formatText`), and the binding stored a link's textFormat and textStyle in the value its text carries: the format change wrote the link's whole value again over its text, and that write brought a concurrently removed link back for everyone. A wrapper's textFormat and textStyle are now stored only while it has no text of its own, as Lexical serializes them; with text, they follow it. +A format change over a link also sets the link's textFormat (`RangeSelection#formatText`), and the binding stored a link's textFormat and textStyle in the value its text carries: the format change wrote the link's whole value again over its text, and that write brought a concurrently removed link back for everyone. A wrapper's textFormat and textStyle are now stored only when Lexical serializes them (`shouldSerializeTextStyles`): while it holds no text, of its own or inside another element in it (a mark in a link). With text, they follow it, and the author's JSON and a fresh reader's agree. Verify: with two clients on a paragraph that holds a link, remove the link on one while the other makes a range covering the whole link code, sync, and check that the link stays removed on both. diff --git a/packages/lexical-website/docs/serialization/serialization.md b/packages/lexical-website/docs/serialization/serialization.md index 6b9c9800901..c6509e04c99 100644 --- a/packages/lexical-website/docs/serialization/serialization.md +++ b/packages/lexical-website/docs/serialization/serialization.md @@ -250,8 +250,9 @@ off the common path, so an element with nothing to persist never calls it. The predicate must be pure and take no arguments: the walk calls it once per property that names it, while generated code hoists one that several properties share and calls it once. This is how `ElementNode` persists `textFormat` and -`textStyle` only for an element with no `TextNode` child, without either -property leaving the direct-field path. +`textStyle` only for an element with no text to derive them from (no +`TextNode` child, and for an inline element such as a link no `TextNode` +anywhere inside it), without either property leaving the direct-field path. `withField(schema, {field, when})` declares the same for a property that is the field in both directions. Either way the gate belongs to the export direction, diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index 5393d6811fd..4b247bf548a 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -64,6 +64,7 @@ issues and docs that carry it. | D30 | **Diffed rewrites snap to whole words**; keystrokes and IME composition keep the exact diff. Scripts without spaces treat each character as a boundary. | Concurrent rewrites otherwise split words. Real word segmentation is a follow-up ([#9467]). | `DECIDED` | 10-26 | [#9400], [#9467] | | D31 | **A list item's nested list gets an item of its own.** `NodeStructureExtension`'s list rule prevents an item holding a nested list next to other content when it is written; the binding does not repair it afterwards. | That shape lost text in outdents, Backspaces and offline merges. Preventing it at write time follows D20. | `DECIDED` | 10-26 | [#9452], [y.md: Limits][limits] | | D32 | **List merging ignores `start`.** | Existing Lexical behaviour, intended. | `DECIDED` | 10-23 | [#9452] | +| D81 | **An inline element's textFormat and textStyle are persisted only while it holds no text at any depth, and the binding stores a wrapper's exactly when Lexical serializes them.** Lexical's `shouldSerializeTextStyles` looks through the elements inside an inline element (a mark in a link), as a reconcile already derives the two from its first text there; a block keeps the direct-children check. Exported JSON changes only for a link, autolink or mark whose children are all elements: those two fields are no longer written (they are still read, into the same state). | Stored while the wrapper holds text, a format change over a link writes the link's value again, and a collaborator's concurrent removal of the link is lost ([#9597]); left unstored while Lexical serializes them, the author's JSON differs from a fresh reader's. One rule, owned by core, keeps both, and the JSON of blocks and of every other node is unchanged (D01). A link with no text at all (only an inline decorator or a line break) still stores them and still comes back ([#9627]). | `DECIDED` | 10-30 | [#9601] | | D70 | **No virtual structure.** The editor does not show repairs that are not stored (padded cells for a short table row, two stored lists drawn as one). The one existing case stays: text left without a line marker shows as a paragraph. | It adds another structure on top of Yjs and the Lexical tree, and the complexity that comes with it. | `DECIDED` | 10-25 | [#9400] | | D72 | **The binding starts at one fixed point, with no further deferral or special-casing.** It starts in `YCollabExtension`'s `afterRegistration`, once every extension has registered and the initial state is applied. An initialization-order bug (a table that exists before its DOM) is fixed in the extension that has it, as [#9501] did. | Deferring the first read further changes when the binding's state exists and still misses code that registers later (legacy table observers, React plugins). | `DECIDED` | 10-28 | [#9501], [#9537] | | D73 | **`@lexical/y` first publishes with its whole launch stack** (the binding, the server API, the playground and per-member sets), with real moves after (D27). The launch stack was chosen on 10-23; moves were moved after it on 10-25. | The first published format should already include what the launch needs, such as per-member sets. | `DECIDED` | 10-25 | [#9400], [#9437], [#9438], [#9439] | @@ -198,3 +199,6 @@ issues and docs that carry it. [#9608]: https://github.com/meta/lexical/issues/9608 [#9609]: https://github.com/meta/lexical/issues/9609 [#9610]: https://github.com/meta/lexical/issues/9610 +[#9597]: https://github.com/meta/lexical/issues/9597 +[#9601]: https://github.com/meta/lexical/pull/9601 +[#9627]: https://github.com/meta/lexical/issues/9627 diff --git a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts new file mode 100644 index 00000000000..39d80994d4e --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts @@ -0,0 +1,440 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +// @vitest-environment node + +// The textFormat and textStyle of an inline element the binding stores as a +// wrapper (link, autolink, mark, overflow), over every nesting of them: the +// catalog's inline elements one to two deep in every order (three with +// MATRIX_FULL=1), around plain, formatted and styled text, two texts, a line +// break, text next to a nested wrapper or an inline decorator, an inline +// decorator alone and a line break alone, with and without text fields of +// their own. +// +// - An author's JSON equals a fresh reader's after a format or style change +// over the paragraph or over the wrapped text alone (#9597's fix stores +// them only where Lexical serializes them; for a wrapper whose text sits +// in another wrapper, a mark in a link, Lexical serializes them only +// since this fix, so the two agree). +// - A wrapper one client removes stays removed while another formats a range +// covering it, in both client id orders (#9597: the format wrote the +// wrapper's value again and brought it back). + +import {buildEditorFromExtensions} from '@lexical/extension'; +import { + $createAutoLinkNode, + $createLinkNode, + $isAutoLinkNode, + $isLinkNode, +} from '@lexical/link'; +import {$createMarkNode, $isMarkNode} from '@lexical/mark'; +import {$createOverflowNode, $isOverflowNode} from '@lexical/overflow'; +import {$patchStyleText} from '@lexical/selection'; +import * as Y from '@y/y'; +import { + $createLineBreakNode, + $createParagraphNode, + $createRangeSelection, + $createTextNode, + $getEditor, + $getRoot, + $isElementNode, + $setSelection, + configExtension, + defineExtension, + type ElementNode, + type LexicalEditor, + type LexicalNode, + type SerializedEditorState, + type TextNode, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; + +import {$createEquationNode} from '../../../../../lexical-playground/src/nodes/EquationNode'; +import {YCollabExtension} from '../../..'; +import {createYDoc, readYState, seedYState} from '../../../headless'; +import {catalogAppExtension, catalogHeadlessConfig} from '../../catalog'; + +const config = catalogHeadlessConfig(); + +type Kind = 'link' | 'autolink' | 'mark' | 'mark2' | 'overflow'; +const KINDS: readonly Kind[] = [ + 'link', + 'autolink', + 'mark', + 'mark2', + 'overflow', +]; + +function $create(kind: Kind): ElementNode { + switch (kind) { + case 'link': + return $createLinkNode('https://wrapped.test'); + case 'autolink': + return $createAutoLinkNode('https://auto.test'); + case 'mark': + return $createMarkNode(['c1']); + case 'mark2': + return $createMarkNode(['c2']); + case 'overflow': + return $createOverflowNode(); + } +} + +/** Whether `node` is the wrapper `kind` makes. */ +function $is(kind: Kind, node: LexicalNode): boolean { + switch (kind) { + case 'link': + return $isLinkNode(node) && !$isAutoLinkNode(node); + case 'autolink': + return $isAutoLinkNode(node); + case 'mark': + case 'mark2': + return ( + $isMarkNode(node) && + node.getIDs().includes(kind === 'mark' ? 'c1' : 'c2') + ); + case 'overflow': + return $isOverflowNode(node); + } +} + +// MATRIX_FULL=1 nests three deep; the default run two (every pair, in both +// orders). +const DEPTH = process.env.MATRIX_FULL === '1' ? 3 : 2; + +/** Every chain of wrappers, outermost first, one to DEPTH deep. */ +function chains(): Kind[][] { + const out: Kind[][] = []; + const grow = (chain: Kind[]) => { + if (chain.length > 0) { + out.push(chain); + } + if (chain.length === DEPTH) { + return; + } + for (const k of KINDS) { + const links = [...chain, k].filter(c => c === 'link' || c === 'autolink'); + if (!chain.includes(k) && links.length <= 1) { + grow([...chain, k]); + } + } + }; + grow([]); + return out; +} + +type Leaf = + | 'text' + | 'bold' + | 'styled' + | 'two-texts' + | 'line-break' + | 'text+wrapper' + | 'text+decorator' + | 'decorator' + | 'no-text'; +const LEAVES: readonly Leaf[] = [ + 'text', + 'bold', + 'styled', + 'two-texts', + 'line-break', + 'text+wrapper', + 'text+decorator', + 'decorator', + 'no-text', +]; + +function $leaf(leaf: Leaf): LexicalNode[] { + switch (leaf) { + case 'text': + return [$createTextNode('wrapped words')]; + case 'bold': + return [$createTextNode('wrapped words').setFormat('bold')]; + case 'styled': + return [$createTextNode('wrapped words').setStyle('color: red')]; + case 'two-texts': + return [ + $createTextNode('wrapped '), + $createTextNode('words').setFormat('italic'), + ]; + case 'line-break': + return [ + $createTextNode('wrapped'), + $createLineBreakNode(), + $createTextNode('words'), + ]; + case 'text+wrapper': + return [ + $createTextNode('wrapped '), + $createMarkNode(['c9']).append($createTextNode('words')), + ]; + case 'text+decorator': + return [$createTextNode('wrapped '), $createEquationNode('x^2', true)]; + case 'decorator': + return [$createEquationNode('x^2', true)]; + case 'no-text': + return [$createLineBreakNode()]; + } +} + +type Fields = 'none' | 'own'; + +function $build(chain: readonly Kind[], leaf: Leaf, fields: Fields): void { + let inner: LexicalNode[] = $leaf(leaf); + for (let i = chain.length - 1; i >= 0; i--) { + const wrapper = $create(chain[i]).append(...inner); + if (fields === 'own') { + wrapper.setTextFormat(2).setTextStyle('font-size: 12px'); + } + inner = [wrapper]; + } + $getRoot() + .clear() + .append( + $createParagraphNode().append( + $createTextNode('before '), + ...inner, + $createTextNode(' after'), + ), + ); +} + +function client(seed: Uint8Array, id: number) { + const doc = createYDoc(seed); + doc.clientID = id; + const editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + dependencies: [ + catalogAppExtension(), + configExtension(YCollabExtension, {doc, schemaVersion: 1}), + ], + name: `wrapper-text-fields-${id}`, + }), + ); + return {doc, editor}; +} + +function sync(a: {doc: Y.Doc}, b: {doc: Y.Doc}): void { + const ua = Y.encodeStateAsUpdate(a.doc); + const ub = Y.encodeStateAsUpdate(b.doc); + Y.applyUpdate(a.doc, ub); + Y.applyUpdate(b.doc, ua); +} + +function $json(): SerializedEditorState { + return $getEditor().getEditorState().toJSON(); +} + +function json(editor: LexicalEditor): SerializedEditorState { + return editor.read($json); +} + +function fresh(doc: Y.Doc): SerializedEditorState { + return readYState(Y.encodeStateAsUpdate(doc), config, $json); +} + +/** The text nodes inside the outermost wrapper. */ +function $wrappedTexts(): TextNode[] { + const paragraph = $getRoot().getFirstChildOrThrow() as ElementNode; + const out: TextNode[] = []; + for (const child of paragraph.getChildren()) { + if ($isElementNode(child)) { + out.push(...child.getAllTextNodes()); + } + } + return out; +} + +function $selectAll(): ReturnType { + const texts = $getRoot().getAllTextNodes(); + const first = texts[0]; + const last = texts[texts.length - 1]; + const selection = $createRangeSelection(); + selection.anchor.set(first.getKey(), 0, 'text'); + selection.focus.set(last.getKey(), last.getTextContentSize(), 'text'); + $setSelection(selection); + return selection; +} + +function $selectWrapped(): ReturnType | null { + const texts = $wrappedTexts(); + if (texts.length === 0) { + return null; + } + const first = texts[0]; + const last = texts[texts.length - 1]; + const selection = $createRangeSelection(); + selection.anchor.set(first.getKey(), 0, 'text'); + selection.focus.set(last.getKey(), last.getTextContentSize(), 'text'); + $setSelection(selection); + return selection; +} + +type Op = 'format-all' | 'format-wrapped' | 'style-all' | 'style-wrapped'; +const OPS: readonly Op[] = [ + 'format-all', + 'format-wrapped', + 'style-all', + 'style-wrapped', +]; + +/** Runs `op`; false when it has nothing to act on. */ +function $op(op: Op): boolean { + const selection = + op === 'format-all' || op === 'style-all' ? $selectAll() : $selectWrapped(); + if (selection === null) { + return false; + } + if (op === 'format-all' || op === 'format-wrapped') { + selection.formatText('code'); + } else { + $patchStyleText(selection, {color: 'blue'}); + } + return true; +} + +/** Moves the children of the wrapper `kind` out of it and removes it. */ +function $unwrap(kind: Kind): void { + const visit = (node: ElementNode) => { + for (const child of node.getChildren()) { + if ($isElementNode(child)) { + if ($is(kind, child)) { + for (const grandchild of child.getChildren()) { + child.insertBefore(grandchild); + } + child.remove(); + } else { + visit(child); + } + } + } + }; + visit($getRoot()); +} + +function count(editor: LexicalEditor, kind: Kind): number { + return editor.read(() => { + let n = 0; + const visit = (node: ElementNode) => { + for (const child of node.getChildren()) { + if ($isElementNode(child)) { + if ($is(kind, child)) { + n++; + } + visit(child); + } + } + }; + visit($getRoot()); + return n; + }); +} + +const CHAINS = chains(); + +type Shape = {chain: Kind[]; leaf: Leaf; fields: Fields; label: string}; +const SHAPES: Shape[] = []; +for (const chain of CHAINS) { + for (const leaf of LEAVES) { + for (const fields of ['none', 'own'] as const) { + SHAPES.push({ + chain, + fields, + label: `${chain.join('>')}>${leaf}${fields === 'own' ? ' own' : ''}`, + leaf, + }); + } + } +} + +describe('an author and a fresh reader have the same JSON', () => { + test.each(SHAPES)('$label', async ({chain, leaf, fields}) => { + const seed = seedYState(config, () => $build(chain, leaf, fields)); + const author = client(seed, 101); + const peer = client(seed, 102); + await Promise.resolve(); + expect(json(author.editor)).toEqual(fresh(author.doc)); + for (const op of OPS) { + let acted = false; + author.editor.update( + () => { + acted = $op(op); + }, + {discrete: true}, + ); + if (!acted) { + continue; + } + sync(author, peer); + await Promise.resolve(); + const expected = fresh(author.doc); + expect(json(author.editor), `${op}: author`).toEqual(expected); + expect(json(peer.editor), `${op}: peer`).toEqual(expected); + } + }); +}); + +// Removal races: every wrapper of a chain, removed by one client while the +// other formats a range covering it, in both id orders. +type Race = Shape & {removed: Kind; ids: [number, number]}; +const RACES: Race[] = []; +for (const shape of SHAPES) { + for (const removed of shape.chain) { + for (const ids of [ + [1, 2], + [2, 1], + ] as [number, number][]) { + RACES.push({...shape, ids, removed}); + } + } +} + +/** + * A link that holds no text (only an inline decorator or a line break): + * Lexical serializes its textFormat and textStyle, so the binding stores + * them in the link's value, and the format change still writes that value + * and brings the removed link back (#9627). + */ +function knownTextlessLink({removed, leaf}: Race): boolean { + return removed === 'link' && (leaf === 'decorator' || leaf === 'no-text'); +} + +describe('a wrapper one client removes stays removed while another formats over it', () => { + for (const [known, races] of [ + [false, RACES.filter(r => !knownTextlessLink(r))], + [true, RACES.filter(knownTextlessLink)], + ] as const) { + (known ? test.fails : test).each(races)( + '$label: remove $removed, ids $ids.0,$ids.1', + async ({chain, leaf, fields, removed, ids}) => { + const seed = seedYState(config, () => $build(chain, leaf, fields)); + const a = client(seed, ids[0]); + const b = client(seed, ids[1]); + await Promise.resolve(); + a.editor.update(() => $unwrap(removed), {discrete: true}); + expect(count(a.editor, removed)).toBe(0); + b.editor.update( + () => { + $selectAll().formatText('code'); + }, + {discrete: true}, + ); + sync(a, b); + await Promise.resolve(); + const expected = fresh(a.doc); + expect(count(a.editor, removed), 'a').toBe(0); + expect(count(b.editor, removed), 'b').toBe(0); + expect(json(a.editor), 'a = fresh').toEqual(expected); + expect(json(b.editor), 'b = fresh').toEqual(expected); + }, + ); + } +}); diff --git a/packages/lexical-y/src/internal/Schema.ts b/packages/lexical-y/src/internal/Schema.ts index 224ce16f98e..336699bc75d 100644 --- a/packages/lexical-y/src/internal/Schema.ts +++ b/packages/lexical-y/src/internal/Schema.ts @@ -1995,13 +1995,14 @@ export function $wrapperAttrs( putValue(own, f, json[f], `${node.getType()}.${f}`); } $stateAttrs(node.getLatest(), own); - // Only while the wrapper holds no text, directly or through inline - // elements inside it (a mark in a link): with text, they follow that text - // (a format of it sets them), and a write of them alone would write the - // whole value again over the wrapper's text, which a collaborator's - // concurrent removal of the wrapper does not delete (the removed link - // came back). - if (!$holdsText(node.getLatest())) { + // Only when Lexical serializes them (`shouldSerializeTextStyles`: the + // wrapper holds no text anywhere inside it, and a mark in a link holds + // text), so that the author and a fresh reader have the same JSON. With + // text, they follow that text (a format of it sets them), and a write of + // them alone would write the whole value again over the wrapper's text, + // which a collaborator's concurrent removal of the wrapper does not delete + // (the removed link came back). + if (node.getLatest().shouldSerializeTextStyles()) { $putTextFields(node.getLatest(), own); } for (const k in own) { @@ -2147,20 +2148,6 @@ export function $lineAttrs( return attrs; } -/** Whether `node` holds a text node, directly or inside its descendants. */ -function $holdsText(node: ElementNode): boolean { - for (let child = node.getFirstChild(); child !== null; ) { - if ($isTextNode(child)) { - return true; - } - if ($isElementNode(child) && $holdsText(child)) { - return true; - } - child = child.getNextSibling(); - } - return false; -} - /** * An element's textFormat/textStyle, which Lexical serializes only while * the element has no text: every peer must hold the same values, so groups diff --git a/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts b/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts new file mode 100644 index 00000000000..f9308a90209 --- /dev/null +++ b/packages/lexical/src/__tests__/unit/InlineElementTextFields.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. + * + */ + +import {buildEditorFromExtensions} from '@lexical/extension'; +import { + $create, + $createLineBreakNode, + $createParagraphNode, + $createTextNode, + $getRoot, + $parseSerializedNode, + defineExtension, + ElementNode, + type LexicalNode, + type SerializedLexicalNode, +} from 'lexical'; +import {describe, expect, test} from 'vitest'; + +// An element's textFormat and textStyle are persisted only when it has no +// text a reconcile would derive them from (#7968). A reconcile derives them +// from the element's first text at any depth; for an inline element (a link, +// a mark) the JSON now follows that rule too, so a link whose text sits in a +// mark no longer persists them. A block keeps the direct-children check, so +// the JSON of a paragraph whose text is all inside links is unchanged. + +class InlineWrapperNode extends ElementNode { + $config() { + return this.config('test-inline-wrapper', {extends: ElementNode}); + } + isInline(): boolean { + return true; + } +} + +function $wrapper(...children: LexicalNode[]): InlineWrapperNode { + return $create(InlineWrapperNode) + .setTextFormat(1) + .setTextStyle('color: red') + .append(...children); +} + +function withEditor(fn: () => void): void { + using editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + name: '[inline-element-text-fields]', + nodes: [InlineWrapperNode], + }), + ); + editor.update(fn, {discrete: true}); +} + +type Fields = {textFormat?: number; textStyle?: string}; + +function fields(node: ElementNode): Fields { + const json = node.exportJSON(); + return {textFormat: json.textFormat, textStyle: json.textStyle}; +} + +/** + * What ElementNode persists for a block (ParagraphNode's exportJSON then + * backfills them from its first text child for #7971). + */ +function serialized(node: ElementNode): Fields { + return { + textFormat: node.getSerializedTextFormat(), + textStyle: node.getSerializedTextStyle(), + }; +} + +const PERSISTED: Fields = {textFormat: 1, textStyle: 'color: red'}; +const OMITTED: Fields = {textFormat: undefined, textStyle: undefined}; + +describe('textFormat and textStyle of an inline element', () => { + test('are omitted when its text sits in an inline element inside it', () => { + withEditor(() => { + const inner = $wrapper($createTextNode('text')); + const outer = $wrapper(inner); + const outermost = $wrapper($wrapper($wrapper($createTextNode('deep')))); + $getRoot().append($createParagraphNode().append(outer, outermost)); + expect(fields(outer)).toEqual(OMITTED); + expect(fields(inner)).toEqual(OMITTED); + expect(fields(outermost)).toEqual(OMITTED); + }); + }); + + test('are omitted when it holds text of its own, as before', () => { + withEditor(() => { + const wrapper = $wrapper($createTextNode('text')); + $getRoot().append($createParagraphNode().append(wrapper)); + expect(fields(wrapper)).toEqual(OMITTED); + }); + }); + + test('are persisted when it holds no text at any depth, as before', () => { + withEditor(() => { + const empty = $wrapper(); + const lineBreak = $wrapper($createLineBreakNode()); + const nested = $wrapper($wrapper($createLineBreakNode())); + $getRoot().append( + $createParagraphNode().append(empty, lineBreak, nested), + ); + expect(fields(empty)).toEqual(PERSISTED); + expect(fields(lineBreak)).toEqual(PERSISTED); + expect(fields(nested)).toEqual(PERSISTED); + }); + }); + + test('a stored value is read and no longer written once it has text inside', () => { + withEditor(() => { + const stored = { + children: [ + { + children: [ + { + detail: 0, + format: 1, + mode: 'normal', + style: 'color: red', + text: 'text', + type: 'text', + version: 1, + }, + ], + direction: null, + format: '', + indent: 0, + textFormat: 1, + textStyle: 'color: red', + type: 'test-inline-wrapper', + version: 1, + }, + ], + direction: null, + format: '', + indent: 0, + textFormat: 1, + textStyle: 'color: red', + type: 'test-inline-wrapper', + version: 1, + } as SerializedLexicalNode; + const outer = $parseSerializedNode(stored) as InlineWrapperNode; + expect(outer.getTextFormat()).toBe(1); + expect(outer.getTextStyle()).toBe('color: red'); + $getRoot().append($createParagraphNode().append(outer)); + expect(fields(outer)).toEqual(OMITTED); + }); + }); +}); + +describe('textFormat and textStyle of a block', () => { + test('are persisted when its text is all inside inline elements, as before', () => { + withEditor(() => { + const paragraph = $createParagraphNode() + .setTextFormat(1) + .setTextStyle('color: red') + .append($wrapper($createTextNode('text'))); + $getRoot().append(paragraph); + expect(serialized(paragraph)).toEqual(PERSISTED); + }); + }); + + test('are omitted when it holds text of its own, as before', () => { + withEditor(() => { + const paragraph = $createParagraphNode() + .setTextFormat(1) + .setTextStyle('color: red') + .append($createTextNode('text'), $wrapper($createTextNode('more'))); + $getRoot().append(paragraph); + expect(serialized(paragraph)).toEqual(OMITTED); + }); + }); +}); diff --git a/packages/lexical/src/nodes/LexicalElementNode.ts b/packages/lexical/src/nodes/LexicalElementNode.ts index ffca86f6a87..99205809887 100644 --- a/packages/lexical/src/nodes/LexicalElementNode.ts +++ b/packages/lexical/src/nodes/LexicalElementNode.ts @@ -167,8 +167,8 @@ const elementNodeSchema = nodeSchema()({ indent: withField(numberValue(0, {integer: true, min: 0}), { field: '__indent', }), - // Persisted only for an element with no TextNode child (see #7968), which - // `shouldSerializeTextStyles` decides. Declaring that as the field it is + // Persisted only for an element with no text to derive them from (see + // #7968), which `shouldSerializeTextStyles` decides. Declaring that as the field it is // plus the predicate that gates it — rather than reading through // `getSerializedTextFormat`/`getSerializedTextStyle` — keeps both on the // direct-field path, and lets generated code call the shared predicate once @@ -896,8 +896,12 @@ export class ElementNode } // JSON serialization /** - * Whether `textFormat`/`textStyle` are persisted at all: only when there are - * no TextNode children from which they would be set on reconcile (#7968). + * Whether `textFormat`/`textStyle` are persisted at all: only when there is + * no text from which they would be set on reconcile (#7968). That is a + * TextNode child, and for an inline element (a link, a mark) a TextNode + * anywhere inside it (a mark in a link), whose first text a reconcile takes + * them from as well. A block keeps the direct-children check, so its JSON + * is unchanged. * * @internal */ @@ -905,20 +909,7 @@ export class ElementNode if ($isRootOrShadowRoot(this)) { return false; } - // Walked rather than materialized with getChildren(), and it exits at the - // first TextNode. Both callers below test their own value first, so an - // element with a default textFormat/textStyle — every element that has - // many non-text children in practice — never reaches this at all. - for ( - let child = this.getFirstChild(); - child !== null; - child = child.getNextSibling() - ) { - if ($isTextNode(child)) { - return false; - } - } - return true; + return !$hasTextChild(this, this.isInline()); } /** @internal Serialized `textFormat`, or undefined to omit it. */ @@ -1088,6 +1079,30 @@ export function $isElementNode( return node instanceof ElementNode; } +/** + * Whether `element` has a TextNode child or, when `deep`, a TextNode at any + * depth inside it. Walked rather than materialized with getChildren(), and it + * exits at the first TextNode. Both callers of shouldSerializeTextStyles test + * their own value first, so an element with a default textFormat/textStyle + * (every element that has many non-text children in practice) never reaches + * this at all. + */ +function $hasTextChild(element: ElementNode, deep: boolean): boolean { + for ( + let child = element.getFirstChild(); + child !== null; + child = child.getNextSibling() + ) { + if ( + $isTextNode(child) || + (deep && $isElementNode(child) && $hasTextChild(child, true)) + ) { + return true; + } + } + return false; +} + function isPointRemoved( point: PointType, nodesToRemoveKeySet: Set, From ecf1f41d52a40291c9af0577272af56115f2f4b7 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 16:48:33 -0700 Subject: [PATCH 04/18] [lexical][lexical-y] Bug Fix: Keep serializing inline elements nested inside each other linear MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description The previous commit made `shouldSerializeTextStyles` look for text anywhere inside an inline element. Each inline element asked that of its whole subtree, so serializing N inline elements nested inside each other, each with a textFormat or textStyle, walked O(N^2) nodes (review: 33,155 `getFirstChild` calls at depth 256, where main makes 515). For a frozen EditorState, the answer for every element the walk visits is now kept (keyed by the state), so each element is walked at most once per state. An update's pending state is not cached, since it can still change. The binding asks only when the wrapper has a textFormat or textStyle to store, as Lexical's serializer does. `WrapperTextFields.test.ts` also records #9629 (a link one client removes comes back when another sets NodeState on it, on main too) as `test.fails`, for a link, a link holding a mark and a link in a mark, both client id orders. ## Test plan `InlineElementTextFields.test.ts`: a new test counts `getFirstChild` calls while serializing 64, 128 and 256 nested inline elements, each with text fields, and requires each doubling to stay under 3x. ### Before ``` $ pnpm exec vitest run --project unit packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts × stays linear in the depth when each carries text fields AssertionError: expected 3.9063809967396366 to be less than 3 Tests 1 failed | 6 passed (7) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Test Files 2 passed (2) Tests 1841 passed | 62 expected fail (1903) $ MATRIX_FULL=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 6982 passed | 206 expected fail (7188) ``` `getFirstChild` calls while serializing nested marks with text fields, at depth 32 / 64 / 128 / 256: 67 / 131 / 259 / 515, as on main. --- .../regressions/WrapperTextFields.test.ts | 57 +++++++++++++++++ packages/lexical-y/src/internal/Schema.ts | 8 ++- .../unit/InlineElementTextFields.test.ts | 47 +++++++++++++- .../lexical/src/nodes/LexicalElementNode.ts | 63 ++++++++++++++++--- 4 files changed, 164 insertions(+), 11 deletions(-) diff --git a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts index 39d80994d4e..c0fe3c04991 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts @@ -45,7 +45,9 @@ import { $getRoot, $isElementNode, $setSelection, + $setState, configExtension, + createState, defineExtension, type ElementNode, type LexicalEditor, @@ -438,3 +440,58 @@ describe('a wrapper one client removes stays removed while another formats over ); } }); + +const note = createState('wrapper-text-fields-note', { + parse: (v: unknown) => (typeof v === 'string' ? v : ''), +}); + +/** The first wrapper `kind` makes, at any depth. */ +function $find(kind: Kind): ElementNode { + let found: ElementNode | null = null; + const visit = (node: ElementNode) => { + for (const child of node.getChildren()) { + if (found === null && $isElementNode(child)) { + if ($is(kind, child)) { + found = child; + } else { + visit(child); + } + } + } + }; + visit($getRoot()); + if (found === null) { + throw new Error(`no ${kind}`); + } + return found; +} + +// A change to the link itself (here NodeState) writes its value again, and +// that write brings back a link a collaborator removed at the same time +// (#9629): not a text format or style, so not covered by #9597's fix. +describe('a link one client removes while another sets NodeState on it', () => { + test.fails.each( + ([['link'], ['link', 'mark'], ['mark', 'link']] as Kind[][]).flatMap( + chain => + ( + [ + [1, 2], + [2, 1], + ] as [number, number][] + ).map(ids => ({chain, ids, label: chain.join('>')})), + ), + )('$label: ids $ids.0,$ids.1 (#9629)', async ({chain, ids}) => { + const seed = seedYState(config, () => $build(chain, 'text', 'none')); + const a = client(seed, ids[0]); + const b = client(seed, ids[1]); + await Promise.resolve(); + a.editor.update(() => $unwrap('link'), {discrete: true}); + b.editor.update(() => void $setState($find('link'), note, 'checked'), { + discrete: true, + }); + sync(a, b); + await Promise.resolve(); + expect(count(a.editor, 'link'), 'a').toBe(0); + expect(count(b.editor, 'link'), 'b').toBe(0); + }); +}); diff --git a/packages/lexical-y/src/internal/Schema.ts b/packages/lexical-y/src/internal/Schema.ts index 336699bc75d..e84509bfd96 100644 --- a/packages/lexical-y/src/internal/Schema.ts +++ b/packages/lexical-y/src/internal/Schema.ts @@ -2002,8 +2002,12 @@ export function $wrapperAttrs( // them alone would write the whole value again over the wrapper's text, // which a collaborator's concurrent removal of the wrapper does not delete // (the removed link came back). - if (node.getLatest().shouldSerializeTextStyles()) { - $putTextFields(node.getLatest(), own); + const latest = node.getLatest(); + if ( + (latest.getTextFormat() !== 0 || latest.getTextStyle() !== '') && + latest.shouldSerializeTextStyles() + ) { + $putTextFields(latest, own); } for (const k in own) { const v = own[k]; diff --git a/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts b/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts index f9308a90209..857f7c54830 100644 --- a/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts +++ b/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts @@ -19,7 +19,7 @@ import { type LexicalNode, type SerializedLexicalNode, } from 'lexical'; -import {describe, expect, test} from 'vitest'; +import {describe, expect, test, vi} from 'vitest'; // An element's textFormat and textStyle are persisted only when it has no // text a reconcile would derive them from (#7968). A reconcile derives them @@ -176,3 +176,48 @@ describe('textFormat and textStyle of a block', () => { }); }); }); + +describe('serializing inline elements nested inside each other', () => { + test('stays linear in the depth when each carries text fields', () => { + const calls: number[] = []; + for (const depth of [64, 128, 256]) { + using editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + name: '[inline-element-text-fields-depth]', + nodes: [InlineWrapperNode], + }), + ); + editor.update( + () => { + let node: LexicalNode = $createTextNode('leaf'); + for (let i = 0; i < depth; i++) { + node = $wrapper(node); + } + $getRoot().append($createParagraphNode().append(node)); + }, + {discrete: true}, + ); + const getFirstChild = ElementNode.prototype.getFirstChild; + let count = 0; + const spy = vi + .spyOn(ElementNode.prototype, 'getFirstChild') + .mockImplementation(function (this: ElementNode) { + count++; + return getFirstChild.call(this); + }); + try { + expect(JSON.stringify(editor.getEditorState().toJSON())).toContain( + 'leaf', + ); + } finally { + spy.mockRestore(); + } + calls.push(count); + } + // Doubling the depth doubles the walk (about 2x), where a walk of each + // element's whole subtree would quadruple it. + expect(calls[1] / calls[0]).toBeLessThan(3); + expect(calls[2] / calls[1]).toBeLessThan(3); + }); +}); diff --git a/packages/lexical/src/nodes/LexicalElementNode.ts b/packages/lexical/src/nodes/LexicalElementNode.ts index 99205809887..e5555d2bb82 100644 --- a/packages/lexical/src/nodes/LexicalElementNode.ts +++ b/packages/lexical/src/nodes/LexicalElementNode.ts @@ -6,6 +6,7 @@ * */ +import type {EditorState} from '../LexicalEditorState'; import type { KlassConstructor, LexicalEditor, @@ -70,7 +71,11 @@ import { $getSlotsTextContentSize, } from '../LexicalSlot'; import {INTERNAL_isNodeStructureEnforced} from '../LexicalStructure'; -import {errorOnReadOnly, getActiveEditor} from '../LexicalUpdates'; +import { + errorOnReadOnly, + getActiveEditor, + internalGetActiveEditorState, +} from '../LexicalUpdates'; import { $getDOMSlot, $getNodeByKey, @@ -1079,6 +1084,14 @@ export function $isElementNode( return node instanceof ElementNode; } +/** + * For each element of a frozen EditorState, whether it holds a TextNode at + * any depth. An inline element asks that of its whole subtree, so without + * it serializing inline elements nested inside each other, each with a + * textFormat or textStyle, would walk the same subtree once per level. + */ +const textInsideCache = new WeakMap>(); + /** * Whether `element` has a TextNode child or, when `deep`, a TextNode at any * depth inside it. Walked rather than materialized with getChildren(), and it @@ -1088,19 +1101,53 @@ export function $isElementNode( * this at all. */ function $hasTextChild(element: ElementNode, deep: boolean): boolean { + if (!deep) { + for ( + let child = element.getFirstChild(); + child !== null; + child = child.getNextSibling() + ) { + if ($isTextNode(child)) { + return true; + } + } + return false; + } + const editorState = internalGetActiveEditorState(); + let cache: Map | null = null; + if (editorState !== null && editorState._readOnly) { + cache = textInsideCache.get(editorState) || null; + if (cache === null) { + cache = new Map(); + textInsideCache.set(editorState, cache); + } + } + return $hasTextInside(element, cache); +} + +function $hasTextInside( + element: ElementNode, + cache: Map | null, +): boolean { + const key = element.getKey(); + const cached = cache !== null ? cache.get(key) : undefined; + if (cached !== undefined) { + return cached; + } + let found = false; for ( let child = element.getFirstChild(); - child !== null; + child !== null && !found; child = child.getNextSibling() ) { - if ( + found = $isTextNode(child) || - (deep && $isElementNode(child) && $hasTextChild(child, true)) - ) { - return true; - } + ($isElementNode(child) && $hasTextInside(child, cache)); } - return false; + if (cache !== null) { + cache.set(key, found); + } + return found; } function isPointRemoved( From 37ed6f4e212ccbbe243f6c0902410489438aa39b Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 16:58:21 -0700 Subject: [PATCH 05/18] [lexical] Bug Fix: Keep serializing nested inline elements linear inside an update too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description The previous commit cached, per frozen EditorState, whether each element holds text, so serializing inline elements nested inside each other stays linear. A copy serializes inside an update (`$getLexicalContent`, `$generateJSONFromSelectedNodes`), on the pending state, which was not cached. There it was still quadratic (review: 32,896 `getFirstChild` calls at depth 256 for a select-all copy of nested marks with text fields, where main makes 256). The cache now covers every state, pending ones included. `getWritable` drops the active state's entry, as it already drops the selection's cached nodes, since a change to any node can change what its ancestors hold. ## Test plan `InlineElementTextFields.test.ts` counts `getFirstChild` calls while serializing 64, 128 and 256 nested inline elements with text fields, in a committed state and inside an update, and requires each doubling to stay under 3x. A new test checks that a change inside an update is seen by the next serialization in it. ### Before ``` $ pnpm exec vitest run --project unit packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts × stays linear in the depth when each carries text fields, in an update AssertionError: expected 3.9063809967396366 to be less than 3 Tests 1 failed | 8 passed (9) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts Tests 9 passed (9) ``` `getFirstChild` calls inside `shouldSerializeTextStyles` for a select-all `$generateJSONFromSelectedNodes` inside an update, nested marks with text fields at depth 32 / 64 / 128 / 256: 32 / 64 / 128 / 256, as on main. --- packages/lexical/src/LexicalNode.ts | 2 + packages/lexical/src/LexicalUtils.ts | 26 ++++ .../unit/InlineElementTextFields.test.ts | 122 ++++++++++++------ .../lexical/src/nodes/LexicalElementNode.ts | 54 +++----- 4 files changed, 127 insertions(+), 77 deletions(-) diff --git a/packages/lexical/src/LexicalNode.ts b/packages/lexical/src/LexicalNode.ts index be001c0d88b..343519d011c 100644 --- a/packages/lexical/src/LexicalNode.ts +++ b/packages/lexical/src/LexicalNode.ts @@ -87,6 +87,7 @@ import { getStaticNodeConfig, getSuperclassOf, internalMarkNodeAsDirty, + internalResetTextInsideCache, } from './LexicalUtils'; const __DEV__ = process.env.NODE_ENV !== 'production'; @@ -1656,6 +1657,7 @@ export class LexicalNode { if (selection !== null) { selection.setCachedNodes(null); } + internalResetTextInsideCache(editorState); if (writableNode !== undefined) { // Transforms clear the dirty node set on each iteration to keep track on newly dirty nodes internalMarkNodeAsDirty(writableNode); diff --git a/packages/lexical/src/LexicalUtils.ts b/packages/lexical/src/LexicalUtils.ts index 8a003db32f4..4869c3db283 100644 --- a/packages/lexical/src/LexicalUtils.ts +++ b/packages/lexical/src/LexicalUtils.ts @@ -3129,6 +3129,32 @@ export function $isKeyInSelectionNodes( return nodes.some(node => node.__key === key); } +/** + * For each element of an EditorState, whether it holds a TextNode at any + * depth, as ElementNode#shouldSerializeTextStyles asks for an inline element. + * Without it, serializing inline elements nested inside each other, each with + * a textFormat or textStyle, walks the same subtree once per level. A pending + * state's entry is dropped by getWritable, since a change to any node can + * change what its ancestors hold. + */ +const textInsideCache = new WeakMap>(); + +/** @internal */ +export function $getTextInsideCache(): Map { + const editorState = getActiveEditorState(); + let cache = textInsideCache.get(editorState); + if (cache === undefined) { + cache = new Map(); + textInsideCache.set(editorState, cache); + } + return cache; +} + +/** @internal */ +export function internalResetTextInsideCache(editorState: EditorState): void { + textInsideCache.delete(editorState); +} + /** @internal */ export type TypeToNodeMap = Map; /** diff --git a/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts b/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts index 857f7c54830..e9a76beaa69 100644 --- a/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts +++ b/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts @@ -177,47 +177,91 @@ describe('textFormat and textStyle of a block', () => { }); }); +/** getFirstChild calls while `fn` runs. */ +function countGetFirstChild(fn: () => void): number { + const getFirstChild = ElementNode.prototype.getFirstChild; + let count = 0; + const spy = vi + .spyOn(ElementNode.prototype, 'getFirstChild') + .mockImplementation(function (this: ElementNode) { + count++; + return getFirstChild.call(this); + }); + try { + fn(); + } finally { + spy.mockRestore(); + } + return count; +} + +/** Each node's exportJSON, as the clipboard exports a selection. */ +function $exportEach(node: LexicalNode): unknown { + const json = node.exportJSON() as {children?: unknown[]}; + if (node instanceof ElementNode) { + json.children = node.getChildren().map($exportEach); + } + return json; +} + describe('serializing inline elements nested inside each other', () => { - test('stays linear in the depth when each carries text fields', () => { - const calls: number[] = []; - for (const depth of [64, 128, 256]) { - using editor = buildEditorFromExtensions( - defineExtension({ - $initialEditorState: null, - name: '[inline-element-text-fields-depth]', - nodes: [InlineWrapperNode], - }), - ); - editor.update( - () => { - let node: LexicalNode = $createTextNode('leaf'); - for (let i = 0; i < depth; i++) { - node = $wrapper(node); - } - $getRoot().append($createParagraphNode().append(node)); - }, - {discrete: true}, - ); - const getFirstChild = ElementNode.prototype.getFirstChild; - let count = 0; - const spy = vi - .spyOn(ElementNode.prototype, 'getFirstChild') - .mockImplementation(function (this: ElementNode) { - count++; - return getFirstChild.call(this); - }); - try { - expect(JSON.stringify(editor.getEditorState().toJSON())).toContain( - 'leaf', + for (const where of ['a committed state', 'an update'] as const) { + test(`stays linear in the depth when each carries text fields, in ${where}`, () => { + const calls: number[] = []; + for (const depth of [64, 128, 256]) { + using editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + name: '[inline-element-text-fields-depth]', + nodes: [InlineWrapperNode], + }), + ); + editor.update( + () => { + let node: LexicalNode = $createTextNode('leaf'); + for (let i = 0; i < depth; i++) { + node = $wrapper(node); + } + $getRoot().append($createParagraphNode().append(node)); + }, + {discrete: true}, ); - } finally { - spy.mockRestore(); + let json = ''; + calls.push( + countGetFirstChild(() => { + if (where === 'a committed state') { + json = JSON.stringify(editor.getEditorState().toJSON()); + } else { + editor.update( + () => { + json = JSON.stringify($exportEach($getRoot())); + }, + {discrete: true}, + ); + } + }), + ); + expect(json).toContain('leaf'); } - calls.push(count); - } - // Doubling the depth doubles the walk (about 2x), where a walk of each - // element's whole subtree would quadruple it. - expect(calls[1] / calls[0]).toBeLessThan(3); - expect(calls[2] / calls[1]).toBeLessThan(3); + // Doubling the depth doubles the walk (about 2x), where a walk of + // each element's whole subtree would quadruple it. + expect(calls[1] / calls[0]).toBeLessThan(3); + expect(calls[2] / calls[1]).toBeLessThan(3); + }); + } + + test('a change in an update is seen by the next serialization in it', () => { + withEditor(() => { + const text = $createTextNode('text'); + const inner = $wrapper(text); + const outer = $wrapper(inner); + $getRoot().append($createParagraphNode().append(outer)); + expect(fields(outer)).toEqual(OMITTED); + text.remove(); + inner.append($createLineBreakNode()); + expect(fields(outer)).toEqual(PERSISTED); + inner.append($createTextNode('again')); + expect(fields(outer)).toEqual(OMITTED); + }); }); }); diff --git a/packages/lexical/src/nodes/LexicalElementNode.ts b/packages/lexical/src/nodes/LexicalElementNode.ts index e5555d2bb82..9fdc08c0a8c 100644 --- a/packages/lexical/src/nodes/LexicalElementNode.ts +++ b/packages/lexical/src/nodes/LexicalElementNode.ts @@ -6,7 +6,6 @@ * */ -import type {EditorState} from '../LexicalEditorState'; import type { KlassConstructor, LexicalEditor, @@ -71,14 +70,11 @@ import { $getSlotsTextContentSize, } from '../LexicalSlot'; import {INTERNAL_isNodeStructureEnforced} from '../LexicalStructure'; -import { - errorOnReadOnly, - getActiveEditor, - internalGetActiveEditorState, -} from '../LexicalUpdates'; +import {errorOnReadOnly, getActiveEditor} from '../LexicalUpdates'; import { $getDOMSlot, $getNodeByKey, + $getTextInsideCache, $isRootOrShadowRoot, getNodeKeyFromDOMNode, isDOMUnmanaged, @@ -1084,14 +1080,6 @@ export function $isElementNode( return node instanceof ElementNode; } -/** - * For each element of a frozen EditorState, whether it holds a TextNode at - * any depth. An inline element asks that of its whole subtree, so without - * it serializing inline elements nested inside each other, each with a - * textFormat or textStyle, would walk the same subtree once per level. - */ -const textInsideCache = new WeakMap>(); - /** * Whether `element` has a TextNode child or, when `deep`, a TextNode at any * depth inside it. Walked rather than materialized with getChildren(), and it @@ -1101,36 +1089,28 @@ const textInsideCache = new WeakMap>(); * this at all. */ function $hasTextChild(element: ElementNode, deep: boolean): boolean { - if (!deep) { - for ( - let child = element.getFirstChild(); - child !== null; - child = child.getNextSibling() - ) { - if ($isTextNode(child)) { - return true; - } - } - return false; + if (deep) { + return $hasTextInside(element, $getTextInsideCache()); } - const editorState = internalGetActiveEditorState(); - let cache: Map | null = null; - if (editorState !== null && editorState._readOnly) { - cache = textInsideCache.get(editorState) || null; - if (cache === null) { - cache = new Map(); - textInsideCache.set(editorState, cache); + for ( + let child = element.getFirstChild(); + child !== null; + child = child.getNextSibling() + ) { + if ($isTextNode(child)) { + return true; } } - return $hasTextInside(element, cache); + return false; } +/** $hasTextChild's deep walk; every element it visits is cached. */ function $hasTextInside( element: ElementNode, - cache: Map | null, + cache: Map, ): boolean { const key = element.getKey(); - const cached = cache !== null ? cache.get(key) : undefined; + const cached = cache.get(key); if (cached !== undefined) { return cached; } @@ -1144,9 +1124,7 @@ function $hasTextInside( $isTextNode(child) || ($isElementNode(child) && $hasTextInside(child, cache)); } - if (cache !== null) { - cache.set(key, found); - } + cache.set(key, found); return found; } From 432157a796a45a8369a7db510ee8bedd6646dc72 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 17:11:21 -0700 Subject: [PATCH 06/18] [lexical][lexical-y] Bug Fix: Cache whether an element holds text only for states that cannot change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description The previous commit cached, for pending states too, whether each element holds text, and dropped a pending state's entries in `getWritable`. A read made while a node is being cloned (an `afterCloneFrom` that calls `exportJSON` on the node it is made from, which is allowed) ran after that drop and before the change, and refilled the cache from the children the change was about to replace. An inline element whose only text was then removed kept answering "holds text", and lost its `textFormat` in JSON, inside the update and after it (review). The cache is now correct by construction: it holds entries only for committed (frozen) states, which never change. A pending state uses the cache of the committed state it was made from only while no node in it has been marked dirty (written, created or removed): until then it holds exactly that state's nodes. A copy serializes in such an update, so it stays linear. An update that has changed nodes walks without a cache. `getWritable` is unchanged. ## Test plan `InlineElementTextFields.test.ts`: a new test with an inline element whose `afterCloneFrom` reads the JSON of the node it is made from; removing its only text keeps its `textFormat` in JSON, inside the update and once committed. ### Before ``` $ pnpm exec vitest run --project unit packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts × a clone that reads JSON before the change it is made for does not keep what the change replaces AssertionError: expected undefined to be 1 // Object.is equality Tests 1 failed | 9 passed (10) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts Tests 10 passed (10) ``` `getFirstChild` calls inside `shouldSerializeTextStyles`, nested marks with text fields at depth 32 / 64 / 128 / 256: a select-all `$generateJSONFromSelectedNodes` inside an update 32 / 64 / 128 / 256, and `toJSON` of the committed state 67 / 131 / 259 / 515, as on main. Renumbers the decision row this pull request adds to D82 (D81 is taken by another pull request). --- packages/lexical-y/DECISIONS.md | 2 +- packages/lexical/src/LexicalNode.ts | 2 - packages/lexical/src/LexicalUtils.ts | 43 +++++++++++----- .../unit/InlineElementTextFields.test.ts | 51 ++++++++++++++++++- .../lexical/src/nodes/LexicalElementNode.ts | 13 +++-- 5 files changed, 89 insertions(+), 22 deletions(-) diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index 4b247bf548a..d32afba4561 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -64,7 +64,7 @@ issues and docs that carry it. | D30 | **Diffed rewrites snap to whole words**; keystrokes and IME composition keep the exact diff. Scripts without spaces treat each character as a boundary. | Concurrent rewrites otherwise split words. Real word segmentation is a follow-up ([#9467]). | `DECIDED` | 10-26 | [#9400], [#9467] | | D31 | **A list item's nested list gets an item of its own.** `NodeStructureExtension`'s list rule prevents an item holding a nested list next to other content when it is written; the binding does not repair it afterwards. | That shape lost text in outdents, Backspaces and offline merges. Preventing it at write time follows D20. | `DECIDED` | 10-26 | [#9452], [y.md: Limits][limits] | | D32 | **List merging ignores `start`.** | Existing Lexical behaviour, intended. | `DECIDED` | 10-23 | [#9452] | -| D81 | **An inline element's textFormat and textStyle are persisted only while it holds no text at any depth, and the binding stores a wrapper's exactly when Lexical serializes them.** Lexical's `shouldSerializeTextStyles` looks through the elements inside an inline element (a mark in a link), as a reconcile already derives the two from its first text there; a block keeps the direct-children check. Exported JSON changes only for a link, autolink or mark whose children are all elements: those two fields are no longer written (they are still read, into the same state). | Stored while the wrapper holds text, a format change over a link writes the link's value again, and a collaborator's concurrent removal of the link is lost ([#9597]); left unstored while Lexical serializes them, the author's JSON differs from a fresh reader's. One rule, owned by core, keeps both, and the JSON of blocks and of every other node is unchanged (D01). A link with no text at all (only an inline decorator or a line break) still stores them and still comes back ([#9627]). | `DECIDED` | 10-30 | [#9601] | +| D82 | **An inline element's textFormat and textStyle are persisted only while it holds no text at any depth, and the binding stores a wrapper's exactly when Lexical serializes them.** Lexical's `shouldSerializeTextStyles` looks through the elements inside an inline element (a mark in a link), as a reconcile already derives the two from its first text there; a block keeps the direct-children check. Exported JSON changes only for a link, autolink or mark whose children are all elements: those two fields are no longer written (they are still read, into the same state). | Stored while the wrapper holds text, a format change over a link writes the link's value again, and a collaborator's concurrent removal of the link is lost ([#9597]); left unstored while Lexical serializes them, the author's JSON differs from a fresh reader's. One rule, owned by core, keeps both, and the JSON of blocks and of every other node is unchanged (D01). A link with no text at all (only an inline decorator or a line break) still stores them and still comes back ([#9627]). | `DECIDED` | 10-30 | [#9601] | | D70 | **No virtual structure.** The editor does not show repairs that are not stored (padded cells for a short table row, two stored lists drawn as one). The one existing case stays: text left without a line marker shows as a paragraph. | It adds another structure on top of Yjs and the Lexical tree, and the complexity that comes with it. | `DECIDED` | 10-25 | [#9400] | | D72 | **The binding starts at one fixed point, with no further deferral or special-casing.** It starts in `YCollabExtension`'s `afterRegistration`, once every extension has registered and the initial state is applied. An initialization-order bug (a table that exists before its DOM) is fixed in the extension that has it, as [#9501] did. | Deferring the first read further changes when the binding's state exists and still misses code that registers later (legacy table observers, React plugins). | `DECIDED` | 10-28 | [#9501], [#9537] | | D73 | **`@lexical/y` first publishes with its whole launch stack** (the binding, the server API, the playground and per-member sets), with real moves after (D27). The launch stack was chosen on 10-23; moves were moved after it on 10-25. | The first published format should already include what the launch needs, such as per-member sets. | `DECIDED` | 10-25 | [#9400], [#9437], [#9438], [#9439] | diff --git a/packages/lexical/src/LexicalNode.ts b/packages/lexical/src/LexicalNode.ts index 343519d011c..be001c0d88b 100644 --- a/packages/lexical/src/LexicalNode.ts +++ b/packages/lexical/src/LexicalNode.ts @@ -87,7 +87,6 @@ import { getStaticNodeConfig, getSuperclassOf, internalMarkNodeAsDirty, - internalResetTextInsideCache, } from './LexicalUtils'; const __DEV__ = process.env.NODE_ENV !== 'production'; @@ -1657,7 +1656,6 @@ export class LexicalNode { if (selection !== null) { selection.setCachedNodes(null); } - internalResetTextInsideCache(editorState); if (writableNode !== undefined) { // Transforms clear the dirty node set on each iteration to keep track on newly dirty nodes internalMarkNodeAsDirty(writableNode); diff --git a/packages/lexical/src/LexicalUtils.ts b/packages/lexical/src/LexicalUtils.ts index 4869c3db283..2ae0d9ea4b8 100644 --- a/packages/lexical/src/LexicalUtils.ts +++ b/packages/lexical/src/LexicalUtils.ts @@ -3130,18 +3130,38 @@ export function $isKeyInSelectionNodes( } /** - * For each element of an EditorState, whether it holds a TextNode at any - * depth, as ElementNode#shouldSerializeTextStyles asks for an inline element. - * Without it, serializing inline elements nested inside each other, each with - * a textFormat or textStyle, walks the same subtree once per level. A pending - * state's entry is dropped by getWritable, since a change to any node can - * change what its ancestors hold. + * For each element of a committed (frozen) EditorState, whether it holds a + * TextNode at any depth, as ElementNode#shouldSerializeTextStyles asks for an + * inline element. Without it, serializing inline elements nested inside each + * other, each with a textFormat or textStyle, walks the same subtree once per + * level. A frozen state never changes, so its entries never go stale. */ const textInsideCache = new WeakMap>(); -/** @internal */ -export function $getTextInsideCache(): Map { - const editorState = getActiveEditorState(); +/** + * @internal + * The cache of $getTextInsideCache's frozen state for the active state, or + * null where none is known to hold: a pending state uses the cache of the + * committed state it was made from only while no node in it has been marked + * dirty (written, created or removed), when it holds exactly that state's + * nodes. A copy serializes in such an update. + */ +export function $getTextInsideCache(): Map | null { + let editorState = getActiveEditorState(); + if (!editorState._readOnly) { + const editor = internalGetActiveEditor(); + if ( + editor === null || + editor._pendingEditorState !== editorState || + editor._dirtyType !== NO_DIRTY_NODES + ) { + return null; + } + editorState = editor._editorState; + if (!editorState._readOnly) { + return null; + } + } let cache = textInsideCache.get(editorState); if (cache === undefined) { cache = new Map(); @@ -3150,11 +3170,6 @@ export function $getTextInsideCache(): Map { return cache; } -/** @internal */ -export function internalResetTextInsideCache(editorState: EditorState): void { - textInsideCache.delete(editorState); -} - /** @internal */ export type TypeToNodeMap = Map; /** diff --git a/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts b/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts index e9a76beaa69..cb22430a9cf 100644 --- a/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts +++ b/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts @@ -37,6 +37,17 @@ class InlineWrapperNode extends ElementNode { } } +/** An inline element whose clone reads the JSON of the node it is made from. */ +class CloneReadsJSONNode extends InlineWrapperNode { + $config() { + return this.config('test-clone-reads-json', {extends: InlineWrapperNode}); + } + afterCloneFrom(prevNode: this): void { + super.afterCloneFrom(prevNode); + prevNode.exportJSON(); + } +} + function $wrapper(...children: LexicalNode[]): InlineWrapperNode { return $create(InlineWrapperNode) .setTextFormat(1) @@ -49,7 +60,7 @@ function withEditor(fn: () => void): void { defineExtension({ $initialEditorState: null, name: '[inline-element-text-fields]', - nodes: [InlineWrapperNode], + nodes: [InlineWrapperNode, CloneReadsJSONNode], }), ); editor.update(fn, {discrete: true}); @@ -264,4 +275,42 @@ describe('serializing inline elements nested inside each other', () => { expect(fields(outer)).toEqual(OMITTED); }); }); + + test('a clone that reads JSON before the change it is made for does not keep what the change replaces', () => { + using editor = buildEditorFromExtensions( + defineExtension({ + $initialEditorState: null, + name: '[inline-element-text-fields-clone]', + nodes: [InlineWrapperNode, CloneReadsJSONNode], + }), + ); + editor.update( + () => { + $getRoot().append( + $createParagraphNode().append( + $create(CloneReadsJSONNode) + .setTextFormat(1) + .append($createTextNode('leaf')), + ), + ); + }, + {discrete: true}, + ); + let inside: Fields = {}; + editor.update( + () => { + const wrapper = ( + $getRoot().getFirstChildOrThrow() as ElementNode + ).getFirstChildOrThrow() as CloneReadsJSONNode; + wrapper.getFirstChildOrThrow().remove(); + inside = fields(wrapper); + }, + {discrete: true}, + ); + expect(inside.textFormat).toBe(1); + const committed = editor.getEditorState().toJSON() as unknown as { + root: {children: {children: Fields[]}[]}; + }; + expect(committed.root.children[0].children[0].textFormat).toBe(1); + }); }); diff --git a/packages/lexical/src/nodes/LexicalElementNode.ts b/packages/lexical/src/nodes/LexicalElementNode.ts index 9fdc08c0a8c..584b3141706 100644 --- a/packages/lexical/src/nodes/LexicalElementNode.ts +++ b/packages/lexical/src/nodes/LexicalElementNode.ts @@ -1104,13 +1104,16 @@ function $hasTextChild(element: ElementNode, deep: boolean): boolean { return false; } -/** $hasTextChild's deep walk; every element it visits is cached. */ +/** + * $hasTextChild's deep walk; with a cache (see $getTextInsideCache), every + * element it visits is cached. + */ function $hasTextInside( element: ElementNode, - cache: Map, + cache: Map | null, ): boolean { const key = element.getKey(); - const cached = cache.get(key); + const cached = cache !== null ? cache.get(key) : undefined; if (cached !== undefined) { return cached; } @@ -1124,7 +1127,9 @@ function $hasTextInside( $isTextNode(child) || ($isElementNode(child) && $hasTextInside(child, cache)); } - cache.set(key, found); + if (cache !== null) { + cache.set(key, found); + } return found; } From 368deb0586bab78a9d319f0cbe3c42bb388a5556 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 17:25:47 -0700 Subject: [PATCH 07/18] [lexical][lexical-clipboard] Bug Fix: Keep a copy linear after a change in the same update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description The previous commit caches whether each element holds text for committed states, and for an update's pending state only until a node in it changes. A copy made in an update that had already changed nodes (for example a format change batched with the copy) walked without a cache. That was quadratic for inline elements nested inside each other, each with text fields (review: 528 / 2,080 / 8,256 / 32,896 visits at depth 32 / 64 / 128 / 256, where main makes 32 / 64 / 128 / 256). An export walk (`EditorState#toJSON`, the clipboard's selection export) now runs with a memo of its own (`INTERNAL_$withExportMemo`). It lives for the walk alone, which changes no node. `getWritable` drops it for the rest of the walk before cloning, so a clone hook that reads cannot refill it, and it cannot answer for content that changed. ## Test plan `InlineElementTextFields.test.ts`: the counted scaling test also copies (`$generateJSONFromSelectedNodes`) after a format change in the same update. ### Before ``` $ pnpm exec vitest run --project unit packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts × stays linear in the depth when each carries text fields, in a copy after a change in the same update AssertionError: expected 3.741687979539642 to be less than 3 Tests 1 failed | 10 passed (11) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts packages/lexical-clipboard Test Files 6 passed (6) Tests 50 passed (50) ``` `getFirstChild` calls inside `shouldSerializeTextStyles` for a copy after a format change in the same update, nested marks with text fields at depth 32 / 64 / 128 / 256: 32 / 64 / 128 / 256, as on main. --- packages/lexical-clipboard/src/clipboard.ts | 11 ++-- packages/lexical/src/LexicalEditorState.ts | 10 ++-- packages/lexical/src/LexicalNode.ts | 4 ++ packages/lexical/src/LexicalUtils.ts | 53 +++++++++++++++++-- .../unit/InlineElementTextFields.test.ts | 22 +++++++- packages/lexical/src/index.ts | 1 + 6 files changed, 86 insertions(+), 15 deletions(-) diff --git a/packages/lexical-clipboard/src/clipboard.ts b/packages/lexical-clipboard/src/clipboard.ts index 0d5ecce6d23..8035d47957a 100644 --- a/packages/lexical-clipboard/src/clipboard.ts +++ b/packages/lexical-clipboard/src/clipboard.ts @@ -53,6 +53,7 @@ import { getDOMSelection, getDOMSelectionPoints, getEditorPropertyFromDOMNode, + INTERNAL_$withExportMemo, isHTMLElement, isLexicalEditor, isSelectionWithinEditor, @@ -703,10 +704,12 @@ export function $generateJSONFromSelectedNodes< const topLevelChildren = ( $isElementNode(slotFrame) ? slotFrame : root ).getChildren(); - for (let i = 0; i < topLevelChildren.length; i++) { - const topLevelNode = topLevelChildren[i]; - $appendNodesToJSON(editor, selection, topLevelNode, nodes); - } + INTERNAL_$withExportMemo(() => { + for (let i = 0; i < topLevelChildren.length; i++) { + const topLevelNode = topLevelChildren[i]; + $appendNodesToJSON(editor, selection, topLevelNode, nodes); + } + }); return { namespace: editor._config.namespace, nodes, diff --git a/packages/lexical/src/LexicalEditorState.ts b/packages/lexical/src/LexicalEditorState.ts index 9ff33a2b346..89e1922ec97 100644 --- a/packages/lexical/src/LexicalEditorState.ts +++ b/packages/lexical/src/LexicalEditorState.ts @@ -23,7 +23,7 @@ import {cloneMap} from './LexicalGenMap'; import {$exportNodeJSON, $withCompactExport} from './LexicalSerializedExport'; import {$getSlot, $getSlotNames} from './LexicalSlot'; import {readEditorState} from './LexicalUpdates'; -import {$getRoot} from './LexicalUtils'; +import {$getRoot, INTERNAL_$withExportMemo} from './LexicalUtils'; import { $isElementNode, type SerializedElementNode, @@ -321,9 +321,11 @@ export class EditorState { // write the compact form for the latter. Anything that is not a stated // form is the legacy one, which is what both overloads promise. return $withCompactExport(typeof compact === 'boolean' && compact, () => - readEditorState(null, this, () => ({ - root: $exportNodeToJSON($getRoot()) as SerializedRootNode, - })), + readEditorState(null, this, () => + INTERNAL_$withExportMemo(() => ({ + root: $exportNodeToJSON($getRoot()) as SerializedRootNode, + })), + ), ); } } diff --git a/packages/lexical/src/LexicalNode.ts b/packages/lexical/src/LexicalNode.ts index be001c0d88b..fb846784367 100644 --- a/packages/lexical/src/LexicalNode.ts +++ b/packages/lexical/src/LexicalNode.ts @@ -87,6 +87,7 @@ import { getStaticNodeConfig, getSuperclassOf, internalMarkNodeAsDirty, + internalStopExportMemo, } from './LexicalUtils'; const __DEV__ = process.env.NODE_ENV !== 'production'; @@ -1646,6 +1647,9 @@ export class LexicalNode { return this; } errorOnReadOnly(); + // Before the clone, whose hooks may read: a node that changes ends an + // export walk's memo (see INTERNAL_$withExportMemo). + internalStopExportMemo(); const editorState = getActiveEditorState(); const editor = getActiveEditor(); const key = this.__key; diff --git a/packages/lexical/src/LexicalUtils.ts b/packages/lexical/src/LexicalUtils.ts index 2ae0d9ea4b8..b2e351a1550 100644 --- a/packages/lexical/src/LexicalUtils.ts +++ b/packages/lexical/src/LexicalUtils.ts @@ -3153,14 +3153,14 @@ export function $getTextInsideCache(): Map | null { if ( editor === null || editor._pendingEditorState !== editorState || - editor._dirtyType !== NO_DIRTY_NODES + editor._dirtyType !== NO_DIRTY_NODES || + !editor._editorState._readOnly ) { - return null; + return exportMemo !== null && exportMemo.editorState === editorState + ? exportMemo.holds + : null; } editorState = editor._editorState; - if (!editorState._readOnly) { - return null; - } } let cache = textInsideCache.get(editorState); if (cache === undefined) { @@ -3170,6 +3170,49 @@ export function $getTextInsideCache(): Map | null { return cache; } +/** + * The memo of one export walk in a pending state that has changed (see + * INTERNAL_$withExportMemo), or null. + */ +let exportMemo: null | { + editorState: EditorState; + holds: Map; +} = null; +/** Whether an export walk is running (with or without its memo). */ +let exportWalk = false; + +/** + * @internal + * Run an export walk (EditorState#toJSON, the clipboard's selection export) + * with a memo of what $getTextInsideCache would hold, for an update whose + * pending state has changed and so has no cache of its own. The memo lives + * for the walk alone, which changes no node: a node made writable during the + * walk (see internalStopExportMemo) drops it for the rest of the walk, so it + * cannot answer for content that has changed. + */ +export function INTERNAL_$withExportMemo(f: () => T): T { + if (exportWalk) { + return f(); + } + const editorState = internalGetActiveEditorState(); + exportWalk = true; + exportMemo = + editorState !== null && !editorState._readOnly + ? {editorState, holds: new Map()} + : null; + try { + return f(); + } finally { + exportWalk = false; + exportMemo = null; + } +} + +/** @internal Drop the export walk's memo (see INTERNAL_$withExportMemo). */ +export function internalStopExportMemo(): void { + exportMemo = null; +} + /** @internal */ export type TypeToNodeMap = Map; /** diff --git a/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts b/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts index cb22430a9cf..39958e751ed 100644 --- a/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts +++ b/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts @@ -6,6 +6,7 @@ * */ +import {$generateJSONFromSelectedNodes} from '@lexical/clipboard'; import {buildEditorFromExtensions} from '@lexical/extension'; import { $create, @@ -13,7 +14,9 @@ import { $createParagraphNode, $createTextNode, $getRoot, + $getSelection, $parseSerializedNode, + $selectAll, defineExtension, ElementNode, type LexicalNode, @@ -216,7 +219,11 @@ function $exportEach(node: LexicalNode): unknown { } describe('serializing inline elements nested inside each other', () => { - for (const where of ['a committed state', 'an update'] as const) { + for (const where of [ + 'a committed state', + 'an update', + 'a copy after a change in the same update', + ] as const) { test(`stays linear in the depth when each carries text fields, in ${where}`, () => { const calls: number[] = []; for (const depth of [64, 128, 256]) { @@ -242,13 +249,24 @@ describe('serializing inline elements nested inside each other', () => { countGetFirstChild(() => { if (where === 'a committed state') { json = JSON.stringify(editor.getEditorState().toJSON()); - } else { + } else if (where === 'an update') { editor.update( () => { json = JSON.stringify($exportEach($getRoot())); }, {discrete: true}, ); + } else { + editor.update( + () => { + $getRoot().getAllTextNodes()[0].toggleFormat('italic'); + $selectAll(); + json = JSON.stringify( + $generateJSONFromSelectedNodes(editor, $getSelection()), + ); + }, + {discrete: true}, + ); } }), ); diff --git a/packages/lexical/src/index.ts b/packages/lexical/src/index.ts index a832e852d33..87d34d40d57 100644 --- a/packages/lexical/src/index.ts +++ b/packages/lexical/src/index.ts @@ -449,6 +449,7 @@ export { getStaticNodeConfig, getTextDirection, INTERNAL_$isBlock, + INTERNAL_$withExportMemo, isBlockDomNode, isDocumentFragment, isDOMCapturingSelection, From bf0e80011abfb7fbf20880092f32c220b438f26c Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 17:35:29 -0700 Subject: [PATCH 08/18] [lexical] Chore: Declare INTERNAL_$withExportMemo in Lexical's Flow types ## Description The previous commit exports `INTERNAL_$withExportMemo` from `lexical` (used by `@lexical/clipboard`) but did not declare it in `packages/lexical/flow/Lexical.js.flow`. It is declared now, next to `INTERNAL_getActiveEditorState`, so the Flow surface matches the TypeScript one (review). ## Test plan ### Before `Lexical.js.flow` has no declaration for `INTERNAL_$withExportMemo`. ### After ``` $ pnpm run ci-check Found 0 errors $ pnpm exec vitest run --project scripts-unit Test Files 22 passed (22) Tests 3497 passed (3497) ``` --- packages/lexical/flow/Lexical.js.flow | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/lexical/flow/Lexical.js.flow b/packages/lexical/flow/Lexical.js.flow index 9c359284bb5..d031da3201e 100644 --- a/packages/lexical/flow/Lexical.js.flow +++ b/packages/lexical/flow/Lexical.js.flow @@ -1358,6 +1358,8 @@ declare export function $getDocument(): Document; declare export function $getEditor(): LexicalEditor; /** @internal */ declare export function INTERNAL_getActiveEditorState(): EditorState | null; +/** @internal */ +declare export function INTERNAL_$withExportMemo(f: () => T): T; declare export function $getEditorDOMRenderConfig( editor?: LexicalEditor, ): EditorDOMRenderConfig; From 4457a2fc6c2a2b5faec62840c8b7d7505c5f5cfe Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 18:06:23 -0700 Subject: [PATCH 09/18] [lexical-y] Bug Fix: Show a wrapper's stored text fields once a collaborator's delete leaves it without text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description A wrapper's textFormat and textStyle are stored only while it holds no text, as Lexical serializes them. `RangeSelection#formatText` sets them on the client that formats a text-holding link, and while the link holds text that value is stored nowhere. If a collaborator then deletes the link's text but not a line break or inline decorator inside it, the link holds no text and Lexical starts serializing the fields. The formatting client exported `textFormat: 1`; the other client and a fresh reader exported none, and nothing wrote it later (review). The reader now shows the stored text fields on a wrapper that holds no text: - when it builds the wrapper's line (as the wrapper's children are set); - when a read changes the content of a stored element inside wrappers (a mark inside a link, an overflow inside a mark inside a link) without reading the wrappers' line: the value comes from the formats stored at the nearest stored element below the wrapper. A wrapper with text is unchanged: its fields are not serialized. Also from the review: the `lexical` changeset says `Verify:` (a JSON change in a fix), not `Detect:`. ## Test plan `WrapperTextFields.test.ts`: a collaborator deletes the wrapped text (but not a line break or an inline decorator) after, or while, the other client formats over the wrapper. Every nesting of the default matrix, both orders, both client id orders. ### Before On bf0e8001: ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts AssertionError: b = fresh: expected { root: { …(8) } } to deeply equal { root: { …(8) } } Tests 112 failed | 2090 passed | 62 expected fail (2264) ``` The review's repro with the package harness (self-check on): link > [text, line break, text] and link > mark > [...], delete before and after the format syncs, both client id orders: 8 of 8 fail. ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 2202 passed | 62 expected fail (2264) $ MATRIX_FULL=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 8022 passed | 206 expected fail (8228) ``` The review's repro: 8 of 8 pass. --- .../lexical-inline-element-text-fields.md | 2 +- .../regressions/WrapperTextFields.test.ts | 53 +++++++++ packages/lexical-y/src/internal/Read.ts | 105 ++++++++++++++++++ 3 files changed, 159 insertions(+), 1 deletion(-) diff --git a/.changeset/lexical-inline-element-text-fields.md b/.changeset/lexical-inline-element-text-fields.md index ea9707725d6..8b8f7b2d749 100644 --- a/.changeset/lexical-inline-element-text-fields.md +++ b/.changeset/lexical-inline-element-text-fields.md @@ -6,4 +6,4 @@ An element persists its textFormat and textStyle only when it has no text to derive them from (#7968), and a reconcile derives them from its first text at any depth. The JSON of an inline element (a link, an autolink, a mark) now follows the same rule: a link whose text sits in a mark no longer writes them, as a link with text of its own already did not. JSON that holds them still reads the same, and a block keeps the direct-children check, so the JSON of a paragraph whose text is all inside links, and of every other node, is unchanged. -Detect: snapshots of exported JSON that hold `textFormat` or `textStyle` on a `link`, `autolink` or `mark` whose children are all elements. +Verify: JSON snapshots of a `link`, `autolink` or `mark` whose children are all elements no longer hold `textFormat` or `textStyle`. diff --git a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts index c0fe3c04991..bd1a209e788 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts @@ -384,6 +384,59 @@ describe('an author and a fresh reader have the same JSON', () => { }); }); +// A collaborator deletes the wrapped text but not a line break or an inline +// decorator next to it, after (or while) the other client formats over the +// wrapper: the wrapper now holds no text, so Lexical serializes its text +// fields, and every client shows the stored ones (the formatting client's +// own, set while the wrapper held text, were not stored). +describe('a wrapper whose text a collaborator deletes, formatted by another client', () => { + const cells = SHAPES.filter( + ({leaf}) => leaf === 'line-break' || leaf === 'text+decorator', + ).flatMap(shape => + (['after', 'concurrent'] as const).flatMap(when => + ( + [ + [1, 2], + [2, 1], + ] as [number, number][] + ).map(ids => ({...shape, ids, when})), + ), + ); + test.each(cells)( + '$label: delete $when, ids $ids.0,$ids.1', + async ({chain, leaf, fields, ids, when}) => { + const seed = seedYState(config, () => $build(chain, leaf, fields)); + const a = client(seed, ids[0]); + const b = client(seed, ids[1]); + await Promise.resolve(); + b.editor.update( + () => { + $selectAll().formatText('bold'); + }, + {discrete: true}, + ); + if (when === 'after') { + sync(a, b); + await Promise.resolve(); + } + a.editor.update( + () => { + for (const text of $wrappedTexts()) { + text.remove(); + } + $setSelection(null); + }, + {discrete: true}, + ); + sync(a, b); + await Promise.resolve(); + const expected = fresh(a.doc); + expect(json(a.editor), 'a = fresh').toEqual(expected); + expect(json(b.editor), 'b = fresh').toEqual(expected); + }, + ); +}); + // Removal races: every wrapper of a chain, removed by one client while the // other formats a range covering it, in both id orders. type Race = Shape & {removed: Kind; ids: [number, number]}; diff --git a/packages/lexical-y/src/internal/Read.ts b/packages/lexical-y/src/internal/Read.ts index b94916c3856..3fd61ca4bad 100644 --- a/packages/lexical-y/src/internal/Read.ts +++ b/packages/lexical-y/src/internal/Read.ts @@ -49,6 +49,7 @@ import { $foreignFlatKeys, $groupFields, $isGroupNode, + $isWrapper, $levelItemFields, $lineAttrs, $lineBreakAttrs, @@ -103,6 +104,7 @@ import { type WrapperValue, } from './Schema'; import { + isGone, isHeld, isShown, isShownEnd, @@ -286,6 +288,11 @@ export class Reader { * type it does not register is an unknown type (see State#unknownTypes). */ private readonly wrapperFields: (type: string) => OwnedFields | null; + /** + * Stored elements (a mark, an overflow) whose children this read set (see + * $settleWrapperTextFields). + */ + private readonly childrenSet = new Set(); /** Item -> unit cache for one read. */ private located = new Map(); /** Inline owners already read from the current document without a hold. */ @@ -407,6 +414,7 @@ export class Reader { try { read(); if (this.depth === 1) { + this.$settleWrapperTextFields(); this.$checkTakenOut(); } } finally { @@ -422,6 +430,7 @@ export class Reader { try { this.$readFullInner(cs, el); if (this.depth === 1) { + this.$settleWrapperTextFields(); this.$checkTakenOut(); } } finally { @@ -2666,6 +2675,24 @@ export class Reader { const stack: Frame[] = []; const finish = (frame: Frame) => { this.$setChildren(frame.node, frame.children); + // A wrapper that holds no text (any more: a collaborator deleted its + // text around a line break) shows the textFormat and textStyle its + // value stores, as a fresh reader does: they are stored exactly while + // Lexical serializes them (see $wrapperAttrs), and one this client set + // while the wrapper held text was not stored. + const node = frame.node.getLatest(); + const {textFormat, textStyle} = frame.value.json; + if ( + (node.getTextFormat() !== + (typeof textFormat === 'number' ? textFormat : 0) || + node.getTextStyle() !== + (typeof textStyle === 'string' ? textStyle : '')) && + node.shouldSerializeTextStyles() + ) { + this.$updateStored(node, () => + $setTextFields(node.getLatest(), frame.value.json), + ); + } }; const runLength = (from: number, sig: string) => { let n = 0; @@ -2901,7 +2928,69 @@ export class Reader { } /** Make `el`'s children exactly `children`, keeping unchanged ends. */ + /** + * The wrappers around a stored element (a link around a mark) whose + * content this read changed, without reading the wrappers' line: one that + * now holds no text shows the textFormat and textStyle its value stores, + * as wrappers built by a read do (see $readInline's `finish`). + */ + private $settleWrapperTextFields(): void { + for (const key of this.childrenSet) { + const el = $getNodeByKey(key); + let anchor = this.state.yByKey.get(key); + if (el === null || anchor === undefined) { + continue; + } + // The wrappers above a stored element are formats in the sequence + // that holds its item; above the next stored element, in that one's. + let values: WrapperValue[] | null = null; + for ( + let w: LexicalNode | null = el.getParent(); + $isElementNode(w); + w = w.getParent() + ) { + const wy = this.state.yByKey.get(w.getKey()); + // Inline content ends at the line: a stored element that is not + // inline (OverflowNode) still sits in it where its parent is inline. + const parent = w.getParent(); + if ( + !w.isInline() && + !(wy !== undefined && $isElementNode(parent) && parent.isInline()) + ) { + break; + } + if (wy !== undefined) { + anchor = wy; + values = null; + continue; + } + if ( + !$isWrapper(w) || + (w.getTextFormat() === 0 && w.getTextStyle() === '') || + !w.shouldSerializeTextStyles() || + anchor._item === null + ) { + continue; + } + if (values === null) { + values = wrappersFromAttrs(attrsAt(anchor._item), this.wrapperFields); + } + const wrapper = w; + const value = values.find(v => v.type === wrapper.getType()); + if (value !== undefined) { + this.$updateStored(wrapper, () => + $setTextFields(wrapper.getLatest(), value.json), + ); + } + } + } + this.childrenSet.clear(); + } + $setChildren(el: ElementNode, children: LexicalNode[]): void { + if (this.state.yByKey.has(el.getKey())) { + this.childrenSet.add(el.getKey()); + } const prev = el.getChildren(); let left = 0; const max = Math.min(prev.length, children.length); @@ -3643,3 +3732,19 @@ function isDeleted(y: YNode): boolean { } return false; } + +/** The formats stored in effect at `item`, in its parent's sequence. */ +function attrsAt(item: Item): Attrs { + const formats = new Map(); + const parent = item.parent as unknown as {_start: Item | null}; + for (let it = parent._start; it !== null && it !== item; it = it.right) { + if (!isGone(it) && it.content instanceof ContentFormat) { + if (it.content.value === null) { + formats.delete(it.content.key); + } else { + formats.set(it.content.key, it.content.value); + } + } + } + return Object.fromEntries(formats) as Attrs; +} From 7c1b49fff65b4f6504e53496b107572b6c1b261f Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 18:32:38 -0700 Subject: [PATCH 10/18] [lexical-y] Bug Fix: Show a wrapper's stored text fields after a client's own delete too, and find them in one pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Two follow-ups to the previous commit (review): - **Own delete.** The client that formats a link whose text sits in a mark (link > mark > text), and then deletes that text itself (keeping a line break), wrote no value for the link: its delete changed only the mark's content. The link holds no text now, so Lexical serializes its textFormat, and that client exported `textFormat: 1` while its peer and a fresh reader exported none. After a local write, a wrapper around a changed stored element that holds no text and shows other text fields than its stored value now has its container read again (as for any change the binding does not write), so it shows the stored ones. - **Linear time.** Finding a wrapper's stored value walked its sequence from the start for each wrapper: a remote change that left n formatted links without text visited O(n²) items (review: 81,792 at n = 128). The formats at all the wrappers' anchors are now found in one pass along each sequence (`$unstoredWrapperTextFields`, shared by the reader and the local path). ## Test plan - `WrapperTextFields.test.ts`: the delete cells also run with the formatting client deleting. - `WrapperTextFieldsCost.test.ts` (new): counts the items a remote read checks while settling 32, 64 and 128 such links, and requires each doubling to stay under 3x. ### Before On 4457a2fc: ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts × link>mark>line-break: self deletes after, ids 1,2 (and 47 more self-delete cells) Tests 48 failed | 2522 passed | 62 expected fail (2632) $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts AssertionError: expected 4.012578616352202 to be less than 3 ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts Tests 2571 passed | 62 expected fail (2633) $ MATRIX_FULL=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 9062 passed | 206 expected fail (9268) $ pnpm exec vitest run --project unit --project scripts-unit Tests 28204 passed | 252 expected fail | 135 skipped (28591) $ pnpm run ci-check Found 0 errors ``` The review's repros pass: self-delete (2 cells) and the 32-cell local/remote delete with undo and redo. --- .../regressions/WrapperTextFields.test.ts | 34 ++-- .../regressions/WrapperTextFieldsCost.test.ts | 101 ++++++++++ packages/lexical-y/src/internal/Binding.ts | 41 +++- packages/lexical-y/src/internal/Read.ts | 179 ++++++++++++------ 4 files changed, 280 insertions(+), 75 deletions(-) create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts diff --git a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts index bd1a209e788..c8f168bec71 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts @@ -384,27 +384,30 @@ describe('an author and a fresh reader have the same JSON', () => { }); }); -// A collaborator deletes the wrapped text but not a line break or an inline -// decorator next to it, after (or while) the other client formats over the -// wrapper: the wrapper now holds no text, so Lexical serializes its text -// fields, and every client shows the stored ones (the formatting client's -// own, set while the wrapper held text, were not stored). -describe('a wrapper whose text a collaborator deletes, formatted by another client', () => { +// A client deletes the wrapped text but not a line break or an inline +// decorator next to it, after (or while) a client formats over the wrapper +// (the other client, or the same one): the wrapper now holds no text, so +// Lexical serializes its text fields, and every client shows the stored ones +// (the formatting client's own, set while the wrapper held text, were not +// stored). +describe('a wrapper whose text a client deletes after a format over it', () => { const cells = SHAPES.filter( ({leaf}) => leaf === 'line-break' || leaf === 'text+decorator', ).flatMap(shape => (['after', 'concurrent'] as const).flatMap(when => - ( - [ - [1, 2], - [2, 1], - ] as [number, number][] - ).map(ids => ({...shape, ids, when})), + (['peer', 'self'] as const).flatMap(who => + ( + [ + [1, 2], + [2, 1], + ] as [number, number][] + ).map(ids => ({...shape, ids, when, who})), + ), ), ); test.each(cells)( - '$label: delete $when, ids $ids.0,$ids.1', - async ({chain, leaf, fields, ids, when}) => { + '$label: $who deletes $when, ids $ids.0,$ids.1', + async ({chain, leaf, fields, ids, when, who}) => { const seed = seedYState(config, () => $build(chain, leaf, fields)); const a = client(seed, ids[0]); const b = client(seed, ids[1]); @@ -419,7 +422,7 @@ describe('a wrapper whose text a collaborator deletes, formatted by another clie sync(a, b); await Promise.resolve(); } - a.editor.update( + (who === 'peer' ? a : b).editor.update( () => { for (const text of $wrappedTexts()) { text.remove(); @@ -428,6 +431,7 @@ describe('a wrapper whose text a collaborator deletes, formatted by another clie }, {discrete: true}, ); + await Promise.resolve(); sync(a, b); await Promise.resolve(); const expected = fresh(a.doc); diff --git a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts new file mode 100644 index 00000000000..a119366303a --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts @@ -0,0 +1,101 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ +// @vitest-environment node + +// A remote change that leaves n formatted wrappers holding no text (a +// collaborator deletes the text in a mark inside each of n links, keeping a +// line break) shows each wrapper's stored text fields: the formats stored at +// the marks are found in one pass along the document, not one per wrapper +// from its start (see $unstoredWrapperTextFields). Counted as the items the +// read checks (isGone), which grow linearly with n. + +import {$createLinkNode} from '@lexical/link'; +import {$createMarkNode} from '@lexical/mark'; +import { + $createLineBreakNode, + $createParagraphNode, + $createTextNode, + $getRoot, + $setSelection, +} from 'lexical'; +import {expect, test, vi} from 'vitest'; + +import {expectConverged, setup} from '../../utils/harness'; + +const counter = vi.hoisted(() => ({on: false, visits: 0})); + +vi.mock('../../../internal/Seen', async importOriginal => { + const original = + await importOriginal(); + return { + ...original, + isGone: (...args: Parameters) => { + if (counter.on) { + counter.visits++; + } + return original.isGone(...args); + }, + }; +}); + +test('wrappers a remote delete leaves without text are settled in linear time', async () => { + const visits: number[] = []; + for (const n of [32, 64, 128]) { + const t = await setup(() => { + for (let i = 0; i < n; i++) { + $getRoot().append( + $createParagraphNode().append( + $createTextNode('before '), + $createLinkNode(`https://link${i}.test`).append( + $createMarkNode([`c${i}`]).append( + $createTextNode(`word${i}`), + $createLineBreakNode(), + ), + ), + $createTextNode(' after'), + ), + ); + } + }); + try { + t.b.update(() => { + const texts = $getRoot().getAllTextNodes(); + const last = texts[texts.length - 1]; + const selection = texts[0].select(0, 0); + selection.focus.set(last.getKey(), last.getTextContentSize(), 'text'); + selection.formatText('bold'); + $setSelection(null); + }); + await t.sync(); + t.offline(); + t.a.update(() => { + for (const text of $getRoot().getAllTextNodes()) { + if (text.getTextContent().startsWith('word')) { + text.remove(); + } + } + $setSelection(null); + }); + counter.visits = 0; + counter.on = true; + await t.sync(); + counter.on = false; + visits.push(counter.visits); + await expectConverged(t.peers); + } finally { + counter.on = false; + for (const p of t.peers) { + p.stop(); + } + } + } + // Doubling n doubles the work (about 2x); one pass per wrapper from the + // start of the document would quadruple it. + expect(visits[1] / visits[0]).toBeLessThan(3); + expect(visits[2] / visits[1]).toBeLessThan(3); +}, 60_000); diff --git a/packages/lexical-y/src/internal/Binding.ts b/packages/lexical-y/src/internal/Binding.ts index 671394ceee2..bd57653d12a 100644 --- a/packages/lexical-y/src/internal/Binding.ts +++ b/packages/lexical-y/src/internal/Binding.ts @@ -90,7 +90,7 @@ import { markFormatCloser, } from './Cursor'; import {$canonicalizeGroup, $switchesWrapper} from './Groups'; -import {$applyStoredAttrs, Reader} from './Read'; +import {$applyStoredAttrs, $unstoredWrapperTextFields, Reader} from './Read'; import { $isGroupNode, $isWrapper, @@ -109,6 +109,7 @@ import { type StorageKind, storageKindOf, withPins, + wrapperFieldsOf, } from './Schema'; import { attrsBefore, @@ -3725,6 +3726,9 @@ export class YBinding { } this.pending.add('normalize'); } + editorState.read(() => + this.$noteUnstoredWrapperTextFields(state, roles, dirtyElements), + ); } if (commit.userEdit) { this.rereads = 0; @@ -4312,6 +4316,41 @@ export class YBinding { return found; } + /** + * A local change that leaves a wrapper around a stored element holding no + * text (its own delete of the text in a mark inside a link) does not write + * the wrapper's value, so the text fields the client set while the wrapper + * held text, which Lexical now serializes, are stored nowhere: the + * wrapper's container is read again (see refused), and shows the stored + * ones, as every other client and a fresh reader do. + */ + private $noteUnstoredWrapperTextFields( + state: State, + roles: Roles, + dirtyElements: Map, + ): void { + const stored = [...dirtyElements.keys()].filter(key => + state.yByKey.has(key), + ); + if (stored.length === 0) { + return; + } + const editor = this._editor; + for (const [wrapper] of $unstoredWrapperTextFields(state, stored, type => + wrapperFieldsOf(editor, type), + )) { + const container = roles.$containerOf(wrapper); + const cs = + container === null || $isRootNode(container) + ? state.containers.get(this._root) + : state.csByKey.get(container.getKey()); + if (cs !== undefined) { + this.refused.add(cs); + this.pending.add('reread'); + } + } + } + /** Containers a write did not write a follow-up's change to (see refused). */ private noteRefused(writer: Writer, naming: boolean): void { for (const cs of writer.refused) { diff --git a/packages/lexical-y/src/internal/Read.ts b/packages/lexical-y/src/internal/Read.ts index 3fd61ca4bad..88d4360b242 100644 --- a/packages/lexical-y/src/internal/Read.ts +++ b/packages/lexical-y/src/internal/Read.ts @@ -2935,54 +2935,14 @@ export class Reader { * as wrappers built by a read do (see $readInline's `finish`). */ private $settleWrapperTextFields(): void { - for (const key of this.childrenSet) { - const el = $getNodeByKey(key); - let anchor = this.state.yByKey.get(key); - if (el === null || anchor === undefined) { - continue; - } - // The wrappers above a stored element are formats in the sequence - // that holds its item; above the next stored element, in that one's. - let values: WrapperValue[] | null = null; - for ( - let w: LexicalNode | null = el.getParent(); - $isElementNode(w); - w = w.getParent() - ) { - const wy = this.state.yByKey.get(w.getKey()); - // Inline content ends at the line: a stored element that is not - // inline (OverflowNode) still sits in it where its parent is inline. - const parent = w.getParent(); - if ( - !w.isInline() && - !(wy !== undefined && $isElementNode(parent) && parent.isInline()) - ) { - break; - } - if (wy !== undefined) { - anchor = wy; - values = null; - continue; - } - if ( - !$isWrapper(w) || - (w.getTextFormat() === 0 && w.getTextStyle() === '') || - !w.shouldSerializeTextStyles() || - anchor._item === null - ) { - continue; - } - if (values === null) { - values = wrappersFromAttrs(attrsAt(anchor._item), this.wrapperFields); - } - const wrapper = w; - const value = values.find(v => v.type === wrapper.getType()); - if (value !== undefined) { - this.$updateStored(wrapper, () => - $setTextFields(wrapper.getLatest(), value.json), - ); - } - } + for (const [wrapper, json] of $unstoredWrapperTextFields( + this.state, + this.childrenSet, + this.wrapperFields, + )) { + this.$updateStored(wrapper, () => + $setTextFields(wrapper.getLatest(), json), + ); } this.childrenSet.clear(); } @@ -3733,18 +3693,119 @@ function isDeleted(y: YNode): boolean { return false; } -/** The formats stored in effect at `item`, in its parent's sequence. */ -function attrsAt(item: Item): Attrs { - const formats = new Map(); - const parent = item.parent as unknown as {_start: Item | null}; - for (let it = parent._start; it !== null && it !== item; it = it.right) { - if (!isGone(it) && it.content instanceof ContentFormat) { - if (it.content.value === null) { - formats.delete(it.content.key); - } else { - formats.set(it.content.key, it.content.value); +/** + * The wrappers around the stored elements `keys` (a link around a mark, and + * around that link's own stored ancestors up to the line) that hold no text + * and show other text fields than their stored value, with that value's. + * Lexical serializes a wrapper's textFormat and textStyle while it holds no + * text, and the binding stores them exactly then (see $wrapperAttrs), so + * every client must show the stored ones: one a client set while the + * wrapper held text was not stored. The wrappers above a stored element are + * formats in the sequence that holds its item; above the next stored + * element, in that one's. + */ +export function $unstoredWrapperTextFields( + state: State, + keys: Iterable, + wrapperFields: (type: string) => OwnedFields | null, +): [ElementNode, Attrs][] { + const candidates: [ElementNode, Item][] = []; + const seen = new Set(); + for (const key of keys) { + const el = $getNodeByKey(key); + let anchor = state.yByKey.get(key); + if (el === null || anchor === undefined) { + continue; + } + for ( + let w: LexicalNode | null = el.getParent(); + $isElementNode(w); + w = w.getParent() + ) { + const wy = state.yByKey.get(w.getKey()); + // Inline content ends at the line: a stored element that is not + // inline (OverflowNode) still sits in it where its parent is inline. + const parent = w.getParent(); + if ( + !w.isInline() && + !(wy !== undefined && $isElementNode(parent) && parent.isInline()) + ) { + break; + } + if (wy !== undefined) { + anchor = wy; + continue; + } + if ( + anchor._item !== null && + !seen.has(w.getKey()) && + $isWrapper(w) && + (w.getTextFormat() !== 0 || w.getTextStyle() !== '') && + w.shouldSerializeTextStyles() + ) { + seen.add(w.getKey()); + candidates.push([w, anchor._item]); } } } - return Object.fromEntries(formats) as Attrs; + if (candidates.length === 0) { + return []; + } + const attrs = attrsAtItems(candidates.map(([, item]) => item)); + const out: [ElementNode, Attrs][] = []; + for (const [w, item] of candidates) { + const value = wrappersFromAttrs(attrs.get(item)!, wrapperFields).find( + v => v.type === w.getType(), + ); + if (value === undefined) { + continue; + } + const {textFormat, textStyle} = value.json; + if ( + w.getTextFormat() !== (typeof textFormat === 'number' ? textFormat : 0) || + w.getTextStyle() !== (typeof textStyle === 'string' ? textStyle : '') + ) { + out.push([w, value.json]); + } + } + return out; +} + +/** + * The formats stored in effect at each of `items`: one pass along each + * sequence that holds some of them, up to the last one. + */ +function attrsAtItems(items: Item[]): Map { + const bySequence = new Map>(); + for (const item of items) { + let set = bySequence.get(item.parent); + if (set === undefined) { + set = new Set(); + bySequence.set(item.parent, set); + } + set.add(item); + } + const out = new Map(); + for (const [parent, wanted] of bySequence) { + const formats = new Map(); + let left = wanted.size; + for ( + let it = (parent as {_start: Item | null})._start; + it !== null && left > 0; + it = it.right + ) { + if (wanted.has(it)) { + out.set(it, Object.fromEntries(formats) as Attrs); + left--; + } + if (!isGone(it) && it.content instanceof ContentFormat) { + if (it.content.value === null) { + formats.delete(it.content.key); + } else { + formats.set(it.content.key, it.content.value); + } + } + } + } + return out; } From 703e7cb11078ee1ef90d1981034d6c5d33b225d6 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 18:46:35 -0700 Subject: [PATCH 11/18] [lexical-y] Bug Fix: Settle a wrapper's text fields inside the client's own delete, with no re-read after it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description The previous commit read a wrapper's container again after a client's own delete left it without text (to show its stored text fields). That re-read is a commit of the binding, which app update listeners see: one that answers collaboration-tagged commits with a follow-up the binding does not write repeated until its fuse (review). The wrapper now shows its stored text fields inside the same update, from a node transform on the stored element the delete marks dirty (the mark inside the link). Only a wrapper that update left without text, with the text fields it had before, is reset: text fields a client sets on a wrapper that already holds no text are stored by the write as before. No commit follows the delete, so there is nothing for an app to answer. Also adds `LinkRetargetedWhileFormatted.test.ts`: a client extends a link over more text with a new URL while another formats text covering the link or inside it. The same root cause as #9597: on main the old URL comes back over the link's first word and the extension is cut. ## Test plan `WrapperTextFieldsOwnDelete.test.ts` (new, 6 cells): the formatting client deletes the link's text itself, with an update listener that does nothing, changes the link's attributes, or makes a change the binding does not write, on each collaboration-tagged commit. No such commit follows the delete, one undo step is added, the link shows `textFormat` 0, and the editor equals a fresh reader. ### Before On 7c1b49ff: ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts AssertionError: no commit of the binding follows: expected 13 to be +0 // Object.is equality AssertionError: no commit of the binding follows: expected 1 to be +0 // Object.is equality Tests 6 failed (6) ``` On main (551fe4a2): ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/LinkRetargetedWhileFormatted.test.ts × A 03 only || B underlines t4z (the link text exactly) (ids 1,2) (and 7 more) Tests 8 failed | 4 passed (12) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts Tests 6 passed (6) $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/LinkRetargetedWhileFormatted.test.ts Tests 12 passed (12) $ MATRIX_FULL=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 9062 passed | 206 expected fail (9268) $ pnpm exec vitest run --project unit --project scripts-unit Tests 28210 passed | 252 expected fail | 135 skipped (28597) $ pnpm run ci-check Found 0 errors ``` --- .../LinkRetargetedWhileFormatted.test.ts | 117 ++++++++++++++++ .../WrapperTextFieldsOwnDelete.test.ts | 126 ++++++++++++++++++ packages/lexical-y/src/internal/Binding.ts | 86 ++++++------ 3 files changed, 291 insertions(+), 38 deletions(-) create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/LinkRetargetedWhileFormatted.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts diff --git a/packages/lexical-y/src/__tests__/unit/regressions/LinkRetargetedWhileFormatted.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/LinkRetargetedWhileFormatted.test.ts new file mode 100644 index 00000000000..78df2d8bca2 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/LinkRetargetedWhileFormatted.test.ts @@ -0,0 +1,117 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ +// @vitest-environment node + +// A links t4z, and B gets it. Then, at the same time, A extends that link +// over t4z..t6z with a new URL (after linking t2z..t3z, or not), while B +// underlines text covering the link or inside it. Every character of t4z, +// t5z and t6z shows A's new link: B's format writes no value for the link +// (#9597, whose fix this shares), so the old URL does not come back over t4z +// and the extension is not cut. Compared per character (a word can span +// text nodes). + +import {$toggleLink, LinkNode} from '@lexical/link'; +import { + $createParagraphNode, + $createTextNode, + $getRoot, + $setSelection, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {$selectIn, expectConverged, setup} from '../../utils/harness'; + +// "t1z t2z t3z t4z t5z t6z": t2z 4..7 t3z 8..11 t4z 12..15 t5z 16..19 t6z 20..23 +for (const ids of [ + [1, 2], + [2, 1], +]) { + for (const [name, withB, with02, bs, be] of [ + ['A 02 + 03 || B underlines t3z-t4z', true, true, 8, 15], + ['A 03 only || B underlines t3z-t4z', true, false, 8, 15], + [ + 'A 03 only || B underlines t4z (the link text exactly)', + true, + false, + 12, + 15, + ], + ['A 03 only || B underlines "4z" (inside the link)', true, false, 13, 15], + [ + 'A 03 only || B underlines t2z-t3z (control: not the link)', + true, + false, + 4, + 11, + ], + ['control: A 02 + 03, B does nothing', false, true, 0, 0], + ] as [string, boolean, boolean, number, number][]) { + test(`${name} (ids ${ids})`, async () => { + const t = await setup( + () => { + $getRoot().append( + $createParagraphNode().append( + $createTextNode('t1z t2z t3z t4z t5z t6z'), + ), + ); + }, + {ids}, + ); + t.a.update(() => { + $selectIn([0], 12, 15); + $toggleLink('https://x.test/01'); + $setSelection(null); + }); + await t.sync(); + t.offline(); + if (with02) { + t.a.update(() => { + $selectIn([0], 4, 11); + $toggleLink('https://x.test/02'); + $setSelection(null); + }); + } + t.a.update(() => { + $selectIn([0], 12, 23); + $toggleLink('https://x.test/03'); + $setSelection(null); + }); + if (withB) { + t.b.update(() => { + $selectIn([0], bs, be).formatText('underline'); + $setSelection(null); + }); + } + await t.sync(); + await expectConverged(t.peers); + // per character link URL, then per word (a word can span text nodes) + const urls = t.a.read(() => { + let text = ''; + const per: string[] = []; + for (const n of $getRoot().getAllTextNodes()) { + const p = n.getParent(); + for (const ch of n.getTextContent()) { + text += ch; + per.push(p instanceof LinkNode ? p.getURL().slice(-2) : '-'); + } + } + const out: Record = {}; + for (const m of text.matchAll(/t\d+z/g)) + out[m[0]] = [ + ...new Set(per.slice(m.index!, m.index! + m[0].length)), + ].join('|'); + return out; + }); + expect({t4z: urls.t4z, t5z: urls.t5z, t6z: urls.t6z}).toEqual({ + t4z: '03', + t5z: '03', + t6z: '03', + }); + }); + } +} diff --git a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts new file mode 100644 index 00000000000..88b15593b31 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts @@ -0,0 +1,126 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ +// @vitest-environment node + +// The client that formats a link whose text sits in a mark, then deletes +// that text itself (keeping a line break): the link holds no text, so +// Lexical serializes its textFormat, which was set while the link held text +// and is stored nowhere. The link shows the stored value in that same +// update, so no commit of the binding follows for an app to answer: an +// update listener that answers collaboration-tagged commits (with a change +// of the link's attributes, or with a change the binding does not write) +// never runs. + +import {$createLinkNode} from '@lexical/link'; +import {$createMarkNode} from '@lexical/mark'; +import { + $createLineBreakNode, + $createParagraphNode, + $createTextNode, + $getRoot, + $setSelection, + COLLABORATION_TAG, + type ElementNode, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import { + $selectIn, + expectConverged, + flush, + freshReader, + setup, +} from '../../utils/harness'; + +const $link = () => + ($getRoot().getFirstChildOrThrow() as ElementNode) + .getChildren() + .find(n => n.getType() === 'link') as ElementNode; + +for (const reply of ['none', 'attrs', 'refused'] as const) { + for (const ids of [ + [1, 2], + [2, 1], + ]) { + test(`own delete leaves the link with its stored fields, reply ${reply}, ids ${ids}`, async () => { + const t = await setup( + () => { + $getRoot().append( + $createParagraphNode().append( + $createTextNode('before '), + $createLinkNode('https://example.test').append( + $createMarkNode(['comment']).append( + $createTextNode('word'), + $createLineBreakNode(), + ), + ), + $createTextNode(' after'), + ), + ); + }, + {ids}, + ); + try { + t.b.update(() => { + $selectIn([0], 0, 11).formatText('bold'); + $setSelection(null); + }); + await t.sync(); + const undoStack = t.b.binding.undoManager!.undoStack; + const before = undoStack.length; + let tagged = 0; + let replies = 0; + const stop = t.b.editor.registerUpdateListener(({tags}) => { + if (!tags.has(COLLABORATION_TAG)) { + return; + } + tagged++; + if (reply === 'none' || replies >= 12) { + return; + } + replies++; + t.b.editor.update( + () => { + if (reply === 'attrs') { + $link().setTextStyle('color: red'); + } else { + $getRoot().append( + $createParagraphNode().append($createTextNode('reply')), + ); + } + }, + {discrete: true, tag: 'block-id-repair'}, + ); + }); + t.b.update(() => { + for (const text of $getRoot().getAllTextNodes()) { + if (text.getTextContent() === 'word') { + text.remove(); + } + } + $setSelection(null); + }); + await t.sync(); + for (let i = 0; i < 3; i++) { + await flush(); + } + stop(); + expect(tagged, 'no commit of the binding follows').toBe(0); + expect(undoStack.length, 'one undo step').toBe(before + 1); + expect(t.b.read(() => $link().getTextFormat())).toBe(0); + const fresh = await freshReader(t.b); + expect(t.b.json()).toEqual(fresh.json()); + await expectConverged(t.peers); + } finally { + for (const p of t.peers) { + p.stop(); + } + } + }); + } +} diff --git a/packages/lexical-y/src/internal/Binding.ts b/packages/lexical-y/src/internal/Binding.ts index bd57653d12a..7d265f98499 100644 --- a/packages/lexical-y/src/internal/Binding.ts +++ b/packages/lexical-y/src/internal/Binding.ts @@ -42,6 +42,7 @@ import { $getNodeByKey, $getRoot, $getSelection, + $hasUpdateTag, $isElementNode, $isNodeSelection, $isRangeSelection, @@ -95,6 +96,7 @@ import { $isGroupNode, $isWrapper, $nodeAttrs, + $setTextFields, canStoreAs, deriveStorageKind, getGroupSpec, @@ -1165,10 +1167,56 @@ export class YBinding { $flushSyncAfterUpdate(); } }; + // A local change that leaves a wrapper around a stored element holding + // no text (its own delete of the text in a mark inside a link) does not + // write the wrapper's value, so the text fields this client set while the + // wrapper held text, which Lexical now serializes, are stored nowhere: + // in the same update, the wrapper shows the stored ones, as every other + // client and a fresh reader do (see $unstoredWrapperTextFields), and + // nothing follows the update for an app to answer. The stored element is + // the one the change marks dirty. + const settleTextFields = (node: LexicalNode) => + withPins(this._pins, () => { + const state = this.state; + if ( + this._status !== 'active' || + this.unread !== null || + !state.yByKey.has(node.getKey()) || + $hasUpdateTag(COLLABORATION_TAG) + ) { + return; + } + const editor = this._editor; + const found = $unstoredWrapperTextFields(state, [node.getKey()], type => + wrapperFieldsOf(editor, type), + ); + if (found.length === 0) { + return; + } + // Only a wrapper this update left without text, with the text fields + // it had: ones this update set on a wrapper without text are stored + // by the write. + const prev = editor.getEditorState(); + const stale = prev.read(() => + found.filter(([wrapper]) => { + const was = $getNodeByKey(wrapper.getKey()); + return ( + $isElementNode(was) && + !was.shouldSerializeTextStyles() && + was.getTextFormat() === wrapper.getTextFormat() && + was.getTextStyle() === wrapper.getTextStyle() + ); + }), + ); + for (const [wrapper, json] of stale) { + $setTextFields(wrapper, json); + } + }); for (const {klass} of this._editor._nodes.values()) { if (klass.prototype instanceof ElementNode) { this.disposers.push( this._editor.registerNodeTransform(klass, noteTransformed), + this._editor.registerNodeTransform(klass, settleTextFields), // The node structure normalization never reaches a named slot. this._editor.registerNodeTransform(klass, $wrapSlotShadowRootInline), ); @@ -3726,9 +3774,6 @@ export class YBinding { } this.pending.add('normalize'); } - editorState.read(() => - this.$noteUnstoredWrapperTextFields(state, roles, dirtyElements), - ); } if (commit.userEdit) { this.rereads = 0; @@ -4316,41 +4361,6 @@ export class YBinding { return found; } - /** - * A local change that leaves a wrapper around a stored element holding no - * text (its own delete of the text in a mark inside a link) does not write - * the wrapper's value, so the text fields the client set while the wrapper - * held text, which Lexical now serializes, are stored nowhere: the - * wrapper's container is read again (see refused), and shows the stored - * ones, as every other client and a fresh reader do. - */ - private $noteUnstoredWrapperTextFields( - state: State, - roles: Roles, - dirtyElements: Map, - ): void { - const stored = [...dirtyElements.keys()].filter(key => - state.yByKey.has(key), - ); - if (stored.length === 0) { - return; - } - const editor = this._editor; - for (const [wrapper] of $unstoredWrapperTextFields(state, stored, type => - wrapperFieldsOf(editor, type), - )) { - const container = roles.$containerOf(wrapper); - const cs = - container === null || $isRootNode(container) - ? state.containers.get(this._root) - : state.csByKey.get(container.getKey()); - if (cs !== undefined) { - this.refused.add(cs); - this.pending.add('reread'); - } - } - } - /** Containers a write did not write a follow-up's change to (see refused). */ private noteRefused(writer: Writer, naming: boolean): void { for (const cs of writer.refused) { From 6a80b3376a34f4df1fe08925e2172181c78deff4 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 19:06:28 -0700 Subject: [PATCH 12/18] [lexical-y] Bug Fix: Keep text fields an update sets on a wrapper it leaves without text, and look stored formats up once per update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Two follow-ups to settling a wrapper's text fields in a client's own delete (review): - **Fields the update sets.** The transform resets only a wrapper whose text fields are the ones it had before the update. It compared them inside a read of the previous state, where the wrapper resolved to its previous version, so the check always held: text fields the same update set on the link (or an app's transform set) were reset to the stored ones. The pending fields are now taken before that read. - **Cost.** The transform runs once for each changed stored element and looked their stored formats up one at a time: O(n²) for an update that empties n marks inside links. The formats at the stored nodes of a sequence are now noted in one pass, once per update. ## Test plan - `WrapperTextFieldsOwnDelete.test.ts`: 6 new cells. One update removes the link's text and sets its textFormat (0, 2, 16) and textStyle. The author, a fresh reader and the peer show them. - `WrapperTextFieldsCost.test.ts`: a second test counts the items checked while a client empties 32, 64 and 128 such marks in one update of its own. ### Before On 2b9d3411: ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts AssertionError: expected { format: +0, style: '' } to deeply equal { format: 2, style: 'color: red' } × wrappers a local delete leaves without text are settled in linear time AssertionError: expected 3.3445378151260505 to be less than 3 ``` (6 of the 12 own-delete cells fail; the remote cost test passes.) ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts Tests 14 passed (14) $ MATRIX_FULL=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 9062 passed | 206 expected fail (9268) $ pnpm exec vitest run --project unit --project scripts-unit Tests 30301 passed | 230 expected fail | 135 skipped (30666) $ pnpm run ci-check Found 0 errors ``` --- .../regressions/WrapperTextFieldsCost.test.ts | 80 +++++++++++++++++-- .../WrapperTextFieldsOwnDelete.test.ts | 57 ++++++++++++- packages/lexical-y/src/internal/Binding.ts | 47 ++++++++--- packages/lexical-y/src/internal/Read.ts | 56 ++++++++++++- 4 files changed, 220 insertions(+), 20 deletions(-) diff --git a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts index a119366303a..15abdba55d2 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts @@ -7,12 +7,13 @@ */ // @vitest-environment node -// A remote change that leaves n formatted wrappers holding no text (a -// collaborator deletes the text in a mark inside each of n links, keeping a -// line break) shows each wrapper's stored text fields: the formats stored at -// the marks are found in one pass along the document, not one per wrapper -// from its start (see $unstoredWrapperTextFields). Counted as the items the -// read checks (isGone), which grow linearly with n. +// A change that leaves n formatted wrappers holding no text (the text in a +// mark inside each of n links is deleted, keeping a line break) shows each +// wrapper's stored text fields: the formats stored at the marks are found in +// one pass along the document, not one per wrapper from its start (see +// $unstoredWrapperTextFields), whether a collaborator's delete is read or +// the client deletes in one update of its own. Counted as the items checked +// (isGone), which grow linearly with n. import {$createLinkNode} from '@lexical/link'; import {$createMarkNode} from '@lexical/mark'; @@ -43,6 +44,41 @@ vi.mock('../../../internal/Seen', async importOriginal => { }; }); +const $build = (n: number) => () => { + for (let i = 0; i < n; i++) { + $getRoot().append( + $createParagraphNode().append( + $createTextNode('before '), + $createLinkNode(`https://link${i}.test`).append( + $createMarkNode([`c${i}`]).append( + $createTextNode(`word${i}`), + $createLineBreakNode(), + ), + ), + $createTextNode(' after'), + ), + ); + } +}; + +const $boldAll = () => { + const texts = $getRoot().getAllTextNodes(); + const last = texts[texts.length - 1]; + const selection = texts[0].select(0, 0); + selection.focus.set(last.getKey(), last.getTextContentSize(), 'text'); + selection.formatText('bold'); + $setSelection(null); +}; + +const $deleteWords = () => { + for (const text of $getRoot().getAllTextNodes()) { + if (text.getTextContent().startsWith('word')) { + text.remove(); + } + } + $setSelection(null); +}; + test('wrappers a remote delete leaves without text are settled in linear time', async () => { const visits: number[] = []; for (const n of [32, 64, 128]) { @@ -99,3 +135,35 @@ test('wrappers a remote delete leaves without text are settled in linear time', expect(visits[1] / visits[0]).toBeLessThan(3); expect(visits[2] / visits[1]).toBeLessThan(3); }, 60_000); + +test('wrappers a local delete leaves without text are settled in linear time', async () => { + const visits: number[] = []; + for (const n of [32, 64, 128]) { + const t = await setup($build(n)); + try { + t.b.update($boldAll); + await t.sync(); + counter.visits = 0; + counter.on = true; + t.b.update($deleteWords); + counter.on = false; + visits.push(counter.visits); + expect( + t.b.read(() => + $getRoot() + .getAllTextNodes() + .some(text => text.getTextContent().startsWith('word')), + ), + ).toBe(false); + await t.sync(); + await expectConverged(t.peers); + } finally { + counter.on = false; + for (const p of t.peers) { + p.stop(); + } + } + } + expect(visits[1] / visits[0]).toBeLessThan(3); + expect(visits[2] / visits[1]).toBeLessThan(3); +}, 60_000); diff --git a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts index 88b15593b31..8e68a9c4c9c 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts @@ -14,7 +14,8 @@ // update, so no commit of the binding follows for an app to answer: an // update listener that answers collaboration-tagged commits (with a change // of the link's attributes, or with a change the binding does not write) -// never runs. +// never runs. Text fields the same update sets on the link are its own, and +// are stored. import {$createLinkNode} from '@lexical/link'; import {$createMarkNode} from '@lexical/mark'; @@ -124,3 +125,57 @@ for (const reply of ['none', 'attrs', 'refused'] as const) { }); } } + +for (const format of [0, 2, 16]) { + for (const ids of [ + [1, 2], + [2, 1], + ]) { + test(`text fields set in the update that leaves the link without text are stored, format ${format}, ids ${ids}`, async () => { + const t = await setup( + () => { + $getRoot().append( + $createParagraphNode().append( + $createTextNode('before '), + $createLinkNode('https://example.test').append( + $createMarkNode(['comment']).append( + $createTextNode('word'), + $createLineBreakNode(), + ), + ), + $createTextNode(' after'), + ), + ); + }, + {ids}, + ); + try { + t.b.update(() => { + $selectIn([0], 0, 11).formatText('bold'); + $setSelection(null); + }); + await t.sync(); + t.b.update(() => { + for (const text of $link().getAllTextNodes()) { + text.remove(); + } + $link().setTextFormat(format).setTextStyle('color: red'); + $setSelection(null); + }); + await t.sync(); + const $fields = () => ({ + format: $link().getTextFormat(), + style: $link().getTextStyle(), + }); + expect(t.b.read($fields)).toEqual({format, style: 'color: red'}); + const fresh = await freshReader(t.b); + expect(fresh.read($fields)).toEqual({format, style: 'color: red'}); + await expectConverged(t.peers); + } finally { + for (const p of t.peers) { + p.stop(); + } + } + }); + } +} diff --git a/packages/lexical-y/src/internal/Binding.ts b/packages/lexical-y/src/internal/Binding.ts index 7d265f98499..f72fd937bfd 100644 --- a/packages/lexical-y/src/internal/Binding.ts +++ b/packages/lexical-y/src/internal/Binding.ts @@ -91,7 +91,12 @@ import { markFormatCloser, } from './Cursor'; import {$canonicalizeGroup, $switchesWrapper} from './Groups'; -import {$applyStoredAttrs, $unstoredWrapperTextFields, Reader} from './Read'; +import { + $applyStoredAttrs, + $unstoredWrapperTextFields, + Reader, + type StoredFormatsMemo, +} from './Read'; import { $isGroupNode, $isWrapper, @@ -869,6 +874,11 @@ export class YBinding { private unread: Unread | null = null; /** While a change is read: the document as it is (see readChange). */ private reading = false; + /** The stored formats one update's transforms looked up (see settleTextFields). */ + private storedFormats: { + state: EditorState | null; + sequences: StoredFormatsMemo; + } = {sequences: new Map(), state: null}; /** This binding's view, for its work (see guard). */ private readonly seer = () => (this.reading ? null : this.unread); /** @@ -1187,24 +1197,41 @@ export class YBinding { return; } const editor = this._editor; - const found = $unstoredWrapperTextFields(state, [node.getKey()], type => - wrapperFieldsOf(editor, type), + // The stored formats are looked up once per update (the document + // does not change during its transforms). + const memo = this.storedFormats; + if (memo.state !== editor._pendingEditorState) { + memo.state = editor._pendingEditorState; + memo.sequences = new Map(); + } + const found = $unstoredWrapperTextFields( + state, + [node.getKey()], + type => wrapperFieldsOf(editor, type), + memo.sequences, + ).map( + ([wrapper, json]) => + [ + wrapper, + json, + wrapper.getTextFormat(), + wrapper.getTextStyle(), + ] as const, ); if (found.length === 0) { return; } // Only a wrapper this update left without text, with the text fields - // it had: ones this update set on a wrapper without text are stored - // by the write. - const prev = editor.getEditorState(); - const stale = prev.read(() => - found.filter(([wrapper]) => { + // it had before the update: ones this update set (on a wrapper + // without text, or in a transform) are stored by the write. + const stale = editor.getEditorState().read(() => + found.filter(([wrapper, , format, style]) => { const was = $getNodeByKey(wrapper.getKey()); return ( $isElementNode(was) && !was.shouldSerializeTextStyles() && - was.getTextFormat() === wrapper.getTextFormat() && - was.getTextStyle() === wrapper.getTextStyle() + was.getTextFormat() === format && + was.getTextStyle() === style ); }), ); diff --git a/packages/lexical-y/src/internal/Read.ts b/packages/lexical-y/src/internal/Read.ts index 88d4360b242..2d1070fb0d8 100644 --- a/packages/lexical-y/src/internal/Read.ts +++ b/packages/lexical-y/src/internal/Read.ts @@ -3708,6 +3708,7 @@ export function $unstoredWrapperTextFields( state: State, keys: Iterable, wrapperFields: (type: string) => OwnedFields | null, + memo?: StoredFormatsMemo, ): [ElementNode, Attrs][] { const candidates: [ElementNode, Item][] = []; const seen = new Set(); @@ -3751,7 +3752,10 @@ export function $unstoredWrapperTextFields( if (candidates.length === 0) { return []; } - const attrs = attrsAtItems(candidates.map(([, item]) => item)); + const attrs = attrsAtItems( + candidates.map(([, item]) => item), + memo, + ); const out: [ElementNode, Attrs][] = []; for (const [w, item] of candidates) { const value = wrappersFromAttrs(attrs.get(item)!, wrapperFields).find( @@ -3771,11 +3775,22 @@ export function $unstoredWrapperTextFields( return out; } +/** + * The formats stored in effect at the stored nodes (Y.Node items) of the + * sequences read so far, by sequence: for lookups from several calls while + * the document does not change (the transforms of one update). + */ +export type StoredFormatsMemo = Map>; + /** * The formats stored in effect at each of `items`: one pass along each - * sequence that holds some of them, up to the last one. + * sequence that holds some of them (up to the last one, or, with `memo`, + * along all of it once, noting them at every stored node). */ -function attrsAtItems(items: Item[]): Map { +function attrsAtItems( + items: Item[], + memo?: StoredFormatsMemo, +): Map { const bySequence = new Map>(); for (const item of items) { let set = bySequence.get(item.parent); @@ -3787,6 +3802,41 @@ function attrsAtItems(items: Item[]): Map { } const out = new Map(); for (const [parent, wanted] of bySequence) { + let known = memo === undefined ? undefined : memo.get(parent); + if (memo !== undefined && known === undefined) { + known = new Map(); + memo.set(parent, known); + const formats = new Map(); + for ( + let it = (parent as {_start: Item | null})._start; + it !== null; + it = it.right + ) { + if (it.content instanceof ContentType) { + known.set(it, Object.fromEntries(formats) as Attrs); + } else if (!isGone(it) && it.content instanceof ContentFormat) { + if (it.content.value === null) { + formats.delete(it.content.key); + } else { + formats.set(it.content.key, it.content.value); + } + } + } + } + if (known !== undefined) { + let all = true; + for (const item of wanted) { + const attrs = known.get(item); + if (attrs === undefined) { + all = false; + } else { + out.set(item, attrs); + } + } + if (all) { + continue; + } + } const formats = new Map(); let left = wanted.size; for ( From 48619320b7a5d8c11758137daae35462eab62e2d Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 19:44:54 -0700 Subject: [PATCH 13/18] [lexical][lexical-y] Chore: Name an app's own inline element in the changeset, run the core test in node, and put a doc comment back ## Description Review suggestions, no behavior change: - The `lexical` changeset's `Verify:` names an app's own inline `ElementNode` too, since the rule keys on `isInline()`. - `InlineElementTextFields.test.ts` runs in the node environment (it only uses editor state and the clipboard's JSON export). - `$setChildren`'s doc comment is back above it. ## Test plan ### Before The test ran under jsdom. ### After ``` $ pnpm exec vitest run --project unit packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts Tests 11 passed (11) $ pnpm run ci-check Found 0 errors ``` --- .changeset/lexical-inline-element-text-fields.md | 2 +- packages/lexical-y/src/internal/Read.ts | 2 +- .../lexical/src/__tests__/unit/InlineElementTextFields.test.ts | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.changeset/lexical-inline-element-text-fields.md b/.changeset/lexical-inline-element-text-fields.md index 8b8f7b2d749..3b8565033b3 100644 --- a/.changeset/lexical-inline-element-text-fields.md +++ b/.changeset/lexical-inline-element-text-fields.md @@ -6,4 +6,4 @@ An element persists its textFormat and textStyle only when it has no text to derive them from (#7968), and a reconcile derives them from its first text at any depth. The JSON of an inline element (a link, an autolink, a mark) now follows the same rule: a link whose text sits in a mark no longer writes them, as a link with text of its own already did not. JSON that holds them still reads the same, and a block keeps the direct-children check, so the JSON of a paragraph whose text is all inside links, and of every other node, is unchanged. -Verify: JSON snapshots of a `link`, `autolink` or `mark` whose children are all elements no longer hold `textFormat` or `textStyle`. +Verify: JSON snapshots of an inline element (a `link`, `autolink` or `mark`, or your own inline `ElementNode`) whose children are all elements no longer hold `textFormat` or `textStyle`. diff --git a/packages/lexical-y/src/internal/Read.ts b/packages/lexical-y/src/internal/Read.ts index 2d1070fb0d8..fa4afabf014 100644 --- a/packages/lexical-y/src/internal/Read.ts +++ b/packages/lexical-y/src/internal/Read.ts @@ -2927,7 +2927,6 @@ export class Reader { ); } - /** Make `el`'s children exactly `children`, keeping unchanged ends. */ /** * The wrappers around a stored element (a link around a mark) whose * content this read changed, without reading the wrappers' line: one that @@ -2947,6 +2946,7 @@ export class Reader { this.childrenSet.clear(); } + /** Make `el`'s children exactly `children`, keeping unchanged ends. */ $setChildren(el: ElementNode, children: LexicalNode[]): void { if (this.state.yByKey.has(el.getKey())) { this.childrenSet.add(el.getKey()); diff --git a/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts b/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts index 39958e751ed..b3dc5c2ed76 100644 --- a/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts +++ b/packages/lexical/src/__tests__/unit/InlineElementTextFields.test.ts @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. * */ +// @vitest-environment node import {$generateJSONFromSelectedNodes} from '@lexical/clipboard'; import {buildEditorFromExtensions} from '@lexical/extension'; From 0e66653b5e5045ffdc994bd80cba5ce49c802012 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 20:39:30 -0700 Subject: [PATCH 14/18] [lexical-y] Chore: Pin the overlapping-bold races that a link no longer protects, naming #9626 ## Description When two collaborators write a format at one spot at the same time, client id order decides which write wins, and part of a range can end plain (#9626). Before #9597's fix, a format over a link also wrote the link's value again, which opened the format inside the link. That protected the link's first word in some of these races. A link now behaves like a mark there (a comment on the same words loses the format on main the same way). Two shapes are pinned as expected failures naming #9626: - `BoldAcrossLinkEnd.test.ts`: A links t5z t6z, then bolds t4z..t7z, while B bolds t3z..t6z. t7z ends plain for both client id orders (main: for ids 1,2 only). The same race without a link fails for both orders on main too. - `BoldIntoLinkBehind.test.ts`: A bolds into a link's first word while B, one update behind, types inside the old bold run. t7z ends plain for ids 1,2 (main keeps it, and loses the space before the link in the same run). ## Test plan ### Before On main (f59f74d7), the cells marked here are expected failures: - `BoldAcrossLinkEnd.test.ts`: the link cells fail for ids 1,2 and pass for ids 2,1; the no-link cells fail for both orders. - `BoldIntoLinkBehind.test.ts`: the typing cells pass. - The same shape with a comment instead of the link fails its 2 typing cells for ids 1,2 on main. ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/BoldAcrossLinkEnd.test.ts packages/lexical-y/src/__tests__/unit/regressions/BoldIntoLinkBehind.test.ts Tests 6 passed | 6 expected fail (12) ``` --- .../regressions/BoldAcrossLinkEnd.test.ts | 95 ++++++++++++ .../regressions/BoldIntoLinkBehind.test.ts | 137 ++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/BoldAcrossLinkEnd.test.ts create mode 100644 packages/lexical-y/src/__tests__/unit/regressions/BoldIntoLinkBehind.test.ts diff --git a/packages/lexical-y/src/__tests__/unit/regressions/BoldAcrossLinkEnd.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/BoldAcrossLinkEnd.test.ts new file mode 100644 index 00000000000..7dda9a611d6 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/BoldAcrossLinkEnd.test.ts @@ -0,0 +1,95 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ +// @vitest-environment node + +// Two collaborators bold overlapping ranges at the same time: A bolds +// t4z..t7z while B bolds t3z..t6z. t7z should end bold: A bolded it and +// nobody else touched it. Two items of one format key at one spot resolve by +// client id, and part of A's range ends plain (#9626), without a link for +// both client id orders. With A first linking t5z t6z (A's bold then crosses +// the link's end), main kept t7z bold for client ids 2,1 only because A's +// bold also wrote the link's value again; a link's text fields are no longer +// stored (#9597), so the link behaves like a mark there and t7z ends plain +// for both orders: expected failures naming #9626. + +import {$toggleLink} from '@lexical/link'; +import { + $createParagraphNode, + $createTextNode, + $getRoot, + $setSelection, + type TextNode, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {$selectIn, expectConverged, setup} from '../../utils/harness'; + +const LINE = 't1z t2z t3z t4z t5z t6z t7z t8z'; +const at = (word: string) => LINE.indexOf(word); + +for (const ids of [ + [1, 2], + [2, 1], +]) { + for (const [name, aLinksFirst, bBolds] of [ + ['A links t5z t6z, then bolds t4z..t7z || B bolds t3z..t6z', true, true], + ['A bolds t4z..t7z || B bolds t3z..t6z (no link)', false, true], + ['control: A links t5z t6z, then bolds t4z..t7z', true, false], + ] as [string, boolean, boolean][]) { + // #9626: overlapping bolds of two collaborators leave part plain. + const known = bBolds; + (known ? test.fails : test)( + `${name}, ids ${ids}: t7z keeps A's bold${known ? ' (#9626)' : ''}`, + async () => { + const t = await setup( + () => { + $getRoot().append( + $createParagraphNode().append($createTextNode(LINE)), + ); + }, + {ids}, + ); + try { + t.offline(); + if (aLinksFirst) { + t.a.update(() => { + $selectIn([0], at('t5z'), at('t6z') + 3); + $toggleLink('https://x.test/12'); + $setSelection(null); + }); + } + t.a.update(() => { + $selectIn([0], at('t4z'), at('t7z') + 3).formatText('bold'); + $setSelection(null); + }); + if (bBolds) { + t.b.update(() => { + $selectIn([0], at('t3z'), at('t6z') + 3).formatText('bold'); + $setSelection(null); + }); + } + await t.sync(); + await expectConverged(t.peers); + expect( + t.a.read(() => + ( + $getRoot() + .getAllTextNodes() + .find(n => n.getTextContent().includes('t7z')) as TextNode + ).hasFormat('bold'), + ), + ).toBe(true); + } finally { + for (const p of t.peers) { + p.stop(); + } + } + }, + ); + } +} diff --git a/packages/lexical-y/src/__tests__/unit/regressions/BoldIntoLinkBehind.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/BoldIntoLinkBehind.test.ts new file mode 100644 index 00000000000..4a00477e432 --- /dev/null +++ b/packages/lexical-y/src/__tests__/unit/regressions/BoldIntoLinkBehind.test.ts @@ -0,0 +1,137 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ +// @vitest-environment node + +// A bolds t5z t6z, links t7z..t10z and underlines t6z, and B gets those. +// Then, at the same time, A bolds "t6z t7z" (the bold now reaches into the +// link's first word) while B, one update behind, types after t5z inside the +// old bold run. B's write closes that run again at its old end, in the same +// spot where A's write extends it: two items of one format key at one spot +// (#9626). For client ids 1,2, B's closer wins, and A's bold ends after t6z. +// The space after t6z loses it on main too. t7z keeps it on main only because +// A's bold over the link also wrote the link's value again and opened bold +// inside it. With a link's text fields no longer stored (#9597), the link +// behaves like a mark there (a comment on t7z loses it on main the same +// way), so t7z loses it too: expected failures naming #9626. + +import {$toggleLink} from '@lexical/link'; +import * as Y from '@y/y'; +import { + $createParagraphNode, + $createTextNode, + $getRoot, + $setSelection, + type TextNode, +} from 'lexical'; +import {expect, test} from 'vitest'; + +import {$selectIn, expectConverged, setup} from '../../utils/harness'; + +// "t1z t2z t3z t4z t5z t6z t7z t8z t9z t10z" +const $range = (from: string, to: string) => { + const text = $getRoot().getTextContent().replace(/\n/g, ''); + return [text.indexOf(from), text.indexOf(to) + to.length]; +}; + +for (const ids of [ + [1, 2], + [2, 1], +]) { + for (const [name, bTypes, bUnstrike] of [ + ['B types after t5z', true, false], + ['B types after t5z, after a no-op unstrike', true, true], + ['control: B does nothing', false, false], + ] as [string, boolean, boolean][]) { + // #9626: B's closer at the old end of the bold run wins over A's + // extension of it for this order. + const known = bTypes && ids[0] === 1; + (known ? test.fails : test)( + `${name}, ids ${ids}: t7z keeps A's bold${known ? ' (#9626)' : ''}`, + async () => { + const t = await setup( + () => { + $getRoot().append( + $createParagraphNode().append( + $createTextNode('t1z t2z t3z t4z t5z t6z t7z t8z t9z t10z'), + ), + ); + }, + {ids}, + ); + const $select = (from: string, to: string) => { + const [s, e] = $range(from, to); + return $selectIn([0], s, e); + }; + try { + t.offline(); + if (bUnstrike) { + t.b.update(() => { + const selection = $select('t8z', 't8z'); + selection.format = 4; + selection.formatText('strikethrough'); + $setSelection(null); + }); + } + t.a.update(() => { + $select('t5z', 't6z').formatText('bold'); + $setSelection(null); + }); + t.a.update(() => { + $select('t7z', 't10z'); + $toggleLink('https://x.test/01'); + $setSelection(null); + }); + t.a.update(() => { + $select('t6z', 't6z').formatText('underline'); + $setSelection(null); + }); + t.a.update(() => { + $select('t6z', 't7z').formatText('bold'); + $setSelection(null); + }); + // B gets A's first three updates, not the fourth. + const pending = ( + t.b as unknown as {pending: Map} + ).pending.get(t.a)!; + for (const update of pending.splice(0, 3)) { + Y.applyUpdate( + t.b.doc, + update, + (t.b as unknown as {net: unknown}).net, + ); + } + await new Promise(resolve => setTimeout(resolve, 0)); + if (bTypes) { + t.b.update(() => { + const [, end] = $range('t5z', 't5z'); + const selection = $selectIn([0], end, end); + selection.format = 0; + selection.insertText(' q12z'); + $setSelection(null); + }); + } + await t.sync(); + await expectConverged(t.peers); + expect( + t.a.read(() => + ( + $getRoot() + .getAllTextNodes() + .find(n => n.getTextContent().includes('t7z')) as TextNode + ).hasFormat('bold'), + ), + ).toBe(true); + } finally { + for (const p of t.peers) { + p.stop(); + } + } + }, + ); + } +} From 15bb1d0dac544451dda51d2700588905df378d30 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 21:03:47 -0700 Subject: [PATCH 15/18] [lexical-website][lexical-y] Documentation Update: Say what a link's text fields no longer stored change in the collaboration docs and D82 ## Description Review suggestions, no code change: - `docs/collaboration/y.md`: - The Undo limits now say that undoing a link while a collaborator formats text inside it removes the link on every client; a link with no text still keeps it (#9627). - Other races name #9626 for overlapping ranges, and its reach to a link's edge and a link's first word. - Open issues drop #9597 and #9521's format-over-the-link's-start case, which this pull request fixes. - D82 adds two facts. A value stored by an earlier build can come back once, until the link's next write. The #9626 trade this pull request accepts is formatting only. - The `WrapperTextFields.test.ts` expected failures for a link with no text name #9627 in their titles. ## Test plan ### Before The docs said a format in a link writes the link again, and listed #9597 as open. ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 2570 passed | 62 expected fail (2632) $ pnpm run ci-check Found 0 errors ``` --- .../lexical-website/docs/collaboration/y.md | 23 +++++++++---------- packages/lexical-y/DECISIONS.md | 7 +++--- .../regressions/WrapperTextFields.test.ts | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/lexical-website/docs/collaboration/y.md b/packages/lexical-website/docs/collaboration/y.md index 548778ffca3..509414d0b7b 100644 --- a/packages/lexical-website/docs/collaboration/y.md +++ b/packages/lexical-website/docs/collaboration/y.md @@ -1199,9 +1199,11 @@ not merge as a user might expect: whether you saw it before you undo or it arrives as you undo: bold `one two`, let a collaborator type plain ` mid` after `one`, then undo the bold, and ` two` stays bold. Undoing a link - while a collaborator formats text inside it keeps the link on its text: - formatting text in a link also sets the link's text format, which writes - the link there again. Where a collaborator overwrote a step of yours + while a collaborator formats text inside it removes the link on every + client: a format writes no link while the link holds text. A link with no + text (only an inline decorator or a line break) still stores its text + format, and stays on its content: + [#9627](https://github.com/meta/lexical/issues/9627). Where a collaborator overwrote a step of yours entirely, the undo manager skips that step as changing nothing, and the same undo also reverts your step before it. When you undo a format change while a collaborator removes that format and undoes the removal, the text @@ -1280,7 +1282,12 @@ not merge as a user might expect: after a Backspace it takes the type of the line before (a paragraph after a heading becomes a heading), after undoing a split it keeps its own. Bold ‖ bold on adjacent ranges of one line - can leave part unformatted. A format removed from a range while a + can leave part unformatted, and so can bold ‖ bold on overlapping ranges + ([#9626](https://github.com/meta/lexical/issues/9626)). A link behaves + like a comment there: a range that crosses a link's edge can lose part + of its bold for either client id order, and a bold reaching into a + link's first word can lose it there when a collaborator one update + behind types inside the old bold range. A format removed from a range while a collaborator types inside it without that format (unbolding a word as they type into it, resolving a comment as they paste into it) comes back on the part of the range after their text: their write restores the @@ -1321,9 +1328,6 @@ reproduces it: - An update listener that throws stops the listeners registered after it, so when it was registered before the binding, that commit is never written: [#9514](https://github.com/meta/lexical/issues/9514). -- Undoing a link while a collaborator formats text over the link's start - keeps the link on its own text (see Undo above): - [#9521](https://github.com/meta/lexical/issues/9521). - A line without a marker joins the start of a paragraph a collaborator appends at the same time: [#9524](https://github.com/meta/lexical/issues/9524). @@ -1333,11 +1337,6 @@ reproduces it: - A comment can spread onto later words when a client types uncommented text inside it offline while a collaborator's deletes there drop the item that ended it: [#9564](https://github.com/meta/lexical/issues/9564). -- A link a collaborator removes comes back when another collaborator - formats a range covering it at the same time; an update that makes a - line's text nodes again without the link while a stale edit formats a - range reaching past the link's text shows the same: - [#9597](https://github.com/meta/lexical/issues/9597). - Turning a paragraph inside a container that is itself inside a container (both not shadow roots) into a list removes the outer container and makes no list, in every `@lexical/y` editor (it runs diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index 64e2dbc7fd6..907dcf584b1 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -64,7 +64,7 @@ issues and docs that carry it. | D30 | **Diffed rewrites snap to whole words**; keystrokes and IME composition keep the exact diff. Scripts without spaces treat each character as a boundary. **A diffed insertion or deletion without a caret that the diff would put inside a word goes to the first place in the run of equal text it could be in where both its ends are at word edges** (a word deleted with the separator before it); a caret's placement, and one already at word edges, stay. | Concurrent rewrites otherwise split words. Real word segmentation is a follow-up ([#9467]). Without a caret a diff takes the common start first and put deletions inside words, so a collaborator's concurrent delete of the same word took a letter of the next one ([#9636]); the first edge keeps a word typed after a deleted word apart from the next one, as typing goes (a space, then a word). | `DECIDED` | 10-30 | [#9400], [#9467], [#9636] | | D31 | **A list item's nested list gets an item of its own.** `NodeStructureExtension`'s list rule prevents an item holding a nested list next to other content when it is written; the binding does not repair it afterwards. | That shape lost text in outdents, Backspaces and offline merges. Preventing it at write time follows D20. | `DECIDED` | 10-26 | [#9452], [y.md: Limits][limits] | | D32 | **List merging ignores `start`.** | Existing Lexical behaviour, intended. | `DECIDED` | 10-23 | [#9452] | -| D82 | **An inline element's textFormat and textStyle are persisted only while it holds no text at any depth, and the binding stores a wrapper's exactly when Lexical serializes them.** Lexical's `shouldSerializeTextStyles` looks through the elements inside an inline element (a mark in a link), as a reconcile already derives the two from its first text there; a block keeps the direct-children check. Exported JSON changes only for a link, autolink or mark whose children are all elements: those two fields are no longer written (they are still read, into the same state). | Stored while the wrapper holds text, a format change over a link writes the link's value again, and a collaborator's concurrent removal of the link is lost ([#9597]); left unstored while Lexical serializes them, the author's JSON differs from a fresh reader's. One rule, owned by core, keeps both, and the JSON of blocks and of every other node is unchanged (D01). A link with no text at all (only an inline decorator or a line break) still stores them and still comes back ([#9627]). | `DECIDED` | 10-30 | [#9601] | +| D82 | **An inline element's textFormat and textStyle are persisted only while it holds no text at any depth, and the binding stores a wrapper's exactly when Lexical serializes them.** Lexical's `shouldSerializeTextStyles` looks through the elements inside an inline element (a mark in a link), as a reconcile already derives the two from its first text there; a block keeps the direct-children check. Exported JSON changes only for a link, autolink or mark whose children are all elements: those two fields are no longer written (they are still read, into the same state). | Stored while the wrapper holds text, a format change over a link writes the link's value again, and a collaborator's concurrent removal of the link is lost ([#9597]); left unstored while Lexical serializes them, the author's JSON differs from a fresh reader's. One rule, owned by core, keeps both, and the JSON of blocks and of every other node is unchanged (D01). A link with no text at all (only an inline decorator or a line break) still stores them and still comes back ([#9627]). A link whose stored value already holds them (written by an earlier build after a format over it) can come back once, until its next write. Accepted with it: a link behaves like a mark in overlapping same-format races ([#9626]), which now reach a link's edge for either client id order and a link's first word; formatting only, no text lost. | `DECIDED` | 10-30 | [#9601] | | D70 | **No virtual structure.** The editor does not show repairs that are not stored (padded cells for a short table row, two stored lists drawn as one). The one existing case stays: text left without a line marker shows as a paragraph. | It adds another structure on top of Yjs and the Lexical tree, and the complexity that comes with it. | `DECIDED` | 10-25 | [#9400] | | D72 | **The binding starts at one fixed point, with no further deferral or special-casing.** It starts in `YCollabExtension`'s `afterRegistration`, once every extension has registered and the initial state is applied. An initialization-order bug (a table that exists before its DOM) is fixed in the extension that has it, as [#9501] did. | Deferring the first read further changes when the binding's state exists and still misses code that registers later (legacy table observers, React plugins). | `DECIDED` | 10-28 | [#9501], [#9537] | | D73 | **`@lexical/y` first publishes with its whole launch stack** (the binding, the server API, the playground and per-member sets), with real moves after (D27). The launch stack was chosen on 10-23; moves were moved after it on 10-25. | The first published format should already include what the launch needs, such as per-member sets. | `DECIDED` | 10-25 | [#9400], [#9437], [#9438], [#9439] | @@ -194,15 +194,16 @@ issues and docs that carry it. [#9593]: https://github.com/meta/lexical/issues/9593 [#9598]: https://github.com/meta/lexical/pull/9598 [#9599]: https://github.com/meta/lexical/issues/9599 +[#9601]: https://github.com/meta/lexical/pull/9601 [#9603]: https://github.com/meta/lexical/issues/9603 [#9605]: https://github.com/meta/lexical/pull/9605 [#9607]: https://github.com/meta/lexical/pull/9607 [#9608]: https://github.com/meta/lexical/issues/9608 [#9609]: https://github.com/meta/lexical/issues/9609 [#9610]: https://github.com/meta/lexical/issues/9610 -[#9597]: https://github.com/meta/lexical/issues/9597 -[#9601]: https://github.com/meta/lexical/pull/9601 [#9621]: https://github.com/meta/lexical/issues/9621 +[#9626]: https://github.com/meta/lexical/issues/9626 [#9627]: https://github.com/meta/lexical/issues/9627 [#9635]: https://github.com/meta/lexical/issues/9635 [#9636]: https://github.com/meta/lexical/issues/9636 +[#9597]: https://github.com/meta/lexical/issues/9597 diff --git a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts index c8f168bec71..07e6e61bcd1 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts @@ -472,7 +472,7 @@ describe('a wrapper one client removes stays removed while another formats over [true, RACES.filter(knownTextlessLink)], ] as const) { (known ? test.fails : test).each(races)( - '$label: remove $removed, ids $ids.0,$ids.1', + `$label: remove $removed, ids $ids.0,$ids.1${known ? ' (#9627)' : ''}`, async ({chain, leaf, fields, removed, ids}) => { const seed = seedYState(config, () => $build(chain, leaf, fields)); const a = client(seed, ids[0]); From b415918ee8aab73f12a06d9b8f3c3603207f0c1e Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 22:07:24 -0700 Subject: [PATCH 16/18] [lexical-y] Bug Fix: Show the stored text fields on a link whose own text a local delete removes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Review of 15bb1d0d, a regression against main through a delete. Take "before " + a link ("wrapped", line break) + " after". B bolds the line, which syncs. Then, at the same time, A removes the link or gives it another URL, while B deletes "wrapped" (the line break stays in the link). The link came back, or kept its old URL, on every client, in both client id orders. Cause: once the delete left the link without text, Lexical serialized the text format B's bold had set on it, which was never stored. B's write then wrote the link's whole value again over the line break, over A's change. A link around a mark was already handled: the text fields were reset in the same update from the stored element the delete marks dirty. Nothing did that for text directly inside the link. The same transform now also runs on the wrapper itself. A wrapper this update left without text, with the text fields it had before the update, shows the value stored at its first atom. That value comes from one walk of its line, and the offsets of all the line's wrappers from one pass, both once per update. The previous state is read with `editor.read('latest', …)` (review nit). Also: - `WrapperTextFields.test.ts` adds the review's cells: the delete with a concurrent removal or URL change, for a line break and an inline decorator left in the wrapper, every nesting, both client id orders. - It also adds the rarer shape: a wrapper holding only a line break or an inline decorator, with text fields of its own, that one client appends text into while the other removes it or changes its URL. Where the text goes into the link itself, the link comes back or keeps its old URL (on main it does not). That is #9627's mechanism, a text-less link storing its text fields in its value: the append drops them and writes the value again. Those cells are expected failures naming #9627, and D82 and the docs say so. - `WrapperTextFieldsCost.test.ts` adds a counted test: a local delete in one line of 32, 64 and 128 links. ## Test plan ### Before On 15bb1d0d: ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts -t 'removes or retargets' × link>line-break: remove link, ids 1,2 (and 63 more: every link shape, remove and retarget, both orders) Tests 64 failed | 376 passed | 2632 skipped (3072) ``` On main (f59f74d7) the same cells pass 440 of 440. The append cells pass 220 of 220 on main; on this branch, 40 fail, the ones now marked for #9627. ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts Tests 3193 passed | 102 expected fail (3295) $ MATRIX_FULL=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 11562 passed | 310 expected fail (11872) $ pnpm exec vitest run --project unit --project scripts-unit Tests 31058 passed | 322 expected fail | 135 skipped (31515) $ pnpm run ci-check Found 0 errors ``` The counted test fails when the per-update cache of a line's atoms is turned off (ratio 3.6). --- .../lexical-website/docs/collaboration/y.md | 3 +- packages/lexical-y/DECISIONS.md | 2 +- .../regressions/WrapperTextFields.test.ts | 211 +++++++++++++++ .../regressions/WrapperTextFieldsCost.test.ts | 91 +++++++ packages/lexical-y/src/internal/Binding.ts | 240 +++++++++++++++--- 5 files changed, 512 insertions(+), 35 deletions(-) diff --git a/packages/lexical-website/docs/collaboration/y.md b/packages/lexical-website/docs/collaboration/y.md index 509414d0b7b..f5a4eb66c39 100644 --- a/packages/lexical-website/docs/collaboration/y.md +++ b/packages/lexical-website/docs/collaboration/y.md @@ -1202,7 +1202,8 @@ not merge as a user might expect: while a collaborator formats text inside it removes the link on every client: a format writes no link while the link holds text. A link with no text (only an inline decorator or a line break) still stores its text - format, and stays on its content: + format, and stays on its content, and so does such a link a collaborator + removes, or gives another URL, while you append text into it: [#9627](https://github.com/meta/lexical/issues/9627). Where a collaborator overwrote a step of yours entirely, the undo manager skips that step as changing nothing, and the same undo also reverts your step before it. When you undo a format change diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index 907dcf584b1..ad1cb27065f 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -64,7 +64,7 @@ issues and docs that carry it. | D30 | **Diffed rewrites snap to whole words**; keystrokes and IME composition keep the exact diff. Scripts without spaces treat each character as a boundary. **A diffed insertion or deletion without a caret that the diff would put inside a word goes to the first place in the run of equal text it could be in where both its ends are at word edges** (a word deleted with the separator before it); a caret's placement, and one already at word edges, stay. | Concurrent rewrites otherwise split words. Real word segmentation is a follow-up ([#9467]). Without a caret a diff takes the common start first and put deletions inside words, so a collaborator's concurrent delete of the same word took a letter of the next one ([#9636]); the first edge keeps a word typed after a deleted word apart from the next one, as typing goes (a space, then a word). | `DECIDED` | 10-30 | [#9400], [#9467], [#9636] | | D31 | **A list item's nested list gets an item of its own.** `NodeStructureExtension`'s list rule prevents an item holding a nested list next to other content when it is written; the binding does not repair it afterwards. | That shape lost text in outdents, Backspaces and offline merges. Preventing it at write time follows D20. | `DECIDED` | 10-26 | [#9452], [y.md: Limits][limits] | | D32 | **List merging ignores `start`.** | Existing Lexical behaviour, intended. | `DECIDED` | 10-23 | [#9452] | -| D82 | **An inline element's textFormat and textStyle are persisted only while it holds no text at any depth, and the binding stores a wrapper's exactly when Lexical serializes them.** Lexical's `shouldSerializeTextStyles` looks through the elements inside an inline element (a mark in a link), as a reconcile already derives the two from its first text there; a block keeps the direct-children check. Exported JSON changes only for a link, autolink or mark whose children are all elements: those two fields are no longer written (they are still read, into the same state). | Stored while the wrapper holds text, a format change over a link writes the link's value again, and a collaborator's concurrent removal of the link is lost ([#9597]); left unstored while Lexical serializes them, the author's JSON differs from a fresh reader's. One rule, owned by core, keeps both, and the JSON of blocks and of every other node is unchanged (D01). A link with no text at all (only an inline decorator or a line break) still stores them and still comes back ([#9627]). A link whose stored value already holds them (written by an earlier build after a format over it) can come back once, until its next write. Accepted with it: a link behaves like a mark in overlapping same-format races ([#9626]), which now reach a link's edge for either client id order and a link's first word; formatting only, no text lost. | `DECIDED` | 10-30 | [#9601] | +| D82 | **An inline element's textFormat and textStyle are persisted only while it holds no text at any depth, and the binding stores a wrapper's exactly when Lexical serializes them.** Lexical's `shouldSerializeTextStyles` looks through the elements inside an inline element (a mark in a link), as a reconcile already derives the two from its first text there; a block keeps the direct-children check. Exported JSON changes only for a link, autolink or mark whose children are all elements: those two fields are no longer written (they are still read, into the same state). | Stored while the wrapper holds text, a format change over a link writes the link's value again, and a collaborator's concurrent removal of the link is lost ([#9597]); left unstored while Lexical serializes them, the author's JSON differs from a fresh reader's. One rule, owned by core, keeps both, and the JSON of blocks and of every other node is unchanged (D01). A link with no text at all (only an inline decorator or a line break) still stores them and still comes back ([#9627]), also when a collaborator appends text into it while another removes it or changes its URL (the append drops the stored fields, which writes its value again). A link whose stored value already holds them (written by an earlier build after a format over it) can come back once, until its next write. Accepted with it: a link behaves like a mark in overlapping same-format races ([#9626]), which now reach a link's edge for either client id order and a link's first word; formatting only, no text lost. | `DECIDED` | 10-30 | [#9601] | | D70 | **No virtual structure.** The editor does not show repairs that are not stored (padded cells for a short table row, two stored lists drawn as one). The one existing case stays: text left without a line marker shows as a paragraph. | It adds another structure on top of Yjs and the Lexical tree, and the complexity that comes with it. | `DECIDED` | 10-25 | [#9400] | | D72 | **The binding starts at one fixed point, with no further deferral or special-casing.** It starts in `YCollabExtension`'s `afterRegistration`, once every extension has registered and the initial state is applied. An initialization-order bug (a table that exists before its DOM) is fixed in the extension that has it, as [#9501] did. | Deferring the first read further changes when the binding's state exists and still misses code that registers later (legacy table observers, React plugins). | `DECIDED` | 10-28 | [#9501], [#9537] | | D73 | **`@lexical/y` first publishes with its whole launch stack** (the binding, the server API, the playground and per-member sets), with real moves after (D27). The launch stack was chosen on 10-23; moves were moved after it on 10-25. | The first published format should already include what the launch needs, such as per-member sets. | `DECIDED` | 10-25 | [#9400], [#9437], [#9438], [#9439] | diff --git a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts index 07e6e61bcd1..b3d725a371e 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts @@ -340,6 +340,25 @@ function count(editor: LexicalEditor, kind: Kind): number { }); } +/** The URLs of the wrappers `kind` (a link or an autolink). */ +function urls(editor: LexicalEditor, kind: Kind): string[] { + return editor.read(() => { + const out: string[] = []; + const visit = (node: ElementNode) => { + for (const child of node.getChildren()) { + if ($isElementNode(child)) { + if ($is(kind, child) && $isLinkNode(child)) { + out.push(child.getURL()); + } + visit(child); + } + } + }; + visit($getRoot()); + return out; + }); +} + const CHAINS = chains(); type Shape = {chain: Kind[]; leaf: Leaf; fields: Fields; label: string}; @@ -441,6 +460,198 @@ describe('a wrapper whose text a client deletes after a format over it', () => { ); }); +// A client that formatted over a wrapper (synced) deletes the wrapper's text +// but not a line break or an inline decorator next to it, while the other +// client removes the wrapper, or for a link gives it another URL: the +// removal or the new URL stays on every client (the delete leaves the +// wrapper without text, and its text fields, set by the format while it +// held text and never stored, must not write its value again). +describe('a wrapper one client removes or retargets while the other deletes its text', () => { + const cells = SHAPES.filter( + ({leaf}) => leaf === 'line-break' || leaf === 'text+decorator', + ).flatMap(shape => + shape.chain.flatMap(target => + (target === 'link' || target === 'autolink' + ? (['remove', 'retarget'] as const) + : (['remove'] as const) + ).flatMap(change => + ( + [ + [1, 2], + [2, 1], + ] as [number, number][] + ).map(ids => ({...shape, change, ids, target})), + ), + ), + ); + test.each(cells)( + '$label: $change $target, ids $ids.0,$ids.1', + async ({chain, leaf, fields, change, ids, target}) => { + const seed = seedYState(config, () => $build(chain, leaf, fields)); + const a = client(seed, ids[0]); + const b = client(seed, ids[1]); + await Promise.resolve(); + b.editor.update( + () => { + $selectAll().formatText('bold'); + }, + {discrete: true}, + ); + sync(a, b); + await Promise.resolve(); + a.editor.update( + () => { + if (change === 'remove') { + $unwrap(target); + } else { + const visit = (node: ElementNode) => { + for (const child of node.getChildren()) { + if ($isElementNode(child)) { + if ($is(target, child) && $isLinkNode(child)) { + child.setURL('https://new.test'); + } + visit(child); + } + } + }; + visit($getRoot()); + } + $setSelection(null); + }, + {discrete: true}, + ); + b.editor.update( + () => { + for (const text of $wrappedTexts()) { + text.remove(); + } + $setSelection(null); + }, + {discrete: true}, + ); + await Promise.resolve(); + sync(a, b); + await Promise.resolve(); + const expected = fresh(a.doc); + expect(json(a.editor), 'a = fresh').toEqual(expected); + expect(json(b.editor), 'b = fresh').toEqual(expected); + if (change === 'remove') { + expect(count(a.editor, target), 'a: removed').toBe(0); + expect(count(b.editor, target), 'b: removed').toBe(0); + } else { + for (const editor of [a.editor, b.editor]) { + expect(urls(editor, target), 'retargeted').toEqual([ + 'https://new.test', + ]); + } + } + }, + ); +}); + +// A wrapper with no text (only a line break or an inline decorator) that +// stores text fields of its own: one client appends text into it while the +// other removes it, or gives a link another URL. The append leaves the +// wrapper holding text, so its text fields are no longer written, and its +// value is written again without them over its old content: a removed or +// retargeted link comes back (#9627, the same storage of a textless link's +// text fields in its value). +describe('a textless wrapper with stored text fields that a client appends text into', () => { + const cells = SHAPES.filter( + ({leaf, fields}) => + fields === 'own' && (leaf === 'no-text' || leaf === 'decorator'), + ).flatMap(shape => + shape.chain.flatMap(target => + (target === 'link' || target === 'autolink' + ? (['remove', 'retarget'] as const) + : (['remove'] as const) + ).flatMap(change => + ( + [ + [1, 2], + [2, 1], + ] as [number, number][] + ).map(ids => ({...shape, change, ids, target})), + ), + ), + ); + // The text goes into the innermost inline wrapper the outermost one leads + // to (an overflow is not inline); known where that is the link. + const intoLink = ({chain, target}: {chain: Kind[]; target: Kind}) => { + let i = 0; + while (i + 1 < chain.length && chain[i + 1] !== 'overflow') { + i++; + } + return target === 'link' && chain[i] === 'link'; + }; + for (const [known, group] of [ + [false, cells.filter(c => !intoLink(c))], + [true, cells.filter(intoLink)], + ] as const) { + (known ? test.fails : test).each(group)( + `$label: $change $target, ids $ids.0,$ids.1${known ? ' (#9627)' : ''}`, + async ({chain, leaf, fields, change, ids, target}) => { + const seed = seedYState(config, () => $build(chain, leaf, fields)); + const a = client(seed, ids[0]); + const b = client(seed, ids[1]); + await Promise.resolve(); + a.editor.update( + () => { + if (change === 'remove') { + $unwrap(target); + } else { + const visit = (node: ElementNode) => { + for (const child of node.getChildren()) { + if ($isElementNode(child)) { + if ($is(target, child) && $isLinkNode(child)) { + child.setURL('https://new.test'); + } + visit(child); + } + } + }; + visit($getRoot()); + } + $setSelection(null); + }, + {discrete: true}, + ); + b.editor.update( + () => { + // Into the innermost wrapper. + let node = ($getRoot().getFirstChildOrThrow() as ElementNode) + .getChildren() + .find($isElementNode) as ElementNode; + for ( + let inner = node.getChildren().find($isElementNode); + inner !== undefined && $isElementNode(inner) && inner.isInline(); + inner = node.getChildren().find($isElementNode) + ) { + node = inner as ElementNode; + } + node.append($createTextNode('typed')); + $setSelection(null); + }, + {discrete: true}, + ); + await Promise.resolve(); + sync(a, b); + await Promise.resolve(); + const expected = fresh(a.doc); + expect(json(a.editor), 'a = fresh').toEqual(expected); + expect(json(b.editor), 'b = fresh').toEqual(expected); + if (change === 'remove') { + expect(count(a.editor, target), 'a: removed').toBe(0); + } else { + expect(urls(a.editor, target), 'retargeted').toEqual([ + 'https://new.test', + ]); + } + }, + ); + } +}); + // Removal races: every wrapper of a chain, removed by one client while the // other formats a range covering it, in both id orders. type Race = Shape & {removed: Kind; ids: [number, number]}; diff --git a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts index 15abdba55d2..59d12cca88c 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts @@ -167,3 +167,94 @@ test('wrappers a local delete leaves without text are settled in linear time', a expect(visits[1] / visits[0]).toBeLessThan(3); expect(visits[2] / visits[1]).toBeLessThan(3); }, 60_000); + +// One line of n links, each "wN" and a line break, bolded by the client +// that then deletes every link's text in one update: each link shows the +// text fields stored at its first atom, found in one walk of the line. +// Counted as the items' `right` links followed during the update. +test('links a local delete leaves without text in one line are settled in linear time', async () => { + const reads: number[] = []; + for (const n of [32, 64, 128]) { + const t = await setup(() => { + const paragraph = $createParagraphNode(); + for (let i = 0; i < n; i++) { + paragraph.append( + $createTextNode(' '), + $createLinkNode(`https://link${i}.test`).append( + $createTextNode(`w${i}`), + $createLineBreakNode(), + ), + ); + } + $getRoot().append(paragraph); + }); + try { + t.b.update(() => { + const texts = $getRoot().getAllTextNodes(); + const last = texts[texts.length - 1]; + const selection = texts[0].select(0, 0); + selection.focus.set(last.getKey(), last.getTextContentSize(), 'text'); + selection.formatText('bold'); + $setSelection(null); + }); + await t.sync(); + let count = 0; + const restores: (() => void)[] = []; + const share = [ + ...(t.b.doc as unknown as {share: Map}).share.values(), + ][0] as {_start: {right: unknown} | null}; + for ( + let it = share._start as {right: unknown} | null; + it !== null; + it = it.right as {right: unknown} | null + ) { + const item = it; + const descriptor = Object.getOwnPropertyDescriptor(item, 'right')!; + const value = item.right; + Object.defineProperty(item, 'right', { + configurable: true, + get() { + count++; + return value; + }, + set(v) { + Object.defineProperty(item, 'right', { + ...descriptor, + value: v, + }); + }, + }); + restores.push(() => { + if ( + Object.getOwnPropertyDescriptor(item, 'right')!.get !== undefined + ) { + Object.defineProperty(item, 'right', {...descriptor, value}); + } + }); + } + try { + t.b.update(() => { + for (const text of $getRoot().getAllTextNodes()) { + if (/^w\d+$/.test(text.getTextContent())) { + text.remove(); + } + } + $setSelection(null); + }); + } finally { + for (const restore of restores) { + restore(); + } + } + reads.push(count); + await t.sync(); + await expectConverged(t.peers); + } finally { + for (const p of t.peers) { + p.stop(); + } + } + } + expect(reads[1] / reads[0]).toBeLessThan(3); + expect(reads[2] / reads[1]).toBeLessThan(3); +}, 60_000); diff --git a/packages/lexical-y/src/internal/Binding.ts b/packages/lexical-y/src/internal/Binding.ts index f72fd937bfd..873e6911da4 100644 --- a/packages/lexical-y/src/internal/Binding.ts +++ b/packages/lexical-y/src/internal/Binding.ts @@ -102,6 +102,7 @@ import { $isWrapper, $nodeAttrs, $setTextFields, + type Attrs, canStoreAs, deriveStorageKind, getGroupSpec, @@ -117,6 +118,7 @@ import { storageKindOf, withPins, wrapperFieldsOf, + wrappersFromAttrs, } from './Schema'; import { attrsBefore, @@ -878,7 +880,8 @@ export class YBinding { private storedFormats: { state: EditorState | null; sequences: StoredFormatsMemo; - } = {sequences: new Map(), state: null}; + lines: Map; + } = {lines: new Map(), sequences: new Map(), state: null}; /** This binding's view, for its work (see guard). */ private readonly seer = () => (this.reading ? null : this.unread); /** @@ -1177,21 +1180,29 @@ export class YBinding { $flushSyncAfterUpdate(); } }; - // A local change that leaves a wrapper around a stored element holding - // no text (its own delete of the text in a mark inside a link) does not - // write the wrapper's value, so the text fields this client set while the - // wrapper held text, which Lexical now serializes, are stored nowhere: - // in the same update, the wrapper shows the stored ones, as every other - // client and a fresh reader do (see $unstoredWrapperTextFields), and - // nothing follows the update for an app to answer. The stored element is - // the one the change marks dirty. + // A local change that leaves a wrapper holding no text (a delete of the + // text in a link, or in a mark inside a link) makes Lexical serialize the + // text fields this client set while the wrapper held text, which are + // stored nowhere: in the same update, the wrapper shows the stored ones, + // as every other client and a fresh reader do, and nothing follows the + // update for an app to answer. Otherwise the write would write the + // wrapper's whole value again over the content it keeps (a line break, + // an inline decorator), over a collaborator's concurrent removal or + // change of it. Runs on the element the change marks dirty: a stored + // element inside wrappers (see $unstoredWrapperTextFields), or the + // wrapper itself (its value is the one stored at its first atom). const settleTextFields = (node: LexicalNode) => withPins(this._pins, () => { const state = this.state; + const stored = state.yByKey.has(node.getKey()); if ( this._status !== 'active' || this.unread !== null || - !state.yByKey.has(node.getKey()) || + state.hidden !== null || + !( + stored || + ($isElementNode(node) && node.isInline() && $isWrapper(node)) + ) || $hasUpdateTag(COLLABORATION_TAG) ) { return; @@ -1203,40 +1214,75 @@ export class YBinding { if (memo.state !== editor._pendingEditorState) { memo.state = editor._pendingEditorState; memo.sequences = new Map(); + memo.lines = new Map(); } - const found = $unstoredWrapperTextFields( - state, - [node.getKey()], - type => wrapperFieldsOf(editor, type), - memo.sequences, - ).map( - ([wrapper, json]) => - [ - wrapper, - json, - wrapper.getTextFormat(), - wrapper.getTextStyle(), - ] as const, - ); + const fieldsOf = (type: string) => wrapperFieldsOf(editor, type); + // The wrappers around the stored element (see + // $unstoredWrapperTextFields), or the wrapper itself when its own + // text changed (a delete of the text directly in a link); its value + // is the one stored at its first atom before the update. + const found: (readonly [ElementNode, Attrs | null, number, string])[] = + stored + ? $unstoredWrapperTextFields( + state, + [node.getKey()], + fieldsOf, + memo.sequences, + ).map( + ([wrapper, json]) => + [ + wrapper, + json, + wrapper.getTextFormat(), + wrapper.getTextStyle(), + ] as const, + ) + : (node as ElementNode).shouldSerializeTextStyles() && + ((node as ElementNode).getTextFormat() !== 0 || + (node as ElementNode).getTextStyle() !== '') + ? [ + [ + node as ElementNode, + null, + (node as ElementNode).getTextFormat(), + (node as ElementNode).getTextStyle(), + ] as const, + ] + : []; if (found.length === 0) { return; } // Only a wrapper this update left without text, with the text fields // it had before the update: ones this update set (on a wrapper // without text, or in a transform) are stored by the write. - const stale = editor.getEditorState().read(() => - found.filter(([wrapper, , format, style]) => { + const stale = editor.read('latest', () => + found.flatMap(([wrapper, json, format, style]) => { const was = $getNodeByKey(wrapper.getKey()); - return ( - $isElementNode(was) && - !was.shouldSerializeTextStyles() && - was.getTextFormat() === format && - was.getTextStyle() === style - ); + if ( + !$isElementNode(was) || + was.shouldSerializeTextStyles() || + was.getTextFormat() !== format || + was.getTextStyle() !== style + ) { + return []; + } + if (json !== null) { + return [[wrapper, json] as const]; + } + const value = this.$storedWrapperValue(was, memo.lines); + return value === null ? [] : [[wrapper, value] as const]; }), ); for (const [wrapper, json] of stale) { - $setTextFields(wrapper, json); + const {textFormat, textStyle} = json; + if ( + wrapper.getTextFormat() !== + (typeof textFormat === 'number' ? textFormat : 0) || + wrapper.getTextStyle() !== + (typeof textStyle === 'string' ? textStyle : '') + ) { + $setTextFields(wrapper, json); + } } }); for (const {klass} of this._editor._nodes.values()) { @@ -4388,6 +4434,110 @@ export class YBinding { return found; } + /** + * The value stored for `wrapper` (in the state being read, which the + * document stores) at its first atom: its JSON as wrappersFromAttrs reads + * it, or null. Each line's atoms, and the offsets of the wrappers in it, + * are found once (`lines`, kept for one update). + */ + private $storedWrapperValue( + wrapper: ElementNode, + lines: Map, + ): Attrs | null { + const owner = $ownerOf(wrapper); + if (owner === null) { + return null; + } + let line = lines.get(owner.getKey()); + if (line === undefined) { + line = {atoms: this.storedAtoms(owner), offsets: $wrapperOffsets(owner)}; + lines.set(owner.getKey(), line); + } + const offset = line.offsets.get(wrapper.getKey()); + if (offset === undefined) { + return null; + } + // The atom range holding `offset` (ranges are in order). + const {atoms} = line; + let lo = 0; + let hi = atoms.length - 1; + let at: [number, number, Attrs] | undefined; + while (lo <= hi) { + const mid = (lo + hi) >> 1; + const [from, to] = atoms[mid]; + if (offset < from) { + hi = mid - 1; + } else if (offset >= to) { + lo = mid + 1; + } else { + at = atoms[mid]; + break; + } + } + if (at === undefined) { + return null; + } + const editor = this._editor; + const value = wrappersFromAttrs(at[2], type => + wrapperFieldsOf(editor, type), + ).find(v => v.type === wrapper.getType()); + return value === undefined ? null : value.json; + } + + /** + * The content of the line (or inline container) `owner` as stored: each + * content item's atom range and the formats in effect at it. + */ + private storedAtoms(owner: ElementNode): [number, number, Attrs][] { + const state = this.state; + const unit = state.unitByKey.get(owner.getKey()); + let start: Item | null; + let formats: Map; + if (unit !== undefined && unit.end !== null) { + const left = prevLiveEnd(unit.end, state.hidden); + start = left === null ? unit.cs.y._start : left.right; + formats = new Map(unit.start || []); + } else if (unit !== undefined && unit.kind === 'implicit') { + const units = unit.cs.units; + const idx = units.indexOf(unit); + let left = idx > 0 ? units[idx - 1].end : null; + while (left !== null && !isShownEnd(left, state.hidden)) { + left = prevLiveEnd(left, state.hidden); + } + start = left === null ? unit.cs.y._start : left.right; + formats = new Map(unit.start || []); + } else { + const cs = state.csByKey.get(owner.getKey()); + if (cs === undefined || cs.kind !== 'i') { + return []; + } + start = cs.y._start; + formats = new Map(); + } + const out: [number, number, Attrs][] = []; + let offset = 0; + for (let it = start; it !== null; it = it.right) { + if (!it.deleted && it.content instanceof ContentFormat) { + if (it.content.value === null) { + formats.delete(it.content.key); + } else { + formats.set(it.content.key, it.content.value); + } + continue; + } + const len = shownLength(it, state.hidden); + if (len === 0) { + if (isShownEnd(it, state.hidden)) { + break; + } + continue; + } + out.push([offset, offset + len, Object.fromEntries(formats) as Attrs]); + offset += len; + } + return out; + } + /** Containers a write did not write a follow-up's change to (see refused). */ private noteRefused(writer: Writer, naming: boolean): void { for (const cs of writer.refused) { @@ -6419,6 +6569,30 @@ function $ownerOf(node: LexicalNode): ElementNode | null { return parent; } +/** A line's stored atoms and the offsets of its wrappers (see YBinding). */ +type StoredLine = { + atoms: [number, number, Attrs][]; + offsets: Map; +}; + +/** The atom offset of every wrapper in the content of `owner`, in one pass. */ +function $wrapperOffsets(owner: ElementNode): Map { + const out = new Map(); + let n = 0; + const visit = (el: ElementNode) => { + for (const c of el.getChildren()) { + if ($isWrapper(c)) { + out.set(c.getKey(), n); + visit(c as ElementNode); + } else { + n += $atomSize(c); + } + } + }; + visit(owner); + return out; +} + function $atomsBefore(owner: ElementNode, target: LexicalNode): number { let n = 0; const visit = (el: ElementNode): boolean => { From fd54ca338e4542c980cfc5e2b8acca380ffae2a1 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 22:29:06 -0700 Subject: [PATCH 17/18] [lexical-y] Bug Fix: Keep a link's stored text fields when it gets text, so a concurrent removal or new URL stays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description A link holding only a line break or an inline decorator stores its text fields in its value (Lexical serializes them while it holds no text). One client appended text into it while the other removed it, or gave it another URL (review). On the previous head, the removal or new URL was lost on every client, while main keeps it: - the append left the link holding text, so its text fields were no longer part of the value the binding wants; - the writer wrote the link's value again, without them, over the line break it kept, over the concurrent change. A wrapper that holds text now writes no text fields and removes none stored there either. Its value is marked as such (`isTextHoldingWrapperValue`), and `formatAsStored` keeps the stored text fields of such a value, like members this editor does not own. So nothing is written for the append, and the writer, its inserts and the self-check agree. The reader's reuse check of a wrapper leaves the text fields out. They are stored only while the wrapper holds no text, which its children decide, and they are then shown from the stored value, also for a wrapper whose text fields were the default before (`$unstoredWrapperTextFields` no longer skips those). The 40 cells pinned for #9627 in the previous commit pass, and are plain tests now. D82 and the docs say so. ## Test plan ### Before On b415918e: ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts -t 'appends text into' × link>no-text own: remove link, ids 1,2 (and 39 more: every shape where the text goes into the link itself, remove and retarget, both orders) ``` On main (f59f74d7), the same 220 cells pass. ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsOwnDelete.test.ts packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFieldsCost.test.ts packages/lexical-y/src/__tests__/unit/regressions/BoldAcrossLinkEnd.test.ts packages/lexical-y/src/__tests__/unit/regressions/BoldIntoLinkBehind.test.ts packages/lexical-y/src/__tests__/unit/regressions/LinkRetargetedWhileFormatted.test.ts Tests 3263 passed | 68 expected fail (3331) $ MATRIX_FULL=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 11666 passed | 206 expected fail (11872) $ pnpm exec vitest run --project unit --project scripts-unit Tests 31098 passed | 282 expected fail | 135 skipped (31515) $ pnpm run ci-check Found 0 errors ``` --- .../lexical-website/docs/collaboration/y.md | 3 +- packages/lexical-y/DECISIONS.md | 2 +- .../regressions/WrapperTextFields.test.ts | 141 ++++++++---------- packages/lexical-y/src/internal/Read.ts | 10 +- packages/lexical-y/src/internal/Schema.ts | 18 +++ packages/lexical-y/src/internal/Write.ts | 12 +- 6 files changed, 102 insertions(+), 84 deletions(-) diff --git a/packages/lexical-website/docs/collaboration/y.md b/packages/lexical-website/docs/collaboration/y.md index f5a4eb66c39..509414d0b7b 100644 --- a/packages/lexical-website/docs/collaboration/y.md +++ b/packages/lexical-website/docs/collaboration/y.md @@ -1202,8 +1202,7 @@ not merge as a user might expect: while a collaborator formats text inside it removes the link on every client: a format writes no link while the link holds text. A link with no text (only an inline decorator or a line break) still stores its text - format, and stays on its content, and so does such a link a collaborator - removes, or gives another URL, while you append text into it: + format, and stays on its content: [#9627](https://github.com/meta/lexical/issues/9627). Where a collaborator overwrote a step of yours entirely, the undo manager skips that step as changing nothing, and the same undo also reverts your step before it. When you undo a format change diff --git a/packages/lexical-y/DECISIONS.md b/packages/lexical-y/DECISIONS.md index ad1cb27065f..e583849347e 100644 --- a/packages/lexical-y/DECISIONS.md +++ b/packages/lexical-y/DECISIONS.md @@ -64,7 +64,7 @@ issues and docs that carry it. | D30 | **Diffed rewrites snap to whole words**; keystrokes and IME composition keep the exact diff. Scripts without spaces treat each character as a boundary. **A diffed insertion or deletion without a caret that the diff would put inside a word goes to the first place in the run of equal text it could be in where both its ends are at word edges** (a word deleted with the separator before it); a caret's placement, and one already at word edges, stay. | Concurrent rewrites otherwise split words. Real word segmentation is a follow-up ([#9467]). Without a caret a diff takes the common start first and put deletions inside words, so a collaborator's concurrent delete of the same word took a letter of the next one ([#9636]); the first edge keeps a word typed after a deleted word apart from the next one, as typing goes (a space, then a word). | `DECIDED` | 10-30 | [#9400], [#9467], [#9636] | | D31 | **A list item's nested list gets an item of its own.** `NodeStructureExtension`'s list rule prevents an item holding a nested list next to other content when it is written; the binding does not repair it afterwards. | That shape lost text in outdents, Backspaces and offline merges. Preventing it at write time follows D20. | `DECIDED` | 10-26 | [#9452], [y.md: Limits][limits] | | D32 | **List merging ignores `start`.** | Existing Lexical behaviour, intended. | `DECIDED` | 10-23 | [#9452] | -| D82 | **An inline element's textFormat and textStyle are persisted only while it holds no text at any depth, and the binding stores a wrapper's exactly when Lexical serializes them.** Lexical's `shouldSerializeTextStyles` looks through the elements inside an inline element (a mark in a link), as a reconcile already derives the two from its first text there; a block keeps the direct-children check. Exported JSON changes only for a link, autolink or mark whose children are all elements: those two fields are no longer written (they are still read, into the same state). | Stored while the wrapper holds text, a format change over a link writes the link's value again, and a collaborator's concurrent removal of the link is lost ([#9597]); left unstored while Lexical serializes them, the author's JSON differs from a fresh reader's. One rule, owned by core, keeps both, and the JSON of blocks and of every other node is unchanged (D01). A link with no text at all (only an inline decorator or a line break) still stores them and still comes back ([#9627]), also when a collaborator appends text into it while another removes it or changes its URL (the append drops the stored fields, which writes its value again). A link whose stored value already holds them (written by an earlier build after a format over it) can come back once, until its next write. Accepted with it: a link behaves like a mark in overlapping same-format races ([#9626]), which now reach a link's edge for either client id order and a link's first word; formatting only, no text lost. | `DECIDED` | 10-30 | [#9601] | +| D82 | **An inline element's textFormat and textStyle are persisted only while it holds no text at any depth, and the binding stores a wrapper's exactly when Lexical serializes them.** Lexical's `shouldSerializeTextStyles` looks through the elements inside an inline element (a mark in a link), as a reconcile already derives the two from its first text there; a block keeps the direct-children check. Exported JSON changes only for a link, autolink or mark whose children are all elements: those two fields are no longer written (they are still read, into the same state). | Stored while the wrapper holds text, a format change over a link writes the link's value again, and a collaborator's concurrent removal of the link is lost ([#9597]); left unstored while Lexical serializes them, the author's JSON differs from a fresh reader's. One rule, owned by core, keeps both, and the JSON of blocks and of every other node is unchanged (D01). A link with no text at all (only an inline decorator or a line break) still stores them and still comes back ([#9627]). While a wrapper holds text, its value without text fields does not remove ones stored there, so a link that gets text keeps a concurrent removal or new URL. A link whose stored value already holds them (written by an earlier build after a format over it) can come back once, until its next write. Accepted with it: a link behaves like a mark in overlapping same-format races ([#9626]), which now reach a link's edge for either client id order and a link's first word; formatting only, no text lost. | `DECIDED` | 10-30 | [#9601] | | D70 | **No virtual structure.** The editor does not show repairs that are not stored (padded cells for a short table row, two stored lists drawn as one). The one existing case stays: text left without a line marker shows as a paragraph. | It adds another structure on top of Yjs and the Lexical tree, and the complexity that comes with it. | `DECIDED` | 10-25 | [#9400] | | D72 | **The binding starts at one fixed point, with no further deferral or special-casing.** It starts in `YCollabExtension`'s `afterRegistration`, once every extension has registered and the initial state is applied. An initialization-order bug (a table that exists before its DOM) is fixed in the extension that has it, as [#9501] did. | Deferring the first read further changes when the binding's state exists and still misses code that registers later (legacy table observers, React plugins). | `DECIDED` | 10-28 | [#9501], [#9537] | | D73 | **`@lexical/y` first publishes with its whole launch stack** (the binding, the server API, the playground and per-member sets), with real moves after (D27). The launch stack was chosen on 10-23; moves were moved after it on 10-25. | The first published format should already include what the launch needs, such as per-member sets. | `DECIDED` | 10-25 | [#9400], [#9437], [#9438], [#9439] | diff --git a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts index b3d725a371e..2108d21fc2d 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts @@ -551,11 +551,10 @@ describe('a wrapper one client removes or retargets while the other deletes its // A wrapper with no text (only a line break or an inline decorator) that // stores text fields of its own: one client appends text into it while the -// other removes it, or gives a link another URL. The append leaves the -// wrapper holding text, so its text fields are no longer written, and its -// value is written again without them over its old content: a removed or -// retargeted link comes back (#9627, the same storage of a textless link's -// text fields in its value). +// other removes it, or gives a link another URL. The removal or the new URL +// stays on every client: the append leaves the wrapper holding text, whose +// text fields are not written, and its value without them does not write the +// stored one again over the content it kept. describe('a textless wrapper with stored text fields that a client appends text into', () => { const cells = SHAPES.filter( ({leaf, fields}) => @@ -575,81 +574,67 @@ describe('a textless wrapper with stored text fields that a client appends text ), ), ); - // The text goes into the innermost inline wrapper the outermost one leads - // to (an overflow is not inline); known where that is the link. - const intoLink = ({chain, target}: {chain: Kind[]; target: Kind}) => { - let i = 0; - while (i + 1 < chain.length && chain[i + 1] !== 'overflow') { - i++; - } - return target === 'link' && chain[i] === 'link'; - }; - for (const [known, group] of [ - [false, cells.filter(c => !intoLink(c))], - [true, cells.filter(intoLink)], - ] as const) { - (known ? test.fails : test).each(group)( - `$label: $change $target, ids $ids.0,$ids.1${known ? ' (#9627)' : ''}`, - async ({chain, leaf, fields, change, ids, target}) => { - const seed = seedYState(config, () => $build(chain, leaf, fields)); - const a = client(seed, ids[0]); - const b = client(seed, ids[1]); - await Promise.resolve(); - a.editor.update( - () => { - if (change === 'remove') { - $unwrap(target); - } else { - const visit = (node: ElementNode) => { - for (const child of node.getChildren()) { - if ($isElementNode(child)) { - if ($is(target, child) && $isLinkNode(child)) { - child.setURL('https://new.test'); - } - visit(child); + test.each(cells)( + '$label: $change $target, ids $ids.0,$ids.1', + async ({chain, leaf, fields, change, ids, target}) => { + const seed = seedYState(config, () => $build(chain, leaf, fields)); + const a = client(seed, ids[0]); + const b = client(seed, ids[1]); + await Promise.resolve(); + a.editor.update( + () => { + if (change === 'remove') { + $unwrap(target); + } else { + const visit = (node: ElementNode) => { + for (const child of node.getChildren()) { + if ($isElementNode(child)) { + if ($is(target, child) && $isLinkNode(child)) { + child.setURL('https://new.test'); } + visit(child); } - }; - visit($getRoot()); - } - $setSelection(null); - }, - {discrete: true}, - ); - b.editor.update( - () => { - // Into the innermost wrapper. - let node = ($getRoot().getFirstChildOrThrow() as ElementNode) - .getChildren() - .find($isElementNode) as ElementNode; - for ( - let inner = node.getChildren().find($isElementNode); - inner !== undefined && $isElementNode(inner) && inner.isInline(); - inner = node.getChildren().find($isElementNode) - ) { - node = inner as ElementNode; - } - node.append($createTextNode('typed')); - $setSelection(null); - }, - {discrete: true}, - ); - await Promise.resolve(); - sync(a, b); - await Promise.resolve(); - const expected = fresh(a.doc); - expect(json(a.editor), 'a = fresh').toEqual(expected); - expect(json(b.editor), 'b = fresh').toEqual(expected); - if (change === 'remove') { - expect(count(a.editor, target), 'a: removed').toBe(0); - } else { - expect(urls(a.editor, target), 'retargeted').toEqual([ - 'https://new.test', - ]); - } - }, - ); - } + } + }; + visit($getRoot()); + } + $setSelection(null); + }, + {discrete: true}, + ); + b.editor.update( + () => { + // Into the innermost wrapper. + let node = ($getRoot().getFirstChildOrThrow() as ElementNode) + .getChildren() + .find($isElementNode) as ElementNode; + for ( + let inner = node.getChildren().find($isElementNode); + inner !== undefined && $isElementNode(inner) && inner.isInline(); + inner = node.getChildren().find($isElementNode) + ) { + node = inner as ElementNode; + } + node.append($createTextNode('typed')); + $setSelection(null); + }, + {discrete: true}, + ); + await Promise.resolve(); + sync(a, b); + await Promise.resolve(); + const expected = fresh(a.doc); + expect(json(a.editor), 'a = fresh').toEqual(expected); + expect(json(b.editor), 'b = fresh').toEqual(expected); + if (change === 'remove') { + expect(count(a.editor, target), 'a: removed').toBe(0); + } else { + expect(urls(a.editor, target), 'retargeted').toEqual([ + 'https://new.test', + ]); + } + }, + ); }); // Removal races: every wrapper of a chain, removed by one client while the diff --git a/packages/lexical-y/src/internal/Read.ts b/packages/lexical-y/src/internal/Read.ts index fa4afabf014..30203cf2614 100644 --- a/packages/lexical-y/src/internal/Read.ts +++ b/packages/lexical-y/src/internal/Read.ts @@ -2893,9 +2893,16 @@ export class Reader { const own: Attrs = {}; $wrapperAttrs(reused, own); const current = wrappersFromAttrs(own, this.wrapperFields)[0]; + // Without the text fields: they are stored only while the wrapper + // holds no text, which its children, set after this, decide, and are + // shown from the stored value then (see $readInline's `finish`). + const sansText = (json: Attrs) => { + const {textFormat: _f, textStyle: _s, ...rest} = json; + return stableStringify(rest); + }; if ( current === undefined || - stableStringify(current.json) !== stableStringify(w.json) + sansText(current.json) !== sansText(w.json) ) { const spec = getWrapperSpec(klassOf(reused))!; const json = $schemaJSON(reused); @@ -3741,7 +3748,6 @@ export function $unstoredWrapperTextFields( anchor._item !== null && !seen.has(w.getKey()) && $isWrapper(w) && - (w.getTextFormat() !== 0 || w.getTextStyle() !== '') && w.shouldSerializeTextStyles() ) { seen.add(w.getKey()); diff --git a/packages/lexical-y/src/internal/Schema.ts b/packages/lexical-y/src/internal/Schema.ts index e84509bfd96..bec9ad6548f 100644 --- a/packages/lexical-y/src/internal/Schema.ts +++ b/packages/lexical-y/src/internal/Schema.ts @@ -1983,6 +1983,21 @@ const INSTANCE_KEY = '#'; * format items: concurrent changes to two fields of one wrapper do not merge * (the last one wins). */ +/** + * Wrapper values (see $wrapperAttrs) of wrappers that hold text: their text + * fields are not written (Lexical does not serialize them), so a value + * without them does not remove text fields stored there (see + * Writer#formatAsStored). + */ +const textHoldingValues = new WeakSet(); + +/** Whether `value` is a wrapper value of a wrapper that holds text. */ +export function isTextHoldingWrapperValue(value: unknown): boolean { + return ( + value !== null && typeof value === 'object' && textHoldingValues.has(value) + ); +} + export function $wrapperAttrs( node: ElementNode, out: Attrs, @@ -2025,6 +2040,9 @@ export function $wrapperAttrs( if (instance !== null) { own[INSTANCE_KEY] = instance; } + if (!latest.shouldSerializeTextStyles()) { + textHoldingValues.add(own); + } // The innermost wrapper of a type wins. out[WRAPPER_PREFIX + node.getType()] = own; } diff --git a/packages/lexical-y/src/internal/Write.ts b/packages/lexical-y/src/internal/Write.ts index 2f316e73640..11ea98c9582 100644 --- a/packages/lexical-y/src/internal/Write.ts +++ b/packages/lexical-y/src/internal/Write.ts @@ -73,6 +73,7 @@ import { INLINE_PREFIX, isMergeableText, isTextFieldKey, + isTextHoldingWrapperValue, KIND_KEY, klassOf, LEVEL_KEY, @@ -3735,7 +3736,16 @@ export class Writer { ) { return value; } - const owns = (m: string) => ownsMember(m, fields); + // A wrapper that holds text writes no text fields (Lexical does not + // serialize them), and does not remove ones stored there either: when a + // wrapper with no text that stores them gets text (a line break in a + // link, then text appended), its value is not written again over the + // content it kept, over a collaborator's concurrent removal or change + // of it (see $wrapperAttrs). + const keepsText = isTextHoldingWrapperValue(value); + const owns = (m: string) => + ownsMember(m, fields) && + !(keepsText && (m === 'textFormat' || m === 'textStyle')); if (same) { const a = value as Attrs; const b = stored as Attrs; From f12eb519a66feeb2f8629f36f1478edd3e39585f Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Fri, 30 Oct 2026 23:03:22 -0700 Subject: [PATCH 18/18] [lexical-clipboard][lexical-y] Chore: Name @lexical/clipboard in the changeset and put two comments back in place ## Description Review suggestions, with no logic change: - The `lexical` changeset also names `@lexical/clipboard`, whose `$generateJSONFromSelectedNodes` this pull request changes. - In `Schema.ts`: - `$wrapperAttrs`'s doc comment is back above it (the text-holding value block sat between them); - `$putTextFields`'s comment says wrapper items carry the text fields only while the wrapper holds no text. ## Test plan ### Before The changeset named `lexical` only. ### After ``` $ pnpm run ci-check Found 0 errors ``` --- .../lexical-inline-element-text-fields.md | 1 + packages/lexical-y/src/internal/Schema.ts | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.changeset/lexical-inline-element-text-fields.md b/.changeset/lexical-inline-element-text-fields.md index 3b8565033b3..c47abf0d40d 100644 --- a/.changeset/lexical-inline-element-text-fields.md +++ b/.changeset/lexical-inline-element-text-fields.md @@ -1,5 +1,6 @@ --- 'lexical': patch +'@lexical/clipboard': patch --- [fix] Stop serializing the textFormat and textStyle of an inline element whose text sits in another element inside it diff --git a/packages/lexical-y/src/internal/Schema.ts b/packages/lexical-y/src/internal/Schema.ts index bec9ad6548f..1e589c5db52 100644 --- a/packages/lexical-y/src/internal/Schema.ts +++ b/packages/lexical-y/src/internal/Schema.ts @@ -1975,14 +1975,6 @@ export function eachOuterLevel( */ const INSTANCE_KEY = '#'; -/** - * The format of one wrapper: one key, `w:`, whose value is a flat - * object of its schema fields and NodeState (`s:`); a value that is not - * a primitive, or a string that starts with NUL, is NUL and its JSON, since - * Yjs compares format values one level deep. One key keeps a link at two - * format items: concurrent changes to two fields of one wrapper do not merge - * (the last one wins). - */ /** * Wrapper values (see $wrapperAttrs) of wrappers that hold text: their text * fields are not written (Lexical does not serialize them), so a value @@ -1998,6 +1990,14 @@ export function isTextHoldingWrapperValue(value: unknown): boolean { ); } +/** + * The format of one wrapper: one key, `w:`, whose value is a flat + * object of its schema fields and NodeState (`s:`); a value that is not + * a primitive, or a string that starts with NUL, is NUL and its JSON, since + * Yjs compares format values one level deep. One key keeps a link at two + * format items: concurrent changes to two fields of one wrapper do not merge + * (the last one wins). + */ export function $wrapperAttrs( node: ElementNode, out: Attrs, @@ -2173,7 +2173,8 @@ export function $lineAttrs( /** * An element's textFormat/textStyle, which Lexical serializes only while * the element has no text: every peer must hold the same values, so groups - * and wrapper items always carry them (as lines do). + * and lines always carry them, and wrapper items while the wrapper holds no + * text (see $wrapperAttrs). */ function $putTextFields(node: ElementNode, out: Attrs): void { const tf = node.getTextFormat();