Repository navigation
Conversation
…ds when collaborators' text lands where no text node is ## Description Three v1 clients share `before [commented] after`, `commented` in a mark. B wraps the `o` in a mark `c1` and types `x` in the rest (`mmented`); C, having seen only B's first change, wraps the first `m` of `mmented` in `c1`, which writes `mmented` again. Once everything is delivered, no editor matches a fresh read of its own document, and the clients disagree (#9583). B's `x` lands in C's document between C's new mark and the deleted old text, where no text node is. `applyChildrenYjsDelta` removes such text from the shared type ("ignore the text"), but the removal was read back as a remote change and applied to collab children that never held the text, so it deleted the child now at that index: C's new mark, which its document kept. On the clients that had `x` inside their text node, C's change then deleted that node's properties map, and the dangling-text heuristic removed the leftover text at an offset into the text node rather than its index in the element, deleting other text (`c` and the map before it). Both places now mark the element instead, and once its deltas are applied `settleOrphanedText` removes the text no text node holds (text not after a text node's properties map) in one transaction with the binding as its origin, so it is not read back, and builds the element's collab children again from the shared type. The text is still removed, as before: a v1 text node needs its properties map, and every client removes the same items. Closes #9583 ## Test plan `packages/lexical-yjs/src/__tests__/unit/NestedMarksConcurrentTyping.test.ts` (new): the issue's steps with three v1 clients wired like the React collaboration plugin, for a mark at depth 1 and 2, with B or C the client that marks and types, in two client id orders: every editor equals a fresh read of its own document, and the clients agree. ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-yjs/src/__tests__/unit/NestedMarksConcurrentTyping.test.ts Tests 8 failed (8) AssertionError: A equals its own document: expected { root: { …(8) } } to deeply equal { root: { …(8) } } ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-yjs/src/__tests__/unit/NestedMarksConcurrentTyping.test.ts Tests 8 passed (8) $ pnpm exec vitest run --project unit packages/lexical-yjs packages/lexical-react Test Files 54 passed (54) Tests 527 passed | 4 expected fail (531) ``` `pnpm run ci-check` passes.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
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: |
|
AI review status for this pull request.
|
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
This looks good to me. Removing orphaned text once all deltas are applied, in a transaction whose origin is the binding, and then rebuilding the element's collab children from the shared type addresses both halves of #9583: the removal is no longer read back against children that never held the text, and the dangling-text case no longer deletes at an offset into the text node. The new test reproduces the issue for both mark depths, with either client typing and in both client-id orders.
A few optional, non-blocking suggestions:
- One orphan path still bypasses
settleOrphanedText(packages/lexical-yjs/src/CollabElementNode.ts, lines 327-357). When a remote embed lands inside a text node and no new text node follows it in the same delta, the split-off tail held inpendingSplitTextstays in the shared type right after the embed with no properties map before it, butorphanedis never set. That client's later deltas for the element are then indexed against children that are short by the tail's length. For example, with three clients: B removes thec1mark aroundf, so A holds one text nodeybfore x; C, without B's change, marks theb; on A,fore xis left orphaned after C's new mark, and the next character C types lands at the start of a later text node in A's editor but not in A's document. Settingorphaned = truewhenpendingSplitText !== nullafter the loop lets the same settle handle it. The base has the same gap, so a follow-up is fine too. - Assert the converged content in the test (
packages/lexical-yjs/src/__tests__/unit/NestedMarksConcurrentTyping.test.ts, lines 246-255). The test checks that each editor matches its own document and that the clients agree, but not what they agree on, so a regression that dropped the same text on every client would still pass. All eight cases converge tobefore commented after, soexpect(a.editor.read(() => $getRoot().getTextContent())).toBe('before commented after')pins it. - Add
// @vitest-environment nodeafter the copyright header of the new test. It only exercises headless editor state, which is the case AGENTS.md ("Testing Strategy") asks the annotation for, and most siblinglexical-yjsunit tests carry it.
| throw new Error('Unexpected delta format'); | ||
| } | ||
| } | ||
| if (orphaned) { |
There was a problem hiding this comment.
Optional, non-blocking: an unconsumed pendingSplitText is a third orphan case that never reaches this check. When a remote embed lands inside a text node and no text node follows it in the same delta (lines 327-350), the tail stays in the shared type right after the embed with no properties map before it, so later deltas for this element are indexed against children that are short by its length and land in the wrong text node. Flagging it lets settleOrphanedText handle it the same way (the base has the same gap, so a follow-up is fine too):
Suggested change
| if (orphaned) { | |
| if (pendingSplitText !== null) { | |
| orphaned = true; | |
| } | |
| if (orphaned) { |
| ).toEqual(fresh.json()); | ||
| } | ||
| expect(b.json()).toEqual(a.json()); | ||
| expect(c.json()).toEqual(a.json()); |
There was a problem hiding this comment.
Optional, non-blocking: this checks that the clients agree with their documents and with each other, but not what they agree on, so a regression that dropped the same text on every client would still pass. All eight cases converge to before commented after (B's x removed), so pinning it is cheap:
Suggested change
| expect(c.json()).toEqual(a.json()); | |
| expect(c.json()).toEqual(a.json()); | |
| expect(a.editor.read(() => $getRoot().getTextContent())).toBe( | |
| 'before commented after', | |
| ); |
| * LICENSE file in the root directory of this source tree. | ||
| * | ||
| */ | ||
|
|
There was a problem hiding this comment.
Optional nit: this file only exercises headless editor state, so AGENTS.md ("Testing Strategy") asks for // @vitest-environment node after the copyright header, as most sibling lexical-yjs unit tests have:
Suggested change
| // @vitest-environment node | |
Member
Author
|
Closing without merging: |
zurfyx
added a commit
that referenced
this pull request
Oct 30, 2026
…ote the v1/v2 fixes merged and closed ## Description Two rows of the `@lexical/y` decision register did not match the code or the pull request history: - D72 said the binding's first read is not deferred until every extension registers. The binding starts in `YCollabExtension`'s `afterRegistration`, once every extension has registered (the `TableOnOpen` test comment #9537 corrects says the same). The row now says that, and that the decision is no further deferral or special-casing: an initialization-order bug is fixed in the extension that has it, as #9501 did. - D12 (no more fixes to `@lexical/yjs` v1 and v2) now notes that one v2 table fix, #9584, merged on 10-30 before the row was checked, and that later v1 and v2 fix pull requests, #9595 and #9596, were closed under it. ## Changeset None: documentation only. ## Test plan ### Before D72 contradicted `packages/lexical-y/src/internal/YExtension.ts`, which starts the binding in `afterRegistration`. ### After ``` $ pnpm run ci-check ... Changeset check passed: 31 pending changeset(s) valid. ... Found 0 errors ```
This branch was successfully deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Three v1 clients share
before [commented] after,commentedin a mark.B wraps the
oin a markc1and typesxin the rest (mmented); C,having seen only B's first change, wraps the first
mofmmentedinc1, which writesmmentedagain. Once everything is delivered, noeditor matches a fresh read of its own document, and the clients
disagree (#9583).
B's
xlands in C's document between C's new mark and the deleted oldtext, where no text node is.
applyChildrenYjsDeltaremoves such textfrom the shared type ("ignore the text"), but the removal was read back
as a remote change and applied to collab children that never held the
text, so it deleted the child now at that index: C's new mark, which its
document kept. On the clients that had
xinside their text node, C'schange then deleted that node's properties map, and the dangling-text
heuristic removed the leftover text at an offset into the text node
rather than its index in the element, deleting other text (
cand themap before it).
Both places now mark the element instead, and once its deltas are applied
settleOrphanedTextremoves the text no text node holds (text not aftera text node's properties map) in one transaction with the binding as its
origin, so it is not read back, and builds the element's collab children
again from the shared type. The text is still removed, as before: a v1
text node needs its properties map, and every client removes the same
items.
Closes #9583
Test plan
packages/lexical-yjs/src/__tests__/unit/NestedMarksConcurrentTyping.test.ts(new): the issue's steps with three v1 clients wired like the React
collaboration plugin, for a mark at depth 1 and 2, with B or C the client
that marks and types, in two client id orders: every editor equals a fresh
read of its own document, and the clients agree.
Before
After
pnpm run ci-checkpasses.Changeset
[fix] Keep a v1 collaboration editor showing what its document holds when text a collaborator typed lands where no text node is, such as inside nested marks marked concurrently
The v1 binding removes such text from the document, as before, but that removal was read back as a remote change and deleted the node at its index instead (a mark next to the text vanished from the editor while its document kept it), and a text node whose properties were deleted in a merge had its leftover text removed at an offset into the node rather than its index in the element, deleting other text. The text is now removed with the binding as the origin, at its index, and the element's children are read again from the document.
Verify: with three v1 clients on a paragraph whose middle word is commented, mark a letter of the word and type after it on one client, mark the next letter on another that saw only the first mark, deliver everything, and check that every editor matches a fresh read of its own document.