Conversation
WalkthroughIntroduces a new WebSocket hook (useWaveWebSocket) with reconnect and subscription logic, refactors useWaveIsTyping to consume raw socket messages, adds a subscription manager, and updates/extends tests accordingly. Removes prior WebSocket abstraction usage in typing logic and stops mocking Math.random in a WebSocketProvider test. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as UI Component
participant UWW as useWaveWebSocket
participant WS as WebSocket
participant SM as SubscriptionMgr (optional)
UI->>UWW: useWaveWebSocket(waveId)
activate UWW
UWW->>WS: new WebSocket(url)
note right of WS: Pending connection
WS-->>UWW: open
UWW->>WS: send SUBSCRIBE_TO_WAVE(waveId)
SM-->>UWW: (if used) waves with count>0
UWW->>WS: send SUBSCRIBE for each pending
WS-->>UWW: close/error
UWW->>UWW: schedule reconnect (≤20 attempts, 2s)
UWW->>WS: new WebSocket(url) (on retry)
UI-->>UWW: disconnect()
UWW->>UWW: disable reconnection
UWW->>WS: close()
deactivate UWW
sequenceDiagram
autonumber
actor UI as UI Component
participant UWT as useWaveIsTyping
participant UWW as useWaveWebSocket
participant WS as WebSocket
participant Timer as Cleanup Timer
UI->>UWT: useWaveIsTyping({ waveId, myHandle })
UWT->>UWW: init socket for waveId
WS-->>UWT: message USER_IS_TYPING
UWT->>UWT: update typersRef (ignore myHandle/mismatched wave)
Timer->>UWT: every 1s prune >5s old
UWT->>UI: setState(new typing string if changed)
WS-->>UWT: message DROP_UPDATE
UWT->>UWT: remove author from typersRef
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
services/websocket/useWaveSubscriptionManager.ts (2)
31-37: Reduce noisy console logging in productionGate debug logs behind an env flag (e.g., NODE_ENV !== "production") or a feature flag to avoid shipping verbose logs.
Also applies to: 53-56, 58-66, 85-86, 112-118, 130-134
1-22: Coding guideline: remove inline comments in ts/tsxThis file contains inline comments. Repo guideline for ts/tsx says not to include comments in code. Please strip or move to docs.
As per coding guidelines
Also applies to: 31-37, 58-66, 90-99, 104-121, 124-136
hooks/useWaveWebSocket.ts (2)
50-55: Standardize subscribe payload to match the rest of the appTo align with the subscription manager and avoid protocol drift, consider including subscribe: true on initial subscribe.
ws.send( JSON.stringify({ type: WsMessageType.SUBSCRIBE_TO_WAVE, + subscribe: true, wave_id: waveId, }) );
19-24: Remove or fix misleading commentComment says it sends “hello world”, but it sends a subscribe message. Given repo guidelines discourage comments in ts/tsx, remove this block.
As per coding guidelines
__tests__/hooks/useWaveWebSocket.test.ts (1)
66-74: Assert no reconnect after disconnectTighten this test by asserting setTimeout isn’t called after disconnect.
it("disconnect stops reconnecting", () => { - const { result } = renderHook(() => useWaveWebSocket("wave1")); + const spy = jest.spyOn(globalThis, "setTimeout"); + const { result } = renderHook(() => useWaveWebSocket("wave1")); const instance = (globalThis.WebSocket as jest.Mock).mock.results[0] .value as MockWebSocket; act(() => { result.current.disconnect(); }); expect(instance.close).toHaveBeenCalled(); + expect(spy).not.toHaveBeenCalled(); });__tests__/hooks/useWaveIsTyping.test.tsx (1)
5-14: Improve test isolation and add a couple of key cases
- Reset listeners and timers per test to avoid bleed-through.
- Add cases for ignoring myHandle, DROP_UPDATE removal, and 2+ typers formatting.
-const listeners: any[] = []; +let listeners: any[] = []; +beforeEach(() => { + listeners = []; + jest.useFakeTimers(); +}); +afterEach(() => { + jest.useRealTimers(); +});Also applies to: 16-28
hooks/useWaveIsTyping.ts (1)
52-61: Coding guideline: remove inline comments in ts/tsxThis file is rich in inline comments. Repo guideline for ts/tsx asks to avoid comments in code. Please prune or move into docstrings/docs.
As per coding guidelines
Also applies to: 68-79, 80-109, 111-129
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
__tests__/hooks/useWaveIsTyping.test.tsx(1 hunks)__tests__/hooks/useWaveWebSocket.test.ts(1 hunks)__tests__/services/websocket/WebSocketProvider.test.tsx(0 hunks)hooks/useWaveIsTyping.ts(2 hunks)hooks/useWaveWebSocket.ts(1 hunks)services/websocket/useWaveSubscriptionManager.ts(1 hunks)
💤 Files with no reviewable changes (1)
- tests/services/websocket/WebSocketProvider.test.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsFollow existing code style and naming conventions
Files:
hooks/useWaveWebSocket.tsservices/websocket/useWaveSubscriptionManager.tshooks/useWaveIsTyping.ts__tests__/hooks/useWaveIsTyping.test.tsx__tests__/hooks/useWaveWebSocket.test.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingUse React functional components with hooks for UI components
Files:
__tests__/hooks/useWaveIsTyping.test.tsx
__tests__/**
📄 CodeRabbit inference engine (tests/AGENTS.md)
Place Jest test suites under the
__tests__directory mirroring source folders (e.g., components, contexts, hooks, utils)
Files:
__tests__/hooks/useWaveIsTyping.test.tsx__tests__/hooks/useWaveWebSocket.test.ts
{**/__tests__/**,**/*.test.{ts,tsx}}
📄 CodeRabbit inference engine (AGENTS.md)
{**/__tests__/**,**/*.test.{ts,tsx}}: If coverage on a modified file is below 80%, add meaningful tests to raise it to at least 80%
Mock external dependencies and APIs in tests
Files:
__tests__/hooks/useWaveIsTyping.test.tsx__tests__/hooks/useWaveWebSocket.test.ts
**/__tests__/**
📄 CodeRabbit inference engine (AGENTS.md)
Place tests in
__tests__/directories when organizing standalone test suites
Files:
__tests__/hooks/useWaveIsTyping.test.tsx__tests__/hooks/useWaveWebSocket.test.ts
**/*.test.tsx
📄 CodeRabbit inference engine (AGENTS.md)
When co-locating tests with components, name them
ComponentName.test.tsx
Files:
__tests__/hooks/useWaveIsTyping.test.tsx
🧬 Code graph analysis (3)
hooks/useWaveIsTyping.ts (4)
generated/models/ApiProfileMin.ts (1)
ApiProfileMin(16-133)hooks/useWaveWebSocket.ts (1)
useWaveWebSocket(25-111)helpers/Types.tsx (2)
WsTypingMessage(331-338)WsDropUpdateMessage(340-343)helpers/time.ts (1)
now(7-9)
__tests__/hooks/useWaveIsTyping.test.tsx (1)
hooks/useWaveIsTyping.ts (1)
useWaveIsTyping(62-135)
__tests__/hooks/useWaveWebSocket.test.ts (1)
hooks/useWaveWebSocket.ts (1)
useWaveWebSocket(25-111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Performance