Repository navigation
[lexical-y] Bug Fix: A run ends in its own container, and no item is linked next to an item of another container #9693
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@lexical/y': patch | ||
| --- | ||
|
|
||
| [fix] A format write never links an item into another container: an editor no longer stops matching its own document after a style change next to a collaborator's run in a table cell | ||
|
|
||
| Verify: in a document with a table, one client colors the end of a cell's text, clears the color of one character there, then colors a word in the paragraph before the table; after syncing, the other client recolors part of that cell. Both editors keep matching their documents through later edits and syncs, and no `@lexical/y/recovered-write` warning is reported. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,201 @@ | ||
| /** | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| * | ||
| */ | ||
|
|
||
| // @vitest-environment node | ||
|
|
||
| // Two clients color ranges of a document with a table (its cells are | ||
| // containers of their own) and sync now and then. After every step, no item | ||
| // of either document may have a neighbour in another container (see | ||
| // misLinkedItems): a write that ended a run after its client's item in | ||
| // another container linked its item into the wrong list | ||
| // (regressions/CrossContainerEnd.test.ts). Seeds: LEXICAL_Y_STRUCTURE_SEEDS | ||
| // (default 200) from LEXICAL_Y_STRUCTURE_FIRST (default 1); LEXICAL_Y_GC=1 | ||
| // for garbage collection on. | ||
|
|
||
| import {buildEditorFromExtensions} from '@lexical/extension'; | ||
| import {MarkStateExtension} from '@lexical/mark'; | ||
| import {RichTextExtension} from '@lexical/rich-text'; | ||
| import {$patchStyleText} from '@lexical/selection'; | ||
| import { | ||
| $createTableCellNode, | ||
| $createTableNode, | ||
| $createTableRowNode, | ||
| TableExtension, | ||
| } from '@lexical/table'; | ||
| import * as Y from '@y/y'; | ||
| import { | ||
| $createParagraphNode, | ||
| $createRangeSelection, | ||
| $createTextNode, | ||
| $getRoot, | ||
| $isElementNode, | ||
| $isParagraphNode, | ||
| $setSelection, | ||
| configExtension, | ||
| defineExtension, | ||
| type ElementNode, | ||
| type LexicalNode, | ||
| } from 'lexical'; | ||
| import {expect, test} from 'vitest'; | ||
|
|
||
| import {createYDoc, YCollabExtension} from '../..'; | ||
| import {seedYState} from '../../headless'; | ||
| import {misLinkedItems} from '../utils/structure'; | ||
|
|
||
| const GC = process.env.LEXICAL_Y_GC === '1'; | ||
| const SEEDS = Number(process.env.LEXICAL_Y_STRUCTURE_SEEDS ?? 200); | ||
| const FIRST = Number(process.env.LEXICAL_Y_STRUCTURE_FIRST ?? 1); | ||
| const STEPS = 40; | ||
| const EXTENSIONS = [RichTextExtension, TableExtension, MarkStateExtension]; | ||
| const COLORS = ['#c00', '#0a0', '#00c']; | ||
|
|
||
| type Op = | ||
| | {k: 'sync'} | ||
| | {k: 'color'; e: number; p: number; a: number; b: number; v: number}; | ||
|
|
||
| function rng(seed: number) { | ||
| let s = seed >>> 0 || 1; | ||
| return () => { | ||
| s ^= s << 13; | ||
| s ^= s >>> 17; | ||
| s ^= s << 5; | ||
| return (s >>> 0) / 4294967296; | ||
| }; | ||
| } | ||
|
|
||
| const SEEDED = seedYState({extensions: EXTENSIONS, schemaVersion: 1}, () => { | ||
| const $cell = (text: string) => | ||
| $createTableCellNode(0).append( | ||
| $createParagraphNode().append($createTextNode(text)), | ||
| ); | ||
| $getRoot() | ||
| .clear() | ||
| .append( | ||
| $createParagraphNode().append($createTextNode('alpha beta gamma')), | ||
| $createTableNode().append( | ||
| $createTableRowNode().append( | ||
| $cell('one two three'), | ||
| $cell('four five six'), | ||
| ), | ||
| ), | ||
| $createParagraphNode().append($createTextNode('delta epsilon zeta')), | ||
| ); | ||
| }); | ||
|
|
||
| function $paragraphs(): ElementNode[] { | ||
| const out: ElementNode[] = []; | ||
| const walk = (n: LexicalNode) => { | ||
| if ($isParagraphNode(n)) { | ||
| out.push(n); | ||
| } else if ($isElementNode(n)) { | ||
| n.getChildren().forEach(walk); | ||
| } | ||
| }; | ||
| walk($getRoot()); | ||
| return out; | ||
| } | ||
|
|
||
| function $color(op: Extract<Op, {k: 'color'}>) { | ||
| const paragraphs = $paragraphs(); | ||
| const texts = paragraphs[op.p % paragraphs.length].getAllTextNodes(); | ||
| const total = texts.reduce((n, t) => n + t.getTextContentSize(), 0); | ||
| if (total === 0) { | ||
| return; | ||
| } | ||
| const at = (offset: number) => { | ||
| let left = Math.max(0, Math.min(offset, total)); | ||
| for (const t of texts) { | ||
| const n = t.getTextContentSize(); | ||
| if (left <= n) { | ||
| return {o: left, t}; | ||
| } | ||
| left -= n; | ||
| } | ||
| const last = texts[texts.length - 1]; | ||
| return {o: last.getTextContentSize(), t: last}; | ||
| }; | ||
| const from = Math.min(op.a, total - 1); | ||
| const x = at(from); | ||
| const y = at(op.b >= 99 ? total : Math.min(total, from + op.b)); | ||
| const selection = $createRangeSelection(); | ||
| selection.anchor.set(x.t.getKey(), x.o, 'text'); | ||
| selection.focus.set(y.t.getKey(), y.o, 'text'); | ||
| $setSelection(selection); | ||
| $patchStyleText(selection, {color: op.v < 3 ? COLORS[op.v] : null}); | ||
| $setSelection(null); | ||
| } | ||
|
|
||
| function gen(seed: number): Op[] { | ||
| const r = rng(seed * 2654435761 + 5); | ||
| const ops: Op[] = []; | ||
| for (let s = 0; s < STEPS; s++) { | ||
| ops.push({ | ||
| a: Math.floor(r() * 14), | ||
| b: r() < 0.35 ? 99 : 1 + Math.floor(r() * 8), | ||
| e: Math.floor(r() * 2), | ||
| k: 'color', | ||
| p: Math.floor(r() * 4), | ||
| v: Math.floor(r() * 4), | ||
| }); | ||
| if (r() < 0.6) { | ||
| ops.push({k: 'sync'}); | ||
| } | ||
| } | ||
| return ops; | ||
| } | ||
|
|
||
| for (let seed = FIRST; seed < FIRST + SEEDS; seed++) { | ||
| test(`seed ${seed}: no item has a neighbour in another container (gc ${GC ? 'on' : 'off'})`, () => { | ||
| const errors: unknown[] = []; | ||
| const docs = [0, 1].map(i => { | ||
| const doc = createYDoc(null, {gc: GC}); | ||
| doc.clientID = 1001 + i; | ||
| Y.applyUpdate(doc, SEEDED); | ||
| return doc; | ||
| }); | ||
| const editors = docs.map((doc, i) => | ||
| buildEditorFromExtensions( | ||
| defineExtension({ | ||
| dependencies: [ | ||
| ...EXTENSIONS, | ||
| configExtension(YCollabExtension, { | ||
| doc, | ||
| schemaVersion: 1, | ||
| synced: true, | ||
| }), | ||
| ], | ||
| name: `structure/${i}`, | ||
| onError: e => errors.push(e), | ||
| }), | ||
| ), | ||
| ); | ||
| try { | ||
| for (const [i, op] of gen(seed).entries()) { | ||
| if (op.k === 'sync') { | ||
| Y.applyUpdate( | ||
| docs[1], | ||
| Y.encodeStateAsUpdate(docs[0], Y.encodeStateVector(docs[1])), | ||
| ); | ||
| Y.applyUpdate( | ||
| docs[0], | ||
| Y.encodeStateAsUpdate(docs[1], Y.encodeStateVector(docs[0])), | ||
| ); | ||
| } else { | ||
| editors[op.e].update(() => $color(op), {discrete: true}); | ||
| } | ||
| for (const doc of docs) { | ||
| expect(misLinkedItems(doc), `after step ${i}`).toEqual([]); | ||
| } | ||
| } | ||
| expect(errors).toEqual([]); | ||
|
There was a problem hiding this comment. Optional: with
Member
Author
There was a problem hiding this comment. Done in #9696 (096f779): the fuzz records the editors' warnings and expects none after each step. With the same-type filter in |
||
| } finally { | ||
| editors.forEach(e => e.dispose()); | ||
| docs.forEach(d => d.destroy()); | ||
| } | ||
| }); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| /** | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| * | ||
| */ | ||
|
|
||
| // @vitest-environment node | ||
|
|
||
| // A format write that restates another client's run at the end of its range | ||
| // gives that run an end of its own, after the end that client's change gave | ||
| // it (see changeEnd). That client's next item of the key, in its clock order, | ||
| // can be in another container: here A colors text in a table cell and then | ||
| // in the paragraph before the table, and B recolors part of the cell. B's | ||
| // write linked its end after A's item in the root while naming the cell as | ||
| // its parent: B's own document held an item in the wrong list (every other | ||
| // client placed it in the cell), and later changes next to it made B's | ||
| // editor stop reading part of the document, for good. A run cannot end in | ||
| // another container: changeEnd only looks in the run's own, and no item is | ||
| // written next to an item of another container. | ||
|
|
||
| import {$patchStyleText} from '@lexical/selection'; | ||
| import { | ||
| $createTableCellNode, | ||
| $createTableNode, | ||
| $createTableRowNode, | ||
| } from '@lexical/table'; | ||
| import { | ||
| $createParagraphNode, | ||
| $createRangeSelection, | ||
| $createTextNode, | ||
| $getRoot, | ||
| $isElementNode, | ||
| $isParagraphNode, | ||
| $setSelection, | ||
| type ElementNode, | ||
| type LexicalNode, | ||
| } from 'lexical'; | ||
| import {describe, expect, test} from 'vitest'; | ||
|
|
||
| import {expectConverged, setup} from '../../utils/harness'; | ||
| import {misLinkedItems} from '../../utils/structure'; | ||
|
|
||
| function $paragraphs(): ElementNode[] { | ||
| const out: ElementNode[] = []; | ||
| const walk = (n: LexicalNode) => { | ||
| if ($isParagraphNode(n)) { | ||
| out.push(n); | ||
| } else if ($isElementNode(n)) { | ||
| n.getChildren().forEach(walk); | ||
| } | ||
| }; | ||
| walk($getRoot()); | ||
| return out; | ||
| } | ||
|
|
||
| /** | ||
| * Color offsets `from` to `to` of paragraph `p` (0: the first paragraph, 1 | ||
| * and 2: the cells, 3: the last paragraph); null clears the color. | ||
| */ | ||
| function $color( | ||
| p: number, | ||
| from: number, | ||
| to: number | 'end', | ||
| color: string | null, | ||
| ) { | ||
| const texts = $paragraphs()[p].getAllTextNodes(); | ||
| const total = texts.reduce((n, t) => n + t.getTextContentSize(), 0); | ||
| const at = (offset: number) => { | ||
| let left = offset; | ||
| for (const t of texts) { | ||
| const n = t.getTextContentSize(); | ||
| if (left <= n) { | ||
| return {o: left, t}; | ||
| } | ||
| left -= n; | ||
| } | ||
| const last = texts[texts.length - 1]; | ||
| return {o: last.getTextContentSize(), t: last}; | ||
| }; | ||
| const x = at(from); | ||
| const y = at(to === 'end' ? total : to); | ||
| const selection = $createRangeSelection(); | ||
| selection.anchor.set(x.t.getKey(), x.o, 'text'); | ||
| selection.focus.set(y.t.getKey(), y.o, 'text'); | ||
| $setSelection(selection); | ||
| $patchStyleText(selection, {color}); | ||
| $setSelection(null); | ||
| } | ||
|
|
||
| describe('a style write next to a run whose client wrote in another container', () => { | ||
| for (const ids of [ | ||
| [1, 2], | ||
| [2, 1], | ||
| ]) { | ||
| test(`links no item into another container (ids ${ids})`, async () => { | ||
| const $cell = (text: string) => | ||
| $createTableCellNode(0).append( | ||
| $createParagraphNode().append($createTextNode(text)), | ||
| ); | ||
| const t = await setup( | ||
| () => | ||
| $getRoot().append( | ||
| $createParagraphNode().append($createTextNode('alpha beta gamma')), | ||
| $createTableNode().append( | ||
| $createTableRowNode().append( | ||
| $cell('one two three'), | ||
| $cell('four five six'), | ||
| ), | ||
| ), | ||
| $createParagraphNode().append( | ||
| $createTextNode('delta epsilon zeta'), | ||
| ), | ||
| ), | ||
| {ids}, | ||
| ); | ||
| t.a.update(() => $color(2, 1, 'end', '#00c')); | ||
| t.a.update(() => $color(2, 7, 8, null)); | ||
| t.a.update(() => $color(0, 13, 17, '#00c')); | ||
| await t.sync(); | ||
| t.b.update(() => $color(2, 4, 8, '#0a0')); | ||
| for (const peer of t.peers) { | ||
| expect(misLinkedItems(peer.doc)).toEqual([]); | ||
| expect(peer.warnings).toEqual([]); | ||
| } | ||
| await t.sync(); | ||
| await expectConverged(t.peers); | ||
| }); | ||
| } | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional: peers don't integrate the stray end into the cell. An item with an origin is encoded without its parent, and
@y/ytakes the parent from the origin item on receipt, so every other client puts it in the root's list, after the author's own null, where it changes nothing. Only the writer's copy names the cell.utils/structure.ts:15-16and #9694 already say this; the same sentence is inCursor.ts:150-151andCrossContainerEnd.test.ts:17-18.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, you are right: peers take the parent from the origin and put the item in the root, where it changes nothing. Fixed in D96,
checkLinkable's comment and theCrossContainerEndheader, in #9696 (096f779) to keep this head stable.