Migrate collaborative editing from simple diffing to CRDT#35
Open
Migrate collaborative editing from simple diffing to CRDT#35
Conversation
Replace the simple character-by-character diffing approach with Yjs CRDT for more robust collaborative editing: - Add Yjs, y-protocols, and y-codemirror.next dependencies - Rewrite CollaborativeDocument Durable Object to use Yjs sync protocol - Update CodeMirror component to use yCollab extension for collaboration - Create YjsWebSocketProvider for client-side Yjs sync - Remove manual cursor/selection tracking (now handled by Yjs awareness) - Update page component to use Yjs document and awareness Benefits: - Automatic conflict resolution instead of "last write wins" - Built-in offline support capability - Better scalability with many concurrent editors - Industry-proven approach (used by Notion, Linear, etc.)
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gether | 58d90b0 | Commit Preview URL Branch Preview URL |
Jan 10 2026, 11:23 PM |
There was a problem hiding this comment.
5 issues found across 8 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/ui/src/lib/components/editor/CodeMirror.svelte">
<violation number="1" location="packages/ui/src/lib/components/editor/CodeMirror.svelte:53">
P1: The `isCollaborative` derived state only checks for `yText`, but the yCollab extension requires both `yText` AND `awareness`. If `yText` is provided without `awareness`, the editor will be non-functional: collaborative sync won't work (yCollab not added), and manual value sync is also disabled.</violation>
</file>
<file name="packages/collab/src/index.ts">
<violation number="1" location="packages/collab/src/index.ts:28">
P3: The `clientIdToWebSocket` map is populated but never read. This appears to be dead code. Either remove it or document its intended use.</violation>
<violation number="2" location="packages/collab/src/index.ts:114">
P2: The client ID generation formula `this.doc.clientID + this.ctx.getWebSockets().length` is prone to collisions. Two simultaneous connections could get the same ID, and IDs can be reused after disconnections. Consider using `crypto.randomUUID()` or an incrementing counter stored in the DO state for unique IDs.</violation>
<violation number="3" location="packages/collab/src/index.ts:129">
P2: Race condition: The initial content check `text.length === 0 && initialContent` is not protected by `blockConcurrencyWhile`. If multiple clients connect simultaneously with initial content, they could all see the document as empty and insert duplicate content.</violation>
<violation number="4" location="packages/collab/src/index.ts:247">
P1: The `removeAwarenessStates` call uses server-assigned `clientId` which doesn't match the actual awareness client IDs. Awareness states are keyed by each client's own Y.Doc clientID (set on the client side), not the server-assigned ID. This will fail to clean up stale awareness states, causing ghost cursors/presence for disconnected users. You need to track the client's actual awareness clientID (received in awareness messages) or query `awareness.getStates()` to find which clientId to remove.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Addresses issues identified in PR review: P1 - Missing Awareness Validation: - Fix isCollaborative check to require both yText AND awareness - Prevents silent failures when only yText is provided P1 - Awareness State Cleanup: - Remove manual awareness cleanup (was using wrong client IDs) - Let Yjs awareness protocol handle cleanup via timeouts - Add documentation explaining why manual cleanup doesn't work P2 - Client ID Collision: - Replace collision-prone formula with crypto.randomUUID() - Connection ID now only used for debugging/logging P2 - Race Condition on Initialization: - Add documentInitialized flag to prevent duplicate content inserts - Check flag before attempting to initialize document content P3 - Dead Code: - Remove unused clientIdToWebSocket map
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.
Replace the simple character-by-character diffing approach with Yjs CRDT
for more robust collaborative editing:
Benefits:
Summary by cubic
Migrate collaborative editing to Yjs CRDT for conflict-free syncing, real-time awareness, and better scalability. Replaces custom diffs and manual cursor tracking with an industry-standard approach.
New Features
Migration
Written for commit 58d90b0. Summary will update on new commits.