Skip to content

[lexical-yjs] Bug Fix: refresh a remote cursor when its peer changes name or colour - #9013

Closed
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:yjs-cursor-awareness
Closed

[lexical-yjs] Bug Fix: refresh a remote cursor when its peer changes name or colour#9013
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:yjs-cursor-awareness

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Description

syncCursorPositions is the awareness-change handler: it runs on every
awareness update and reconciles binding.cursors against the current states.
It reads three fields from each peer's state, but only ever applies name and
color on the pass that first creates the cursor:

const {name, color, focusing} = awareness;
...
let cursor = cursors.get(clientID);

if (cursor === undefined) {
  cursor = createCursor(name, color);
  cursors.set(clientID, cursor);
}

Every later pass updates only anchor / focus. cursor.name and
cursor.color are baked into the DOM once, when createCursorSelection builds
the caret (name.textContent = cursor.name, the caret and label background
from cursor.color) and registers the ::highlight() rule via
addCursorHighlightRule. So a peer that renames itself or changes colour keeps
its old label and colour on every other client until it disconnects, which is
the only thing that removes the cursor.

This is reachable through the supported API rather than a hypothetical:
LexicalCollaborationPlugin takes username and cursorColor props, and
useYjsCollaboration's effect lists name and color in its dependencies and
re-publishes local awareness through initLocalState when they change (
useYjsFocusTracking does the same via setLocalStateFocus). The local peer
therefore does broadcast the change; remote peers just ignore it. A common
case is a session that starts anonymous and picks up a real display name after
sign-in.

When the name or colour actually changes, this destroys the stale selection
(which releases its caret DOM and its ::highlight() rule) and clears
cursor.selection, so the existing code below rebuilds it from the new values
on the same pass. A peer whose name and colour are unchanged keeps its cursor
object untouched, so there is no rebuild churn on ordinary cursor movement.

Test plan

New unit test packages/lexical-yjs/src/__tests__/unit/SyncCursorsAwarenessRefresh.test.ts,
driving syncCursorPositions through its getAwarenessStates option. The
third case asserts an unchanged peer keeps the same cursor object, pinning
that the rebuild only happens when the awareness fields really change.

Before

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

     × a peer that renames itself updates its cursor name 9ms
     × a peer that changes colour updates its cursor colour 1ms

AssertionError: expected 'Bob' to be 'Robert' // Object.is equality
AssertionError: expected '#ff0000' to be '#0000ff' // Object.is equality

      Tests  2 failed | 1 passed (3)

After

$ 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)

The DOM rebuild itself is not exercised by the unit test — jsdom has no layout,
so updateCursor returns before touching the caret. It was not verified in a
real browser.

…name or colour

## Description

`syncCursorPositions` is the awareness-change handler: it runs on every
awareness update and reconciles `binding.cursors` against the current states.
It reads three fields from each peer's state, but only ever applies `name` and
`color` on the pass that first creates the cursor:

```js
const {name, color, focusing} = awareness;
...
let cursor = cursors.get(clientID);

if (cursor === undefined) {
  cursor = createCursor(name, color);
  cursors.set(clientID, cursor);
}
```

Every later pass updates only `anchor` / `focus`. `cursor.name` and
`cursor.color` are baked into the DOM once, when `createCursorSelection` builds
the caret (`name.textContent = cursor.name`, the caret and label background
from `cursor.color`) and registers the `::highlight()` rule via
`addCursorHighlightRule`. So a peer that renames itself or changes colour keeps
its old label and colour on every other client until it disconnects, which is
the only thing that removes the cursor.

This is reachable through the supported API rather than a hypothetical:
`LexicalCollaborationPlugin` takes `username` and `cursorColor` props, and
`useYjsCollaboration`'s effect lists `name` and `color` in its dependencies and
re-publishes local awareness through `initLocalState` when they change (
`useYjsFocusTracking` does the same via `setLocalStateFocus`). The local peer
therefore does broadcast the change; remote peers just ignore it. A common
case is a session that starts anonymous and picks up a real display name after
sign-in.

When the name or colour actually changes, this destroys the stale selection
(which releases its caret DOM and its `::highlight()` rule) and clears
`cursor.selection`, so the existing code below rebuilds it from the new values
on the same pass. A peer whose name and colour are unchanged keeps its cursor
object untouched, so there is no rebuild churn on ordinary cursor movement.

## Test plan

New unit test `packages/lexical-yjs/src/__tests__/unit/SyncCursorsAwarenessRefresh.test.ts`,
driving `syncCursorPositions` through its `getAwarenessStates` option. The
third case asserts an unchanged peer keeps the *same* cursor object, pinning
that the rebuild only happens when the awareness fields really change.

### Before

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

     × a peer that renames itself updates its cursor name 9ms
     × a peer that changes colour updates its cursor colour 1ms

AssertionError: expected 'Bob' to be 'Robert' // Object.is equality
AssertionError: expected '#ff0000' to be '#0000ff' // Object.is equality

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

### After

```
$ 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)
```

The DOM rebuild itself is not exercised by the unit test — jsdom has no layout,
so `updateCursor` returns before touching the caret. It was not verified in a
real browser.
@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