feat(apps/playground): add modular online collaborative editor #595
Conversation
- Create room-based architecture with unique room IDs for collaboration - Implement local-first storage using IndexedDB with localStorage fallback - Add real-time sync adapter using BroadcastChannel (WebSocket-ready) - Split functionality into focused modules (room-manager, storage, sync-adapter) - Create separate hooks for editor, text changes, and recent rooms - Use direct imports instead of barrel files for better dependency tracing - Support room creation, joining, and sharing via URL parameters - Add participant tracking and presence indicators - Persist documents and room history locally - Each module under 200 lines following codebase preferences
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
zhyd1997
had a problem deploying
to
apps/web-e2e
GitHub Actions
Error
zhyd1997
had a problem deploying
to
apps/web-e2e-smoke
GitHub Actions
Error
📝 WalkthroughWalkthroughAdds a full collaborative editor to the playground: types, local persistence, a SyncAdapter (WebSocket/BroadcastChannel), RoomManager (CRDT orchestration), React hooks (lifecycle, recent rooms, debounced text sync), utilities, and a new demo route with UI for creating/joining rooms and real-time editing. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Client (UI)
participant Hook as useCollabEditor
participant RoomMgr as RoomManager
participant Storage as LocalStorage
participant CRDT as EgWalker API
participant Sync as SyncAdapter
participant Peer as Remote Peer
rect rgba(220,240,255,0.7)
Note over User,RoomMgr: Create / Join flow
User->>Hook: createRoom(name) / joinRoom(id)
Hook->>Storage: saveUser / getUser
Storage-->>Hook: user
Hook->>RoomMgr: createRoom / joinRoom
RoomMgr->>Storage: saveRoom / getDocument
Storage-->>RoomMgr: room / stored events
RoomMgr->>CRDT: apply stored events
CRDT-->>RoomMgr: reconstructed state
RoomMgr->>Sync: initSyncAdapter(roomId, user)
Sync->>Peer: emit "join"
RoomMgr-->>Hook: currentRoom, participants, text
Hook-->>User: render joined UI
end
sequenceDiagram
participant User as Client (editing)
participant Hook as useTextChange
participant RoomMgr as RoomManager
participant CRDT as EgWalker API
participant Storage as LocalStorage
participant Sync as SyncAdapter
participant Peer as Remote Peer
rect rgba(240,255,240,0.7)
Note over User,Peer: Local edit and sync
User->>Hook: onChange(newText)
Hook->>Hook: debounce + diff calc
Hook->>RoomMgr: handleLocalChange(op, pos, text/len)
RoomMgr->>CRDT: apply operation -> event
CRDT-->>RoomMgr: new event (+state)
RoomMgr->>Storage: saveDocument (content, version, events)
RoomMgr->>Sync: send(event)
Sync->>Peer: broadcast "event"
Peer->>Sync: remote "event"
Sync-->>RoomMgr: received remote event
RoomMgr->>CRDT: applyEvent(remote event)
CRDT-->>RoomMgr: updated text
RoomMgr-->>Hook: onContentChange -> update UI
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage Report for packages/eg-walker
File CoverageNo changed files found. |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (14)
apps/playground/src/routes/index.tsx (1)
149-152: Avoidanytype cast for feature properties.Per coding guidelines, avoid using the
anytype. Define a proper interface for the feature object to maintain type safety.🔎 Proposed fix
Define the feature type at the top of the component:
interface Feature { icon: React.ReactNode; title: string; description: string; link: string; external?: boolean; }Then type the features array:
- const features = [ + const features: Feature[] = [And update the template:
- target={(feature as any).external ? "_blank" : "_self"} - rel={ - (feature as any).external ? "noopener noreferrer" : undefined - } + target={feature.external ? "_blank" : "_self"} + rel={feature.external ? "noopener noreferrer" : undefined}apps/playground/src/modules/collab-editor/hooks/use-collab-editor.ts (2)
17-33: Init errors are silently swallowed.If
roomManager.init()fails, the error is only logged to console. Consider surfacing initialization failures to the UI via theerrorstate so users know when the system isn't ready.🔎 Proposed fix
useEffect(() => { - roomManager.init().catch(console.error); + roomManager.init().catch((err) => { + console.error(err); + setError(err instanceof Error ? err.message : "Failed to initialize"); + }); // Override event handlers
40-45: Consider extracting user ID generation to a utility.The user ID generation pattern
Math.random().toString(36).substring(2, 9)is duplicated in bothcreateRoom(line 42) andjoinRoom(line 73). Consider extracting this to agenerateUserId()utility inutils.tsalongside the existinggenerateRoomId().apps/playground/src/modules/collab-editor/hooks/use-recent-rooms.ts (1)
43-59: Unnecessaryasyncandtry-catchwrappers.
removeRecentRoomandclearRecentRoomsdon't perform any async operations andsetStatedoesn't throw synchronously. Theasynckeyword andtry-catchblocks add unnecessary complexity.🔎 Proposed simplification
- const removeRecentRoom = async (roomId: string) => { - try { - // Note: We don't actually delete from storage, just filter from display - setRecentRooms((prev) => prev.filter((r) => r.id !== roomId)); - } catch (error) { - console.error("Failed to remove recent room:", error); - } - }; + const removeRecentRoom = (roomId: string) => { + // Note: We don't actually delete from storage, just filter from display + setRecentRooms((prev) => prev.filter((r) => r.id !== roomId)); + }; - const clearRecentRooms = async () => { - try { - // Clear from display only - setRecentRooms([]); - } catch (error) { - console.error("Failed to clear recent rooms:", error); - } - }; + const clearRecentRooms = () => { + // Clear from display only + setRecentRooms([]); + };apps/playground/src/modules/collab-editor/utils.ts (1)
42-51: Debounce function doesn't handle cleanup properly.The returned debounced function doesn't expose a way to cancel pending timeouts. This can cause issues in React components if they unmount before the timeout fires. Consider returning a cancel method or using a ref-based approach in hooks.
🔎 Proposed enhancement
export function debounce<Args extends unknown[], Return>( func: (...args: Args) => Return, delay: number, -): (...args: Args) => void { +): { (...args: Args): void; cancel: () => void } { let timeoutId: ReturnType<typeof setTimeout>; - return (...args: Args) => { + const debounced = (...args: Args) => { clearTimeout(timeoutId); timeoutId = setTimeout(() => func(...args), delay); }; + debounced.cancel = () => clearTimeout(timeoutId); + return debounced; }apps/playground/src/modules/collab-editor/hooks/use-text-change.ts (1)
13-46: Stale closure:roomManagerwon't update after initial render.The
useCallbackhas an empty dependency array ([]), butdebouncecapturesroomManagerin the closure during the first render. IfroomManagerchanges (though unlikely with current implementation), the callback won't see the new value.Additionally, the
sourceparameter is unused since it always defaults to"local"and is never passed with a different value.🔎 Proposed fix
const handleTextChange = useCallback( - debounce((value: string, source: "local" | "remote" = "local") => { - if (!roomManager || source === "remote") return; + debounce((value: string) => { + if (!roomManager) return; const currentText = roomManager.getText(); // ... rest of logic }, 100), - [], + [roomManager], );Note: Adding
roomManagerto dependencies means a new debounced function is created whenroomManagerchanges. If this causes issues, consider using a ref to hold the latestroomManager.apps/playground/src/routes/demo/online-collab-editor.tsx (3)
32-36: Use double quotes for string literals.Per coding guidelines, use double quotes for strings. Lines 32-34 use single quotes.
🔎 Proposed fix
- const [userName, setUserName] = useState(''); - const [roomName, setRoomName] = useState(''); - const [joinRoomId, setJoinRoomId] = useState(''); + const [userName, setUserName] = useState(""); + const [roomName, setRoomName] = useState(""); + const [joinRoomId, setJoinRoomId] = useState("");
55-61: Consider using TanStack Router's search params instead ofwindow.location.Since you're using
@tanstack/react-router, you could leverage its built-in search parameter handling for better type safety and consistency with the router's state management.
176-180: String literals should use double quotes.Lines 179, 216, etc. use single quotes. Apply consistent double quote formatting throughout.
apps/playground/src/modules/collab-editor/sync-adapter.ts (3)
22-26: Silent failure when BroadcastChannel is unavailable in demo mode.If
wsUrlis not provided butBroadcastChannelis not supported (older browsers/environments), the adapter silently remains unconnected with no feedback to the caller. Consider emitting an error or warning.🔎 Proposed fix
if (this.useBroadcastChannel && "BroadcastChannel" in window) { this.initBroadcastChannel(); + } else if (this.useBroadcastChannel) { + console.warn("BroadcastChannel not supported; sync adapter will not connect"); + this.emit("error", new Error("BroadcastChannel not supported")); } else if (wsUrl) { this.connect(); }
38-40: Potential duplicate WebSocket if called during CONNECTING state.The guard only checks for
OPENstate. Ifconnect()is called while WebSocket is inCONNECTINGstate, a new WebSocket will be created, potentially causing issues.🔎 Proposed fix
private connect(): void { - if (this.ws?.readyState === WebSocket.OPEN) return; + if (this.ws?.readyState === WebSocket.OPEN || this.ws?.readyState === WebSocket.CONNECTING) return;
78-85: Consider exponential backoff for reconnection.The fixed 3-second reconnect delay is acceptable for a demo, but could cause rapid reconnection attempts if the server is down. Consider exponential backoff for production use.
apps/playground/src/modules/collab-editor/room-manager.ts (2)
23-35: Handle joinRoom failure in createRoom.If
joinRoomreturnsfalse, the room is saved to storage but the user never joined. This could leave orphaned rooms. Consider checking the return value.🔎 Proposed fix
await storage.saveRoom(room); - await this.joinRoom(room.id, user); + const joined = await this.joinRoom(room.id, user); + if (!joined) { + throw new Error("Failed to join newly created room"); + } return room;
85-85: Unused_roomIdparameter.The
_roomIdparameter is unused. If it's reserved for future use (e.g., room-specific filtering), consider adding a TODO comment; otherwise, remove it.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
apps/playground/package.jsonapps/playground/src/modules/collab-editor/hooks/use-collab-editor.tsapps/playground/src/modules/collab-editor/hooks/use-recent-rooms.tsapps/playground/src/modules/collab-editor/hooks/use-text-change.tsapps/playground/src/modules/collab-editor/room-manager.tsapps/playground/src/modules/collab-editor/storage.tsapps/playground/src/modules/collab-editor/sync-adapter.tsapps/playground/src/modules/collab-editor/types.tsapps/playground/src/modules/collab-editor/utils.tsapps/playground/src/routes/demo/online-collab-editor.tsxapps/playground/src/routes/index.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indent, double quotes, and semicolons for code formatting
Files:
apps/playground/src/modules/collab-editor/hooks/use-collab-editor.tsapps/playground/package.jsonapps/playground/src/routes/demo/online-collab-editor.tsxapps/playground/src/routes/index.tsxapps/playground/src/modules/collab-editor/utils.tsapps/playground/src/modules/collab-editor/storage.tsapps/playground/src/modules/collab-editor/hooks/use-text-change.tsapps/playground/src/modules/collab-editor/sync-adapter.tsapps/playground/src/modules/collab-editor/room-manager.tsapps/playground/src/modules/collab-editor/hooks/use-recent-rooms.tsapps/playground/src/modules/collab-editor/types.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Do not use TypeScriptanytype; use proper types, union types, generics, or specific interfaces instead
When proper typing is not feasible for TypeScript errors, use@ts-expect-errorwith a descriptive comment explaining why, never use@ts-ignore
Do not use TypeScript enums; use const objects withas constassertion instead for performance and bundle-size optimization
Files:
apps/playground/src/modules/collab-editor/hooks/use-collab-editor.tsapps/playground/src/routes/demo/online-collab-editor.tsxapps/playground/src/routes/index.tsxapps/playground/src/modules/collab-editor/utils.tsapps/playground/src/modules/collab-editor/storage.tsapps/playground/src/modules/collab-editor/hooks/use-text-change.tsapps/playground/src/modules/collab-editor/sync-adapter.tsapps/playground/src/modules/collab-editor/room-manager.tsapps/playground/src/modules/collab-editor/hooks/use-recent-rooms.tsapps/playground/src/modules/collab-editor/types.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Write pure functions without side effects that return the same output for the same inputs
Avoid mutating data structures; return new copies instead using spread operators, Array.map(), Object.freeze(), etc.
Leverage higher-order functions such as map, filter, and reduce for functional programming patterns
Build complex logic by composing smaller, reusable functions
Files:
apps/playground/src/modules/collab-editor/hooks/use-collab-editor.tsapps/playground/src/routes/demo/online-collab-editor.tsxapps/playground/src/routes/index.tsxapps/playground/src/modules/collab-editor/utils.tsapps/playground/src/modules/collab-editor/storage.tsapps/playground/src/modules/collab-editor/hooks/use-text-change.tsapps/playground/src/modules/collab-editor/sync-adapter.tsapps/playground/src/modules/collab-editor/room-manager.tsapps/playground/src/modules/collab-editor/hooks/use-recent-rooms.tsapps/playground/src/modules/collab-editor/types.ts
🧠 Learnings (5)
📚 Learning: 2025-12-27T07:28:24.288Z
Learnt from: CR
Repo: softmaple/softmaple PR: 0
File: packages/eg-walker/CLAUDE.md:0-0
Timestamp: 2025-12-27T07:28:24.288Z
Learning: Run 'pnpm --filter softmaple/eg-walker typecheck' to verify no TypeScript errors before committing
Applied to files:
apps/playground/package.json
📚 Learning: 2025-12-27T07:28:35.736Z
Learnt from: CR
Repo: softmaple/softmaple PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T07:28:35.736Z
Learning: Applies to apps/web/app/**/*.{ts,tsx} : Use lowercase-hyphenated naming for route files
Applied to files:
apps/playground/src/routes/index.tsx
📚 Learning: 2025-12-27T07:28:24.288Z
Learnt from: CR
Repo: softmaple/softmaple PR: 0
File: packages/eg-walker/CLAUDE.md:0-0
Timestamp: 2025-12-27T07:28:24.288Z
Learning: Applies to packages/eg-walker/src/crdt/**/*.ts : Query RetreatAdvanceCoordinator as the single source of truth for state rather than duplicating tracking in separate classes
Applied to files:
apps/playground/src/modules/collab-editor/room-manager.ts
📚 Learning: 2025-12-27T07:28:45.016Z
Learnt from: CR
Repo: softmaple/softmaple PR: 0
File: packages/eg-walker/AGENTS.md:0-0
Timestamp: 2025-12-27T07:28:45.016Z
Learning: Applies to packages/eg-walker/src/{eg-walker,crdt}.ts : Core algorithm implementation should be in `src/eg-walker.ts` and `src/crdt.ts`
Applied to files:
apps/playground/src/modules/collab-editor/room-manager.ts
📚 Learning: 2025-12-27T07:28:24.288Z
Learnt from: CR
Repo: softmaple/softmaple PR: 0
File: packages/eg-walker/CLAUDE.md:0-0
Timestamp: 2025-12-27T07:28:24.288Z
Learning: Applies to packages/eg-walker/src/{graph,crdt}/**/*.ts : Use efficient data structures: Map<EventId, GraphEvent> for O(1) lookup, Set<EventId> for O(1) membership checks
Applied to files:
apps/playground/src/modules/collab-editor/types.ts
🧬 Code graph analysis (6)
apps/playground/src/modules/collab-editor/utils.ts (1)
apps/playground/src/modules/collab-editor/room-manager.ts (1)
generateRoomId(263-265)
apps/playground/src/modules/collab-editor/storage.ts (1)
apps/playground/src/modules/collab-editor/types.ts (3)
Room(1-6)Document(12-17)User(19-23)
apps/playground/src/modules/collab-editor/hooks/use-text-change.ts (2)
apps/playground/src/modules/collab-editor/room-manager.ts (1)
RoomManager(9-270)apps/playground/src/modules/collab-editor/utils.ts (1)
debounce(42-51)
apps/playground/src/modules/collab-editor/sync-adapter.ts (1)
apps/playground/src/modules/collab-editor/types.ts (1)
SyncMessage(44-48)
apps/playground/src/modules/collab-editor/room-manager.ts (3)
apps/playground/src/modules/collab-editor/sync-adapter.ts (1)
SyncAdapter(8-146)apps/playground/src/modules/collab-editor/types.ts (4)
Room(1-6)User(19-23)SyncMessage(44-48)Document(12-17)apps/playground/src/modules/collab-editor/storage.ts (1)
storage(170-170)
apps/playground/src/modules/collab-editor/hooks/use-recent-rooms.ts (2)
apps/playground/src/modules/collab-editor/types.ts (1)
Room(1-6)apps/playground/src/modules/collab-editor/storage.ts (1)
storage(170-170)
🪛 GitHub Check: CodeQL
apps/playground/src/modules/collab-editor/room-manager.ts
[failure] 68-68: Insecure randomness
This uses a cryptographically insecure random number generated at Math.random() in a security context.
This uses a cryptographically insecure random number generated at Math.random() in a security context.
[failure] 77-77: Insecure randomness
This uses a cryptographically insecure random number generated at Math.random() in a security context.
This uses a cryptographically insecure random number generated at Math.random() in a security context.
🔇 Additional comments (18)
apps/playground/package.json (2)
20-48: Dependency organization and version pinning look good.The dependencies are appropriately pinned with caret ranges (
^), allowing for non-breaking updates while maintaining stability. The addition ofeventemitter3for the collaborative editor features and the reordering of existing dependencies are reasonable changes.Confirm that the
@playwright/testspecification at line 51 (version^1.57.0) is intentional and aligns with your project's testing requirements.
38-38: Eventemitter3@5.0.1 is stable and appropriate for this use case.The package is the latest stable version with no known security vulnerabilities and active maintenance. The codebase correctly uses it in the SyncAdapter class for the collaborative editor feature's event-driven architecture. JSON formatting is correct.
apps/playground/src/modules/collab-editor/types.ts (1)
1-48: Well-structured type definitions.The discriminated union pattern for
SyncMessageDatais a good choice for type-safe message handling. The re-export ofGraphEventfrom@softmaple/eg-walkerprovides a clean abstraction layer.apps/playground/src/routes/index.tsx (1)
38-44: LGTM!The new feature card for the Online Collaborative Editor is correctly integrated with appropriate icon, description, and link.
apps/playground/src/modules/collab-editor/hooks/use-text-change.ts (1)
80-91:findLastDifferencemay return incorrect index for edge cases.When strings have different lengths, the return value
Math.max(len1, len2) - 1 - ican be confusing. For example, comparing"abc"with"ab"would return0, which doesn't represent a meaningful "last difference" position instr1.Consider adding unit tests to verify edge cases:
- Same-length strings with single char difference
- Different-length strings
- Identical strings
apps/playground/src/modules/collab-editor/storage.ts (1)
169-170: Storage singleton initialization is properly handled through RoomManager.The
storage.init()method is automatically called during component initialization. WhenuseCollabEditormounts, itsuseEffectinvokesroomManager.init()(line 19), which callsstorage.init()before any storage methods are used. All subsequent storage operations (saveUser,saveRoom, etc.) occur after this initialization completes, ensuring IndexedDB is properly set up. Additionally, all storage methods include fallback logic to localStorage if IndexedDB is unavailable, so the system operates correctly even without explicit initialization.Likely an incorrect or invalid review comment.
apps/playground/src/modules/collab-editor/sync-adapter.ts (4)
29-36: LGTM!BroadcastChannel initialization is straightforward and correctly emits the connected event.
87-111: LGTM!Message routing is clean and correctly maps message types to specific events. The switch without a default allows for future extensibility.
113-121: LGTM!Send logic correctly handles both transport types with appropriate serialization.
127-145: LGTM!Disconnect method properly cleans up all resources and state.
apps/playground/src/modules/collab-editor/room-manager.ts (8)
1-17: LGTM!Class structure and imports are well-organized with proper typing.
19-21: LGTM!Simple initialization delegation.
37-83: LGTM!Room joining logic properly loads existing state, initializes CRDT, and sets up synchronization with appropriate error handling for event application.
146-164: LGTM!Sync response handling properly validates data and applies events with error handling.
200-212: LGTM!Document saving logic correctly captures state for persistence.
234-244: LGTM!Simple accessor methods are well-implemented.
246-261:currentUsernot cleared on leaveRoom.All room state is cleared except
currentUser. If this is intentional (user persists for rejoining), consider adding a comment. Otherwise, addthis.currentUser = null;.
267-269: LGTM!Override hooks provide extensibility for content and participant change notifications.
| this.syncAdapter?.send({ | ||
| type: "sync-response", | ||
| roomId: this.currentRoom.id, | ||
| userId: this.currentUser?.id, | ||
| data: { events: newEvents, version: events.length }, | ||
| timestamp: Date.now(), | ||
| }); |
There was a problem hiding this comment.
Potential undefined userId in SyncMessage.
this.currentUser?.id could be undefined if currentUser is null, but SyncMessage requires userId: string. This should be guarded.
🔎 Proposed fix
private async handleSyncRequest(msg: SyncMessage): Promise<void> {
- if (!this.api || !this.currentRoom) return;
+ if (!this.api || !this.currentRoom || !this.currentUser) return;
if (msg.type !== "sync-request") return;
const events = this.api.exportEventGraph();
const requestVersion = msg.data.version || 0;
// Send only events after the requested version
const newEvents = events.slice(requestVersion);
if (newEvents.length > 0) {
this.syncAdapter?.send({
type: "sync-response",
roomId: this.currentRoom.id,
- userId: this.currentUser?.id,
+ userId: this.currentUser.id,
data: { events: newEvents, version: events.length },
timestamp: Date.now(),
});
}
}🤖 Prompt for AI Agents
In apps/playground/src/modules/collab-editor/room-manager.ts around lines
136-142, the SyncMessage is using this.currentUser?.id which can be undefined
but SyncMessage requires userId: string; guard against a missing currentUser by
checking if this.currentUser exists before sending the message—if it's absent,
log or handle the error and return (do not send), otherwise use
this.currentUser.id (non-null) when constructing the message so userId is always
a string.
| this.syncAdapter?.send({ | ||
| type: "event", | ||
| roomId: this.currentRoom.id, | ||
| userId: this.currentUser?.id, | ||
| data: latestEvent, | ||
| timestamp: Date.now(), | ||
| }); |
There was a problem hiding this comment.
Potential undefined userId in broadcast message.
Same issue as handleSyncRequest: this.currentUser?.id could be undefined. Consider adding currentUser to the guard at line 172.
🔎 Proposed fix
async handleLocalChange(
operation: "insert" | "delete",
position: number,
text?: string,
length?: number,
): Promise<void> {
- if (!this.api || !this.currentRoom) return;
+ if (!this.api || !this.currentRoom || !this.currentUser) return;Then update line 190:
- userId: this.currentUser?.id,
+ userId: this.currentUser.id,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/playground/src/modules/collab-editor/room-manager.ts around lines 187 to
193, the broadcast uses this.currentUser?.id which may be undefined; add
currentUser to the existing guard at line 172 so the method exits early if
currentUser is missing, and then change the send call at line 190 to use
this.currentUser.id (non-optional) so the message always contains a defined
userId.
| replace(position: number, length: number, text: string): void { | ||
| // Replace is a delete followed by an insert | ||
| this.delete(position, length); | ||
| this.insert(position, text); | ||
| } |
There was a problem hiding this comment.
Potential race condition in replace operation.
replace() calls delete() and insert() synchronously, but both internally call async handleLocalChange with fire-and-forget .catch(). This could result in out-of-order operations being broadcast. Consider making replace async.
🔎 Proposed fix
- replace(position: number, length: number, text: string): void {
+ async replace(position: number, length: number, text: string): Promise<void> {
// Replace is a delete followed by an insert
- this.delete(position, length);
- this.insert(position, text);
+ await this.handleLocalChange("delete", position, undefined, length);
+ await this.handleLocalChange("insert", position, text);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| replace(position: number, length: number, text: string): void { | |
| // Replace is a delete followed by an insert | |
| this.delete(position, length); | |
| this.insert(position, text); | |
| } | |
| async replace(position: number, length: number, text: string): Promise<void> { | |
| // Replace is a delete followed by an insert | |
| await this.delete(position, length); | |
| await this.insert(position, text); | |
| } |
🤖 Prompt for AI Agents
In apps/playground/src/modules/collab-editor/room-manager.ts around lines
228-232, replace() currently calls delete() and insert() synchronously which can
cause out-of-order broadcasts because both internals call async
handleLocalChange with fire-and-forget; change replace to be async and await the
completion of delete and insert so the operations are applied and broadcast in
order (update delete/insert to return the Promise from handleLocalChange if they
don't already, change replace signature to async replace(...): Promise<void>,
await delete(position,length) then await insert(position,text), and update any
callers to handle the returned Promise).
| async saveRoom(room: Room): Promise<void> { | ||
| if (this.db) { | ||
| const tx = this.db.transaction(["rooms"], "readwrite"); | ||
| await tx.objectStore("rooms").put(room); | ||
| } else { | ||
| // Fallback to localStorage | ||
| const rooms = this.getLocalStorageRooms(); | ||
| rooms[room.id] = room; | ||
| localStorage.setItem("collab-rooms", JSON.stringify(rooms)); | ||
| } | ||
| } |
There was a problem hiding this comment.
await on IDBRequest doesn't wait for completion.
IDBObjectStore.put() returns an IDBRequest, not a Promise. The await resolves immediately without waiting for the write to complete. This can cause data loss or race conditions.
🔎 Proposed fix - wrap IDBRequest in Promise
async saveRoom(room: Room): Promise<void> {
if (this.db) {
const tx = this.db.transaction(["rooms"], "readwrite");
- await tx.objectStore("rooms").put(room);
+ const request = tx.objectStore("rooms").put(room);
+ return new Promise((resolve, reject) => {
+ request.onsuccess = () => resolve();
+ request.onerror = () => reject(request.error);
+ });
} else {
// Fallback to localStorage
const rooms = this.getLocalStorageRooms();
rooms[room.id] = room;
localStorage.setItem("collab-rooms", JSON.stringify(rooms));
}
}Alternatively, consider using a helper function for all IDB operations:
private wrapRequest<T>(request: IDBRequest<T>): Promise<T> {
return new Promise((resolve, reject) => {
request.onsuccess = () => resolve(request.result);
request.onerror = () => reject(request.error);
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async saveRoom(room: Room): Promise<void> { | |
| if (this.db) { | |
| const tx = this.db.transaction(["rooms"], "readwrite"); | |
| await tx.objectStore("rooms").put(room); | |
| } else { | |
| // Fallback to localStorage | |
| const rooms = this.getLocalStorageRooms(); | |
| rooms[room.id] = room; | |
| localStorage.setItem("collab-rooms", JSON.stringify(rooms)); | |
| } | |
| } | |
| async saveRoom(room: Room): Promise<void> { | |
| if (this.db) { | |
| const tx = this.db.transaction(["rooms"], "readwrite"); | |
| const request = tx.objectStore("rooms").put(room); | |
| return new Promise((resolve, reject) => { | |
| request.onsuccess = () => resolve(); | |
| request.onerror = () => reject(request.error); | |
| }); | |
| } else { | |
| // Fallback to localStorage | |
| const rooms = this.getLocalStorageRooms(); | |
| rooms[room.id] = room; | |
| localStorage.setItem("collab-rooms", JSON.stringify(rooms)); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/playground/src/modules/collab-editor/storage.ts around lines 61 to 71,
the code uses `await tx.objectStore("rooms").put(room)` but `put` returns an
IDBRequest (not a Promise) so the await does not wait for the write to complete;
replace the direct await with a wrapped Promise that listens to the IDBRequest
`onsuccess` and `onerror` (or create a reusable `wrapRequest<T>(request:
IDBRequest<T>): Promise<T>` helper that resolves on `onsuccess` with
`request.result` and rejects on `onerror` with `request.error`), then await that
Promise (optionally also ensure the transaction completes by waiting for
`tx.oncomplete` or `tx.onerror` as needed).
| async saveDocument(doc: Document): Promise<void> { | ||
| if (this.db) { | ||
| const tx = this.db.transaction(["documents"], "readwrite"); | ||
| await tx.objectStore("documents").put(doc); | ||
| } else { | ||
| const docs = this.getLocalStorageDocuments(); | ||
| docs[doc.roomId] = doc; | ||
| localStorage.setItem("collab-documents", JSON.stringify(docs)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Same IDBRequest issue in saveDocument.
Same issue as saveRoom - the await on put() doesn't wait for completion.
🔎 Proposed fix
async saveDocument(doc: Document): Promise<void> {
if (this.db) {
const tx = this.db.transaction(["documents"], "readwrite");
- await tx.objectStore("documents").put(doc);
+ const request = tx.objectStore("documents").put(doc);
+ return new Promise((resolve, reject) => {
+ request.onsuccess = () => resolve();
+ request.onerror = () => reject(request.error);
+ });
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async saveDocument(doc: Document): Promise<void> { | |
| if (this.db) { | |
| const tx = this.db.transaction(["documents"], "readwrite"); | |
| await tx.objectStore("documents").put(doc); | |
| } else { | |
| const docs = this.getLocalStorageDocuments(); | |
| docs[doc.roomId] = doc; | |
| localStorage.setItem("collab-documents", JSON.stringify(docs)); | |
| } | |
| } | |
| async saveDocument(doc: Document): Promise<void> { | |
| if (this.db) { | |
| const tx = this.db.transaction(["documents"], "readwrite"); | |
| const request = tx.objectStore("documents").put(doc); | |
| return new Promise((resolve, reject) => { | |
| request.onsuccess = () => resolve(); | |
| request.onerror = () => reject(request.error); | |
| }); | |
| } else { | |
| const docs = this.getLocalStorageDocuments(); | |
| docs[doc.roomId] = doc; | |
| localStorage.setItem("collab-documents", JSON.stringify(docs)); | |
| } | |
| } |
| async saveUser(user: User): Promise<void> { | ||
| if (this.db) { | ||
| const tx = this.db.transaction(["users"], "readwrite"); | ||
| await tx.objectStore("users").put(user); | ||
| } else { | ||
| localStorage.setItem(`user-${user.id}`, JSON.stringify(user)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Same IDBRequest issue in saveUser.
Same issue - the await on put() doesn't wait for completion.
🔎 Proposed fix
async saveUser(user: User): Promise<void> {
if (this.db) {
const tx = this.db.transaction(["users"], "readwrite");
- await tx.objectStore("users").put(user);
+ const request = tx.objectStore("users").put(user);
+ return new Promise((resolve, reject) => {
+ request.onsuccess = () => resolve();
+ request.onerror = () => reject(request.error);
+ });
} else {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/playground/src/modules/collab-editor/storage.ts around lines 134 to 141,
the code awaits tx.objectStore("users").put(user) but that IDBRequest isn't
awaited properly; instead await the transaction completion by wrapping the
request/transaction events in a Promise. Replace the direct await with code that
calls objectStore.put(user) then returns a Promise that resolves on
tx.oncomplete (or request.onsuccess) and rejects on
tx.onerror/tx.onabort/request.onerror, and await that Promise so the saveUser
method only returns after the write is finished; keep the localStorage branch
unchanged.
| export function generateRoomId(): string { | ||
| return `room_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the generateRoomId implementations in both files
echo "=== Checking utils.ts ==="
rg -n "generateRoomId" apps/playground/src/modules/collab-editor/utils.ts -A 3
echo ""
echo "=== Checking room-manager.ts ==="
rg -n "generateRoomId" apps/playground/src/modules/collab-editor/room-manager.ts -A 3Repository: softmaple/softmaple
Length of output: 485
🏁 Script executed:
#!/bin/bash
# Check if substr vs substring matters for the logic
# substr(2, 9) extracts 9 characters starting at index 2
# substring(2, 11) extracts characters from index 2 to 11 (9 characters total)
# Let's verify the actual length of Math.random().toString(36)
python3 <<'EOF'
import random
# In JS: Math.random().toString(36) produces string like "0.abc123xyz"
# After removing "0.", we have "abc123xyz" (typically 8-17 chars, but can vary)
# For generating random hex-like string, we need consistent length
# substr(2, 9) = take 9 chars from position 2
# substring(2, 11) = from position 2 to 11 = 9 chars
# Both produce same length output but substring is preferred
print("substr(2, 9) takes 9 characters starting at index 2")
print("substring(2, 11) takes characters from index 2 to 11 (9 chars)")
print("Both are equivalent in output length")
EOFRepository: softmaple/softmaple
Length of output: 149
🏁 Script executed:
#!/bin/bash
# Search for all imports of generateRoomId to understand usage patterns
echo "=== Usages of generateRoomId ==="
rg "generateRoomId" apps/playground/src/modules/collab-editor/ -lRepository: softmaple/softmaple
Length of output: 204
Use substring instead of deprecated substr and adjust the end index accordingly.
substr is deprecated. Replace with substring(2, 11) to maintain the same 9-character output (the original substr(2, 9) takes 9 chars from position 2, while substring(2, 11) takes chars from index 2 to 11, also 9 chars).
🔎 Proposed fix
export function generateRoomId(): string {
- return `room_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
+ return `room_${Date.now()}_${Math.random().toString(36).substring(2, 11)}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function generateRoomId(): string { | |
| return `room_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; | |
| } | |
| export function generateRoomId(): string { | |
| return `room_${Date.now()}_${Math.random().toString(36).substring(2, 11)}`; | |
| } |
🤖 Prompt for AI Agents
In apps/playground/src/modules/collab-editor/utils.ts around lines 27 to 29, the
function uses the deprecated String.prototype.substr; replace it with
substring(2, 11) to produce the same nine-character slice, i.e. change the call
from .substr(2, 9) to .substring(2, 11) so the generated room id remains
room_{timestamp}_{9chars} and avoid using deprecated APIs.
| const copyRoomLink = useCallback(() => { | ||
| if (!currentRoom) return; | ||
|
|
||
| const roomLink = `${window.location.origin}/demo/online-collab-editor?room=${currentRoom.id}`; | ||
| navigator.clipboard.writeText(roomLink); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); | ||
| }, [currentRoom]); |
There was a problem hiding this comment.
Handle clipboard write errors.
navigator.clipboard.writeText can fail (e.g., missing permissions, non-HTTPS context). The promise rejection is unhandled, which could cause an unhandled promise rejection error.
🔎 Proposed fix
const copyRoomLink = useCallback(() => {
if (!currentRoom) return;
const roomLink = `${window.location.origin}/demo/online-collab-editor?room=${currentRoom.id}`;
- navigator.clipboard.writeText(roomLink);
- setCopied(true);
- setTimeout(() => setCopied(false), 2000);
+ navigator.clipboard.writeText(roomLink)
+ .then(() => {
+ setCopied(true);
+ setTimeout(() => setCopied(false), 2000);
+ })
+ .catch((err) => {
+ console.error("Failed to copy:", err);
+ });
}, [currentRoom]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const copyRoomLink = useCallback(() => { | |
| if (!currentRoom) return; | |
| const roomLink = `${window.location.origin}/demo/online-collab-editor?room=${currentRoom.id}`; | |
| navigator.clipboard.writeText(roomLink); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 2000); | |
| }, [currentRoom]); | |
| const copyRoomLink = useCallback(() => { | |
| if (!currentRoom) return; | |
| const roomLink = `${window.location.origin}/demo/online-collab-editor?room=${currentRoom.id}`; | |
| navigator.clipboard.writeText(roomLink) | |
| .then(() => { | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 2000); | |
| }) | |
| .catch((err) => { | |
| console.error("Failed to copy:", err); | |
| }); | |
| }, [currentRoom]); |
🤖 Prompt for AI Agents
In apps/playground/src/routes/demo/online-collab-editor.tsx around lines 116 to
123, the call to navigator.clipboard.writeText is unhandled and can reject
(permissions, non-HTTPS, etc.); change copyRoomLink to handle the promise
rejection: make it async (or use .then/.catch), await writeText inside
try/catch, only setCopied(true) on success, and on failure show a user-visible
fallback (e.g., try a textarea + document.execCommand('copy') fallback or
surface an error/toast) and ensure errors are logged so no unhandled promise
rejection occurs.
- Replace Math.random() with crypto.randomUUID() for secure IDs - Ensure cryptographically secure room and user ID generation - Fix ID generation in room-manager, hooks, and utils
zhyd1997
requested a deployment
to
apps/web-e2e-smoke
GitHub Actions
Waiting
zhyd1997
requested a deployment
to
apps/web-e2e
GitHub Actions
Waiting
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/playground/src/modules/collab-editor/room-manager.ts (3)
125-144: Add guard forthis.currentUserto prevent undefineduserId.Line 139 uses
this.currentUser?.idwhich could beundefinedifcurrentUseris null, butSyncMessagerequiresuserId: string.🔎 Proposed fix
private async handleSyncRequest(msg: SyncMessage): Promise<void> { - if (!this.api || !this.currentRoom) return; + if (!this.api || !this.currentRoom || !this.currentUser) return; if (msg.type !== "sync-request") return; const events = this.api.exportEventGraph(); const requestVersion = msg.data.version || 0; // Send only events after the requested version const newEvents = events.slice(requestVersion); if (newEvents.length > 0) { this.syncAdapter?.send({ type: "sync-response", roomId: this.currentRoom.id, - userId: this.currentUser?.id, + userId: this.currentUser.id, data: { events: newEvents, version: events.length }, timestamp: Date.now(), }); } }
166-198: Add guard forthis.currentUserto prevent undefineduserId.Line 190 uses
this.currentUser?.idwhich could beundefined. AddcurrentUserto the guard at line 172.🔎 Proposed fix
async handleLocalChange( operation: "insert" | "delete", position: number, text?: string, length?: number, ): Promise<void> { - if (!this.api || !this.currentRoom) return; + if (!this.api || !this.currentRoom || !this.currentUser) return; // Apply operation locally if (operation === "insert" && text) { this.api.insert(position, text); } else if (operation === "delete" && length) { this.api.delete(position, length); } // Get the latest event const events = this.api.exportEventGraph(); const latestEvent = events[events.length - 1]; if (latestEvent) { // Broadcast to other participants this.syncAdapter?.send({ type: "event", roomId: this.currentRoom.id, - userId: this.currentUser?.id, + userId: this.currentUser.id, data: latestEvent, timestamp: Date.now(), }); // Save to local storage await this.saveDocument(); } }
228-232: Fix race condition in replace operation.The
replacemethod callsdeleteandinsertsynchronously, but both internally invoke asynchandleLocalChangewith fire-and-forget error handling. This can result in out-of-order operations being broadcast to peers.🔎 Proposed fix
- replace(position: number, length: number, text: string): void { + async replace(position: number, length: number, text: string): Promise<void> { // Replace is a delete followed by an insert - this.delete(position, length); - this.insert(position, text); + await this.handleLocalChange("delete", position, undefined, length); + await this.handleLocalChange("insert", position, text); }Note: Callers of
replacewill need to handle the returned Promise.
🧹 Nitpick comments (3)
apps/playground/src/modules/collab-editor/hooks/use-collab-editor.ts (2)
30-32: Unawaited async cleanup may leave operations incomplete.The cleanup function calls
roomManager.leaveRoom()which is async, but the result isn't awaited. This could result in incomplete cleanup (e.g., disconnect messages not sent) if the component unmounts quickly.React's cleanup functions cannot be async directly, but you can fire-and-forget with error handling:
🔎 Proposed enhancement for cleanup
return () => { - roomManager.leaveRoom(); + roomManager.leaveRoom().catch(console.error); };
101-108: Handle async leaveRoom operation properly.The
leaveRoommethod is async but not awaited, which means errors won't be caught and state is reset immediately even if the operation fails or is still in progress.🔎 Proposed fix
const leaveRoom = useCallback(async () => { - await roomManager.leaveRoom(); + try { + await roomManager.leaveRoom(); + } catch (err) { + console.error("Failed to leave room:", err); + } setCurrentRoom(null); setCurrentUser(null); setText(""); setParticipants([]); setIsConnected(false); }, [roomManager]);apps/playground/src/modules/collab-editor/room-manager.ts (1)
88-93: Validate user data before type assertion.Line 90 uses
msg.data as Userwithout validation. If a malformed message arrives, this could lead to runtime errors when the data is used as aUserobject.🔎 Proposed validation
this.syncAdapter.on("user-joined", (msg: SyncMessage) => { if (msg.userId !== user.id) { - this.participants.set(msg.userId, msg.data as User); + const userData = msg.data as User; + if (userData && typeof userData.id === "string" && typeof userData.name === "string") { + this.participants.set(msg.userId, userData); + } else { + console.error("Invalid user data received:", msg.data); + return; + } this.onParticipantsChange(); } });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/playground/src/modules/collab-editor/hooks/use-collab-editor.tsapps/playground/src/modules/collab-editor/room-manager.tsapps/playground/src/modules/collab-editor/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/playground/src/modules/collab-editor/utils.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indent, double quotes, and semicolons for code formatting
Files:
apps/playground/src/modules/collab-editor/hooks/use-collab-editor.tsapps/playground/src/modules/collab-editor/room-manager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Do not use TypeScriptanytype; use proper types, union types, generics, or specific interfaces instead
When proper typing is not feasible for TypeScript errors, use@ts-expect-errorwith a descriptive comment explaining why, never use@ts-ignore
Do not use TypeScript enums; use const objects withas constassertion instead for performance and bundle-size optimization
Files:
apps/playground/src/modules/collab-editor/hooks/use-collab-editor.tsapps/playground/src/modules/collab-editor/room-manager.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Write pure functions without side effects that return the same output for the same inputs
Avoid mutating data structures; return new copies instead using spread operators, Array.map(), Object.freeze(), etc.
Leverage higher-order functions such as map, filter, and reduce for functional programming patterns
Build complex logic by composing smaller, reusable functions
Files:
apps/playground/src/modules/collab-editor/hooks/use-collab-editor.tsapps/playground/src/modules/collab-editor/room-manager.ts
🧠 Learnings (2)
📚 Learning: 2025-12-27T07:28:24.288Z
Learnt from: CR
Repo: softmaple/softmaple PR: 0
File: packages/eg-walker/CLAUDE.md:0-0
Timestamp: 2025-12-27T07:28:24.288Z
Learning: Applies to packages/eg-walker/src/crdt/**/*.ts : Query RetreatAdvanceCoordinator as the single source of truth for state rather than duplicating tracking in separate classes
Applied to files:
apps/playground/src/modules/collab-editor/room-manager.ts
📚 Learning: 2025-12-27T07:28:45.016Z
Learnt from: CR
Repo: softmaple/softmaple PR: 0
File: packages/eg-walker/AGENTS.md:0-0
Timestamp: 2025-12-27T07:28:45.016Z
Learning: Applies to packages/eg-walker/src/{eg-walker,crdt}.ts : Core algorithm implementation should be in `src/eg-walker.ts` and `src/crdt.ts`
Applied to files:
apps/playground/src/modules/collab-editor/room-manager.ts
🧬 Code graph analysis (1)
apps/playground/src/modules/collab-editor/hooks/use-collab-editor.ts (4)
apps/playground/src/modules/collab-editor/room-manager.ts (4)
RoomManager(9-270)createRoom(23-35)joinRoom(37-83)leaveRoom(246-261)apps/playground/src/modules/collab-editor/types.ts (2)
Room(1-6)User(19-23)apps/playground/src/modules/collab-editor/utils.ts (1)
getRandomColor(8-22)apps/playground/src/modules/collab-editor/storage.ts (1)
storage(170-170)
🔇 Additional comments (11)
apps/playground/src/modules/collab-editor/hooks/use-collab-editor.ts (3)
35-64: LGTM!The
createRoomcallback is well-structured with proper error handling, secure ID generation usingcrypto.randomUUID(), and appropriate state management.
66-99: LGTM!The
joinRoomcallback follows the same solid pattern ascreateRoomwith proper error handling and state management.
110-122: LGTM!The hook's public API is well-designed, exposing all necessary state and methods for collaborative editing.
apps/playground/src/modules/collab-editor/room-manager.ts (8)
1-21: LGTM!Imports and initialization are properly structured.
23-35: LGTM!The
createRoommethod properly creates and persists the room, then joins it automatically.
37-83: LGTM!The
joinRoommethod properly handles room initialization, event replay, and synchronization setup with appropriate error handling.
146-164: LGTM!The
handleSyncResponsemethod properly applies remote events with error handling and version tracking.
200-212: LGTM!The
saveDocumentmethod properly persists the current document state.
214-226: LGTM!The
getText,insert, anddeletemethods are properly implemented with error handling.
234-261: LGTM!The getter methods and
leaveRoomcleanup logic are properly implemented.
263-269: LGTM! Security issue resolved.The
generateRoomIdmethod now correctly usescrypto.randomUUID()instead ofMath.random(), addressing the insecure randomness issue flagged in previous reviews.
| export function useCollabEditor(wsUrl?: string) { | ||
| const [roomManager] = useState(() => new RoomManager(wsUrl)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check usage patterns of useCollabEditor
rg -nP --type=ts --type=tsx -C5 'useCollabEditor\s*\(' -g '!use-collab-editor.ts'Repository: softmaple/softmaple
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Check usage patterns of useCollabEditor
rg -nP -C5 'useCollabEditor\s*\(' --type-add 'typescriptx:*.tsx' -t ts -t typescriptx -g '!use-collab-editor.ts'Repository: softmaple/softmaple
Length of output: 987
🏁 Script executed:
#!/bin/bash
# Read the full hook implementation
cat -n apps/playground/src/modules/collab-editor/hooks/use-collab-editor.tsRepository: softmaple/softmaple
Length of output: 4129
🏁 Script executed:
#!/bin/bash
# Find and read RoomManager class
find apps/playground/src/modules/collab-editor -name "*.ts" -o -name "*.tsx" | head -20Repository: softmaple/softmaple
Length of output: 537
🏁 Script executed:
#!/bin/bash
# Read RoomManager class
cat -n apps/playground/src/modules/collab-editor/room-manager.tsRepository: softmaple/softmaple
Length of output: 9018
The wsUrl parameter is not used in practice and the hook should either be simplified or properly support dynamic URL changes.
The wsUrl parameter is optional and never passed in actual usage (the hook is called without arguments). While the concern about RoomManager not updating if wsUrl changed is technically valid, the hook was not designed to support dynamic URL changes—it lacks wsUrl in the dependency arrays.
If dynamic wsUrl support is not needed, remove the parameter to simplify the API. If it should be supported, add wsUrl to the useEffect and callback dependencies to ensure RoomManager is properly recreated when the URL changes.
🤖 Prompt for AI Agents
In apps/playground/src/modules/collab-editor/hooks/use-collab-editor.ts around
lines 7-8, the optional wsUrl parameter is never used and creates a misleading
API; remove the wsUrl parameter from the hook signature and stop passing it into
RoomManager (construct RoomManager with its internal/default URL), and update
any call sites and types that referenced the old signature; alternatively, if
you want dynamic URLs instead, add wsUrl to the dependency arrays (useState
initializer replacement with useEffect or useMemo to recreate RoomManager when
wsUrl changes) and include wsUrl in any callback/useEffect dependency lists so
the RoomManager is recreated correctly on URL change.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.