Skip to content

Comment IDs can collide between collaborating peers (Comment.id is a module-level scalar) #257

@jedrazb

Description

@jedrazb

Problem

Comment IDs are allocated from a module-level scalar (`packages/react/src/components/DocxEditor.tsx:611` — `let nextCommentId = 1`). On document load, the editor bumps it above all known IDs (`:1093`). But:

  • Two peers in the same collab room start with `nextCommentId = 1`.
  • The collab demo seeds with `createEmptyDocument()` so the load-time bump never fires.
  • The first comment User A creates and the first comment User B creates both get `id: 1`.

Once the IDs collide, downstream lookups go to the wrong thread:

  • Reply lookups via `commentsRef.current.find(c => c.id === id)` (`DocxEditor.tsx:3566`, `:3571`) collapse onto whichever entry comes first in the array.
  • `injectReplyRangeMarkers` builds a `Map<number, number[]>` keyed by id (`:624`) — entries collide.
  • Resolves and deletes apply to the wrong comment.

Repro

  1. Open the collab demo (`examples/collaboration`) in two tabs against the same room.
  2. Without typing, click the comment button in tab A, add a comment.
  3. Click the comment button in tab B, add a different comment.
  4. Either tab's reply / resolve / delete actions on "comment 1" hit the other peer's comment.

(Hard to verify visually right now because remote cursors aren't rendered — see #256 — but the data corruption is real.)

Fix options

A. Composite IDs (cleanest). Change `Comment.id` from `number` to `string` and use ```${clientId}-${localCounter}```. `clientId` could come from a new `localPeerId` prop on `DocxEditor` (default to `crypto.randomUUID()` for non-collab use). Touches the schema and the comment serializer/parser (`commentParser.ts`, `commentSerializer.ts`) — DOCX uses numeric IDs in `<w:comment w:id="…">`, so the serializer needs to emit a stable mapping (string-id → numeric-id) at save time.

B. Numeric ID partitioning. Keep `Comment.id: number` but partition the space: `(clientPartition << 24) | localCounter`. New prop `commentIdBase: number` (default 0). Demo passes `commentIdBase = clientId * 1e6`. Less invasive than (A); preserves DOCX serialization. But still needs a way to hand `clientId` to the editor.

C. Re-id on receive in the demo (workaround, not a real fix). In `useCollaboration.setComments`, when a remote comment arrives with an id that already exists locally, rewrite the incoming id. Hides the collision from the editor but breaks reply/resolve referencing the original id.

Recommendation: B. Smallest change, preserves the OOXML-friendly numeric schema, only adds one optional prop.

Discovered in

PR #255 review (UX reviewer) — `examples/collaboration` / Yjs collab demo.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions