Skip to content

[lexical-yjs] Bug Fix: convert element selection points to yjs child indices in collab-v2 - #9016

Closed
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:yjs-v2-element-position
Closed

[lexical-yjs] Bug Fix: convert element selection points to yjs child indices in collab-v2#9016
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:yjs-v2-element-position

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Description

In collab-v2, normalizeNodeContent (SyncV2) serializes a run of adjacent
TextNodes as a single XmlText child. A paragraph whose lexical children
are [Text, Text, Decorator] therefore has only two yjs children:
[XmlText, XmlElement]. Lexical child offsets and yjs child indices are not
the same number.

createRelativePositionV2 walks one lexical child per step and hands the
lexical offset straight to createRelativePositionFromTypeIndex:

let i = 0;
let child = node.getFirstChild();
while (child !== null && i < offset) {
  if ($isTextNode(child)) {
    let nextSibling = child.getNextSibling();
    while ($isTextNode(nextSibling)) {
      nextSibling = nextSibling.getNextSibling();   // computed, then discarded
    }
  }
  i++;
  child = child.getNextSibling();
}
return createRelativePositionFromTypeIndex(yType, i, assoc);

The inner loop walks the text run but throws the result away — child and i
still advance one lexical child at a time. Its exact inverse,
$getNodeAndOffsetV2, does the collapse correctly: it consumes one yjs offset
per child and then skips the rest of a text run. The discarded nextSibling is
the fossil of the same collapse that was meant to happen here.

Consequence: for the paragraph above, a caret before the decorator is lexical
offset 2, which encodes as yjs index 2 — past the decorator. Remote peers
render the cursor on the wrong side of the decorator. At the end of the
paragraph the encoded index is out of range and collapses to "end of type".

This advances a lexical cursor to offset while counting each text run as one
yjs child, so the encoder mirrors the decoder.

Test plan

New unit test packages/lexical-yjs/src/__tests__/unit/SyncCursorsV2ElementPoint.test.ts.
It builds a real V2 binding, serializes a [Text, Text, Decorator] paragraph,
pushes an element-type selection through syncLexicalSelectionToYjs (the
awareness encoder remote peers actually receive) and decodes it back with
$getAnchorAndFocusForUserState. Offsets 0 and 3 are included as controls —
they already round trip, so the failure is pinned to a point that sits after a
text run and before another child.

Before

$ npx vitest run packages/lexical-yjs/src/__tests__/unit/SyncCursorsV2ElementPoint.test.ts

     × an element point before a decorator that follows a text run round trips 14ms

AssertionError: expected 3 to be 2 // Object.is equality

      Tests  1 failed | 2 passed (3)

After

$ npx vitest run packages/lexical-yjs/src/__tests__/unit/SyncCursorsV2ElementPoint.test.ts

 Test Files  1 passed (1)
      Tests  3 passed (3)

$ npx vitest run packages/lexical-yjs

 Test Files  6 passed (6)
      Tests  81 passed (81)

$ npx vitest run packages/lexical-react

 Test Files  31 passed (31)
      Tests  182 passed (182)

…indices in collab-v2

## Description

In collab-v2, `normalizeNodeContent` (`SyncV2`) serializes a run of adjacent
`TextNode`s as a *single* `XmlText` child. A paragraph whose lexical children
are `[Text, Text, Decorator]` therefore has only two yjs children:
`[XmlText, XmlElement]`. Lexical child offsets and yjs child indices are not
the same number.

`createRelativePositionV2` walks one lexical child per step and hands the
lexical offset straight to `createRelativePositionFromTypeIndex`:

```js
let i = 0;
let child = node.getFirstChild();
while (child !== null && i < offset) {
  if ($isTextNode(child)) {
    let nextSibling = child.getNextSibling();
    while ($isTextNode(nextSibling)) {
      nextSibling = nextSibling.getNextSibling();   // computed, then discarded
    }
  }
  i++;
  child = child.getNextSibling();
}
return createRelativePositionFromTypeIndex(yType, i, assoc);
```

The inner loop walks the text run but throws the result away — `child` and `i`
still advance one lexical child at a time. Its exact inverse,
`$getNodeAndOffsetV2`, does the collapse correctly: it consumes one yjs offset
per child and then skips the rest of a text run. The discarded `nextSibling` is
the fossil of the same collapse that was meant to happen here.

Consequence: for the paragraph above, a caret *before* the decorator is lexical
offset 2, which encodes as yjs index 2 — past the decorator. Remote peers
render the cursor on the wrong side of the decorator. At the end of the
paragraph the encoded index is out of range and collapses to "end of type".

This advances a lexical cursor to `offset` while counting each text run as one
yjs child, so the encoder mirrors the decoder.

## Test plan

New unit test `packages/lexical-yjs/src/__tests__/unit/SyncCursorsV2ElementPoint.test.ts`.
It builds a real V2 binding, serializes a `[Text, Text, Decorator]` paragraph,
pushes an element-type selection through `syncLexicalSelectionToYjs` (the
awareness encoder remote peers actually receive) and decodes it back with
`$getAnchorAndFocusForUserState`. Offsets 0 and 3 are included as controls —
they already round trip, so the failure is pinned to a point that sits after a
text run and before another child.

### Before

```
$ npx vitest run packages/lexical-yjs/src/__tests__/unit/SyncCursorsV2ElementPoint.test.ts

     × an element point before a decorator that follows a text run round trips 14ms

AssertionError: expected 3 to be 2 // Object.is equality

      Tests  1 failed | 2 passed (3)
```

### After

```
$ npx vitest run packages/lexical-yjs/src/__tests__/unit/SyncCursorsV2ElementPoint.test.ts

 Test Files  1 passed (1)
      Tests  3 passed (3)

$ npx vitest run packages/lexical-yjs

 Test Files  6 passed (6)
      Tests  81 passed (81)

$ npx vitest run packages/lexical-react

 Test Files  31 passed (31)
      Tests  182 passed (182)
```
@vercel

vercel Bot commented Aug 9, 2026

Copy link
Copy Markdown

@LeSingh1 is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 9, 2026
@LeSingh1

Copy link
Copy Markdown
Contributor Author

Consolidated into #9057 with the other PRs that share this defect, per @etrepum's note on #9027 and @mayrang's on #9035. Same fix and same tests, one review.

@LeSingh1 LeSingh1 closed this Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant