Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
7fa612e to
740f9d6
Compare
740f9d6 to
218a4bf
Compare
218a4bf to
4616877
Compare
4616877 to
b6ea70a
Compare
b6ea70a to
1781c5e
Compare
1781c5e to
c6b99ca
Compare
c6b99ca to
16df52d
Compare
16df52d to
8fd16a3
Compare
8fd16a3 to
285b54d
Compare
285b54d to
1b3e981
Compare
1b3e981 to
515feab
Compare
515feab to
0460122
Compare
0460122 to
d0625c6
Compare
…t nodes made again with other text
## Description
When an update removes a line's text nodes and makes new ones in their place with some of the text changed (an agent's rewrite of a block from its JSON with one word changed, a code highlighter's new tokens after a paste), `@lexical/y` wrote every character of a removed node whose text was not all there again. A removed node's text was aligned with what replaced it whole or not at all (`TextReplay#stretch`, invariant 4). A collaborator's letter typed at the same time into an unchanged word then stayed in front of the rewritten text, out of its word, and a character a collaborator deleted there at the same time came back:
- `t63 t71z t13z t69z t15z`, made again as one node with `t81z` for `t69z`, while a collaborator typed `Q` after `t63`, gave `Qt63 t71z t13z t81z t15z`;
- the same line while a collaborator deleted the `7` of `t71z` gave the `7` back.
Now the text of nodes made in a stretch also keeps a removed node's text where it continues the common start or end in part. One more step runs after the existing alignment wherever a stretch has text of nodes made and text of removed nodes. It only fills gaps between what the existing passes keep, it counts whole words (a new `diffWords` in `Inline.ts`: a run of letters, digits and `_`, or any single other character, compared whole), and every run it adds to has to start and end at word boundaries of both whole lines: the new line as it is after the update (text made and deleted again left out) and the old line (for text that came from another line, that line); and so must every part of such a run whose old characters follow each other in one old line (text typed exactly between them joins them, text typed anywhere else does not), where it holds an added character (so removed text from two places, read as one, makes no piece of an old word look whole and joins no new word). A word or character is whatever goes with it: its marks, a joiner or stacking sign and what it joins, an emoji's parts (the platform's grapheme segmenter, and these rules on top of it); a word reads through a soft hyphen, a word joiner and an inline node. Where the platform has no grapheme segmenter (`Intl.Segmenter`), the step does not run and the line is written as before.
- a word that gained or lost letters (`t1z` made again as `t91z`) is a new word, so a collaborator's concurrent replacement of the old word takes no letter from the new one;
- `one` made again right before a set `two` (`onetwo`), `one` of an old `onex` whose `x` was a removed node of its own, and `red` of an old `foored` made again inside a new `ored` are new words too;
- text pasted before a word that starts with the paste's first letter (`t68z ` before `t50z` in a highlighted code line) no longer writes that letter again;
- removed text that the made text holds twice is kept only where the whole words around it are there once: a piece of a word a highlighter split off (` t` of `t49z`) keeps its characters next to pasted tokens that hold ` t` too, but words an app made twice keep none.
A text set (`setTextContent`) that adds letters inside a word still writes them as typed: `t1z` set to `t91z` while a collaborator replaces `t1z` with `t94z` gives `9t94z`. That is unchanged here and kept as a known failure in the new tests.
## Changeset
`.changeset/lexical-y-keep-words-of-remade-text.md`: `[fix]` for `@lexical/y`, with a `Verify:` paragraph.
## Test plan
New cases in `packages/lexical-y/src/__tests__/unit/regressions/TextIdentity.test.ts` (S9-7): one node and three nodes made again with one word changed, while a peer types into or deletes from an unchanged word; a removed node that shares no text before one that holds all of it; a piece of a word made again after text that holds it too; a paste before a word that starts with the paste's first letter; a word made again as another word that holds its letters while a peer replaces the old word, also while the editor composes in another line; a made word joined to the set word after it; removed words made again twice; a word whose first or last letter was a removed node of its own; a removed node that was one word with the node before it; a made word whose separator was deleted again; an Arabic word joined by a ZWJ, an emoji sequence, a kana with a combining mark, Khmer and Myanmar conjuncts and Thai and Lao consonants with AM whose first character changed, and a Khmer conjunct whose stacked consonant changed; a Khmer conjunct across an unchanged node, and one whose middle is an unchanged node; removed words made again twice after a different word; a word with a soft hyphen or a word joiner whose first syllable changed; a word that went on into a mark; removed text of two lines read as one (a word and an emoji sequence, two whole words joined into one new word, after text typed and removed, between letters main keeps, and a whole old word before or after text typed and removed); text typed in another line, or elsewhere in the same line, and removed between old letters of a word; without a grapheme segmenter, a stacking sign joins across the marks after it and a halfwidth kana keeps its sound mark, and text of nodes made keeps no removed text in part. The S7-39 cases now expect only the changed word to be written. `packages/lexical-y/src/__tests__/unit/WordDiff.test.ts` checks the word diff against a walk from the ends on random texts.
### Before
```
$ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/TextIdentity.test.ts -t "S9-7|S7-39"
Tests 25 failed | 65 passed | 2 expected fail | 131 skipped (223)
AssertionError: expected { del: 't63z ', ins: 't80z ' } to deeply equal { del: 't63z', ins: 't80z' }
AssertionError: expected [ 'Qt63 t71z t13z t81z t15z', …(2) ] to deeply equal [ 't63Q t71z t13z t81z t15z', …(2) ]
AssertionError: expected [ 't63 t71z t13z t81z t15z', …(2) ] to deeply equal [ 't63 t1z t13z t81z t15z', …(2) ]
AssertionError: expected [ 't12z Qt13z t81z t15z', …(2) ] to deeply equal [ 't12z t1Q3z t81z t15z', …(2) ]
AssertionError: expected [ 'one two X', 'one two X', …(1) ] to deeply equal [ 'oe two X', 'oe two X', 'oe two X' ]
AssertionError: expected [ 't48zZQ t78z t49z t50z', …(2) ] to deeply equal [ 't48zQ t78z tZ49z t50z', …(2) ]
```
### After
```
$ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/TextIdentity.test.ts -t "S9-7|S7-39"
Tests 90 passed | 2 expected fail | 131 skipped (223)
$ pnpm vitest run --project unit packages/lexical-y
Tests 7568 passed | 48 expected fail | 4 skipped (7620)
```
d0625c6 to
2902d5a
Compare
There was a problem hiding this comment.
Thanks for the thorough write-up and the regression matrix. Keeping the unchanged words of text nodes made again fixes a real merge problem for agent rewrites: the new S9-7 rows and the updated S7-39 expectations fail on main and pass here, and a collaborator's concurrent letter or delete inside an unchanged word now stays where it was made. Two agents rewriting different words of one line at once now merge (alpha BETA gamma DELTA) instead of leaving both lines one after the other.
Defect: a removed node's edge space is kept for the rewrite's new space
In the made-text pass of TextReplay#stretch, the start and end walks pass over a removed node that shares nothing at that edge and then take the next removed node when only its edge character matches (packages/lexical-y/src/internal/TextEdits.ts:1666-1696). keepWords then reads that node's text right next to what the existing passes kept, and a lone space or punctuation mark is a word of its own under wordCut (packages/lexical-y/src/internal/Inline.ts:726-761), so the whole and islands checks (TextEdits.ts:1933-2036) accept it. The space the rewrite puts at a new junction inherits a space from a different junction of the old line, and a concurrent edit of that old space glues or interleaves the rewritten words, which main does not do:
| A makes the line again as one text node; B concurrently | main |
this PR |
|---|---|---|
Note: + un + believable results as truly believable results; B deletes Note: |
truly believable results |
trulybelievable results |
the same; B replaces Note: with Tip: |
Tip: truly believable results |
trulyTip: believable results |
the build + ing + (draft) as the build is green; B deletes (draft) |
the build is green |
the buildis green |
Step 1: + fix + the build. as Run the build.; B deletes Step 1: |
Run the build. |
Runthe build. |
Every peer and a fresh reader converge in both client orders, so the self-check does not flag it, yet two words the rewrite wrote end as one. The space after Note: was followed by un, which the rewrite deleted, not by believable; one direction is to refuse an added run whose neighbour in the made text is kept text that was not its neighbour in the old line, keeping rows such as "a removed node with none of the new text before one with all of it" passing. A regression row per shape above, in the S9-7 style, would pin it.
Open design question (a maintainer decision before this leaves draft)
The PR relaxes invariant 4 for the text of nodes made in place of removed ones, with a word that is a run of letters, digits and _, or one other character (wordCut), finer than the non-space word snapToWords replaces whole for sets. A punctuated token made again with one piece changed keeps its unchanged pieces, so a collaborator's concurrent replacement of the old token leaves a mix:
| Made again (collaborator's concurrent replacement of the old token) | main |
this PR |
|---|---|---|
pi is 3.14 ok with 3.15 (2.71) |
pi is 2.713.15 ok |
pi is 2.7115 ok |
I don't know with won't (do not) |
I do notwon't know |
I wondo not know |
use v1.2.3 now with v1.2.4 (v2.0.0) |
use v2.0.0v1.2.4 now |
use v2.0.04 now |
Should made text keep removed text in part, and if so with the letter-run word or with the non-space word? Relaxing the rule also means that a collaborator who deletes the whole old line while an agent rewrites one word now leaves only the changed word (t63 t71z t13z t69z t15z made again with t81z ends as t81z, where main keeps the rewritten line). That is the merge of typing the word in place, and worth a sentence in y.md.
Smaller suggestions
- Name the word unit in the docs.
packages/lexical-website/docs/collaboration/y.md(lines 676-681) says made text keeps the "whole words" it shares, while line 686 uses "whole words" for the non-space unit; once the unit is settled, name it there and in the changeset'sVerify:paragraph (CONTRIBUTING.md asks that paragraph to say what to test). - Test nit.
vi.stubGlobal('Intl', {...Intl, Segmenter: undefined})(packages/lexical-y/src/__tests__/unit/regressions/TextIdentity.test.ts:2575,:2623) copies none ofIntl's members, which are not enumerable;Object.assign(Object.create(Intl), {Segmenter: undefined})removes only the segmenter.
Making a long text node again costs about 4x main (about 8 ms against 2 ms at 20,000 characters, 20 ms against 4 ms at 64,000), while typing costs the same.
| for (let k = 0; k < left.length && j < s.length; ) { | ||
| const [g, t] = group(k, false); | ||
| const m = same(t, j, s.length, false); | ||
| if (m > 0 || left[k].from === null) { | ||
| if (k === twice && !once(j, j + m)) { | ||
| break; | ||
| } | ||
| more.fill(0, k, g); | ||
| lo = g; | ||
| j += m; | ||
| if (m < t.length) { | ||
| break; | ||
| } | ||
| } | ||
| k = g; | ||
| } | ||
| for (let e = left.length, i = s.length; e > lo && i > j; ) { | ||
| const [g, t] = group(e, true); | ||
| const m = same(t, i, j, true); | ||
| if (m > 0 || left[e - 1].from === null) { | ||
| if (g === twice && !once(i - m, i)) { | ||
| break; | ||
| } | ||
| more.fill(0, g, e); | ||
| i -= m; | ||
| if (m < t.length) { | ||
| break; | ||
| } | ||
| } | ||
| e = g; |
There was a problem hiding this comment.
These walks pass over a removed node that shares nothing here and then take the next one when only its edge character matches. With a lone space, keepWords accepts it (a space is a word of its own under wordCut), so the rewrite's new space inherits an old space from another junction: Note: + un + believable results made again as truly believable results ends as trulybelievable results when a collaborator concurrently deletes Note: (main: truly believable results); with (draft) after the build + ing the start walk does the same. The space after Note: was followed by un, which this rewrite deleted, not by believable.
|
Superseded by #9598, which replaces this draft with y-prosemirror's character rule: the text of nodes made again keeps the common start and end it shares with the line's old text, by character and as the same Yjs items, and never keeps less than main does, so a collaborator's delete in an unchanged word cannot come back. The rows from this PR's review ( |
Important
Design proposal, draft: needs Gerard's decision. It lifts a deliberate rule of the
@lexical/ywriter (a removed text node's text is aligned with what replaced it whole or not at all). Please do not merge before that decision.Description
When an update removes a line's text nodes and makes new ones in their place with some of the text changed,
@lexical/ywrote every character of each removed node whose text was not all there again. Agents do this when they rewrite a block from its JSON with one word changed, and code highlighters do it when they retokenize after a paste. A collaborator's letter typed at the same time into an unchanged word then showed up in front of the rewritten text, out of its word. A character a collaborator deleted there at the same time came back, and a comment or format they added there was dropped.TextReplay#stretchaligns, between characters that stayed in place, the text that left (removed nodes, the old text asetTextContentreplaced) with the text that arrived (set text, text of nodes made). Invariant 4 drops a removed node's text from that alignment unless all of it shows up at the common start or end. This PR keeps that rule for text sets and adds one step for the text of nodes made:The new step runs only where a stretch has text of nodes made and text of removed nodes. It only fills gaps between what the existing passes keep and never changes what they keep. It counts whole words (a new
diffWords: a run of letters, digits and_, or any single other character, compared whole), and every run it adds to has to start and end at word boundaries of both whole lines: the new line as it is after the update (text made and deleted again left out) and the old line (for text that came from another line, that line); and so must every part of such a run whose old characters follow each other in one old line (text typed exactly between them joins them, text typed anywhere else does not), where it holds an added character (so removed text from two places, read as one, makes no piece of an old word look whole and joins no new word). A word or character is whatever goes with it: its marks, a joiner or stacking sign and what it joins, an emoji's parts (the platform's grapheme segmenter, and these rules on top of it); a word reads through a soft hyphen, a word joiner and an inline node. Where the platform has no grapheme segmenter (Intl.Segmenter), the step does not run and the line is written as before:t1zmade again ast91z) is a new word, so a collaborator's concurrent replacement of the old word takes no letter from the new one;onemade again right before a settwo(onetwo),oneof an oldonexwhosexwas a removed node of its own, andredof an oldfooredmade again inside a neworedare new words too;inof one line andcomeofbecomein another, emptied links moved into a new line andincomemade there) is a new word too;t68zbeforet50zin a highlighted code line) no longer writes that letter again;tpiece of a highlightedt49zkeeps its characters next to pasted tokens that all holdt, but words an app made twice keep none;t69z→t81z; B typesQaftert63Qt63 t71z t13z t81z t15zt63Q t71z t13z t81z t15z7oft71z7comes backt12z+t13z t69z+t15zmade again as one node; B typesQint13zt12z Qt13z t81z t15zt12z t1Q3z t81z t15zxyz+one twomade again asone two X; B deletes thenone two Xoe two Xt48z+t+49z t50zmade again ast48zQ t78z t49z t50z; B typesZafter the oldtt48zZQ t78z t49z t50zt48zQ t78z tZ49z t50zt68z t69z t70zpasted beforet50z, tokens made again; B deletes thetoft50ztcomes backt1zmade again ast91z(also while the editor composes in another line); B replacest1zwitht94zone+two!made again asone+ settwo(onetwo); B deletes the oldoneonetwo tailonetwo tailredmade again asblue red red(a template that repeats the word); B deletes the oldredblue red red tailblue red red tailblue one+xmade again asgold one; B deletes the oldonexgold one tailgold one tailfoo+red,redmade again asblue red ored; B deletes the oldredfooblue red ored tailfooblue red ored tailone bluemade again asone!gold, then the!deleted; B deletes the oldoneonegold tailonegold tailblue بار(a ZWJ after the first letter) made again asgold دار; B deletes the old wordgold دار tailgold دار tailinandbe/come blue(two lines) emptied, moved into one line,income goldmade there; B deletes the oldbecomeincome goldincome goldt1zset tot91z(setTextContent); B replacest1zwitht94z9t94z9t94z(unchanged, known failure)Why invariant 4 exists, and what the new step keeps from it
A version that aligns removed text in part everywhere, as set text is, broke 15 existing tests. Each shows a job of the rule:
MadeNodeEdits, "a node removed before a node set whole": a removedxyzin front of a node set fromone twotoone twoXemptied the common start, so the set node's text was written again and a peer's concurrent delete came back (oe twoXbecameone twoX).RewriteMerge, "agent rewrite of a line with a bold word" (6 client orders): the set text's.aligned with the end of the removedverify timing.and moved a peer's concurrent typing to the start of the rewritten line.TextIdentity, "a move, a move of all of its destination, then that node set again": a character came out twice.In this PR the new step never gives removed text to set text, so 1 to 3 run exactly as on main. Only the S7-39 cases change: the rewrite now writes
t63z→t80zinstead oft63z→t80z, with the same convergence and fresh-reader checks.Invariants
Evidence
@lexical/yunit suite: 7,568 passed, 48 expected fail (46 on main plus the 2 new known failures);pnpm run ci-checkpasses.Stored bytes, per test: Doc state hashes over the
@lexical/yunit suite with deterministic client ids, 7,049 tests that are stable across two runs of main (67,568 docs). 10 tests differ, and every one makes text nodes again in place of removed ones with other text: the 6 S7-39 cases, 3TextNewlinescases (a node made again withone\ntwoforone two, which now keepsoneandtwo), and a scratch S9-7 repro. In every differing test the new step is what changed the alignment.Golden fuzz corpus, 8,100 seeds over 7 lanes (moves, gc, block ids, long, comments as keys): state bytes, editor JSON, warnings, selection and node map are identical to main.
Fuzz.test.tsseeds 1 to 3,000 (gc 1,000) and 50,000 to 51,999: the same failures as main (seed 2277 and 8 blocks between 50,000 and 51,999, all failing on main).Collaboration soak, 1,500 randomized sessions (3 profiles, 18 rounds, 3 peers and agents; the same seeds on main):
No new kind of flag. The letters still written again are mostly in code tokens a paste glued to the old word before them (
t47zt48z, the paste replacingt47z): that old word changed, so the step leavest48zas main does. Sessions diverge after the first update this PR writes differently, so per-session counts of the noisy code-annotation flags move in both directions; the one new undo flag (an undo of a move that drops a comment added later to the moved copy) and one new letter flag are in sessions that diverged.Randomized rewrite property check, 1,000 cases: a line of unique words in 1 to 4 text nodes is made again as 1 to 3 nodes with one or two word edits (replace, insert, delete, a letter added inside a word), while a peer types into, deletes from or replaces an old word. Oracles: the peer's letter stays in a word the rewrite kept, the peer's delete there stays, every word the rewrite kept or added is there whole.
The 7 remaining one-edit cases all start from what the existing passes keep: a removed node whose text is a piece of a word, matched whole at the common start or end (
. t3oft37ztaken for the start oft38z); the new step does not build on such a piece. With two edits, the words between them are written again: the diff keeps a common start and end only.With [lexical-code-core][lexical-code-prism][lexical-code-shiki][lexical-y] Bug Fix: Highlighting keeps the NodeState of every character in a code block #9558 applied (code highlighting keeps per-character NodeState), the session that lost a token after a paste no longer loses it, and one agent warning is gone; three self-check differences on a set-valued NodeState in code lines remain with and without this PR.
Review: an independent model reviewed it in twelve rounds, each re-running all earlier cases. Round 1 found two losses main does not have (composition in another line turned off word safety; a made word joined to the set word after it); round 2 two more (a removed neighbour deleted whole made a piece of a word look whole; a one-character context at an unchanged neighbour lent old letters to a different new word) and a gap in the copy rule; round 3 text made and deleted again in the same update faking a word boundary, and a joiner, mark or emoji part letting a changed character keep part of the old one; round 4 the same for Khmer and Myanmar conjuncts and Thai and Lao AM; round 5 a conjunct spanning an unchanged node and a slowdown of plain typing on long lines; round 6 words through a soft hyphen, a word joiner or a mark, and a slowdown when many nodes move between lines; rounds 5 to 7 also found characters the rules alone do not join where there is no grapheme segmenter, so the step no longer runs there; round 7 a slowdown segmenting many old lines; round 8 removed text of two lines read as one word, and a slowdown on very long lines; round 9 three more ways for removed text of two places to pass as one word (after typed text, between kept letters, two whole old words joined into one new word) and a word-diff shortcut that misread characters sharing a surrogate; round 10 text typed and removed at the edge of such a part hiding where it joins a new word; rounds 11 and 12 text typed in another line, or elsewhere in the same line, bridging old letters of a word. All are fixed here, each loss with a regression test. Its 4,233 probe cases, re-run on this version: every reported loss passes, with and without a segmenter; 49 strict-oracle rows fail only on this PR, all judged non-defects by the reviewer (a peer's letter typed where the rewrite deleted text now sits next to the unchanged word before it rather than at the line start; a word piece main keeps completed into its whole old word; old history kept around a character typed exactly into a made word), and 12 more call internals that changed; 182 fail on both and 230 only on main.
Known defect (open, to fix before this leaves draft)
TextReplay#stretch(TextEdits.tsaround lines 1666-1696), the start and end walks pass over a removed node that shares nothing at that edge and take the next removed node when only its edge character matches; a lone space or punctuation mark then counts as a word underwordCut(Inline.ts). A concurrent edit of that old space glues the rewritten words, which main does not do:Note:+un+believable resultsmade again astruly believable resultswhile a collaborator deletesNote:ends astrulybelievable results(main:truly believable results); the same with(draft)afterthe build+ing. When the collaborator instead replaces theNote:node with aTip:node, the result isTiptruly believable resultsin both client orders (main:Tip: truly believable results): the colon and the space are lost. Peers and a fresh reader converge, so the self-check does not flag it. The fix needs a regression row per shape.Risks
tpiece oft49z). An app that types a copy of a word fragment right before the original could see a peer's concurrent edit land on the copy.t47zt48zpasted overt47z), is written as main writes it.stretchand word helpers inInline.ts(about 900 added lines, a fifth of them comments). Nothing runs where no node is made in place of removed ones. Where it runs, it is linear in the line: each line text is segmented once per update, the word diff starts where the texts part, the run check scans twice at most, and the copy check runs oneindexOfpair per kept run of the text refused as there twice. Measured by the reviewers' probes: plain typing on 64K-character lines as main; one 64K-character node made again with its last word changed 18 ms (main 6 ms); 2,048 nodes moved between lines 68 ms (main 80 ms); 1,280 leaves moved and made again from 5 lines 243 ms (main 90 ms).Intl.Segmenter(all current browsers and Node): no word boundary falls inside one of its graphemes, and fixed rules (marks, joiners, stacking signs, prepend signs, Thai and Lao AM, emoji parts, regional indicators, soft hyphens, word joiners, inline nodes) apply on top of it. Elsewhere lines are written as before this PR.What it would close
Changeset
.changeset/lexical-y-keep-words-of-remade-text.md:[fix]for@lexical/y, with aVerify:paragraph.Test plan
New S9-7 cases in
packages/lexical-y/src/__tests__/unit/regressions/TextIdentity.test.ts; the S7-39 cases now expect only the changed word to be written.packages/lexical-y/src/__tests__/unit/WordDiff.test.tschecks the word diff against a walk from the ends on random texts.Before
After
Browser and e2e suites were not run locally; CI runs them.