[TASK-11342] feat: charges websocket #857
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
## Walkthrough
The changes introduce WebSocket-based real-time transaction updates to the application. This includes updating environment variables for API and WebSocket URLs, adding a new WebSocket service and hook, and modifying the `HomeHistory` component to merge live transaction data with initially fetched history. No changes were made to error or loading state handling.
## Changes
| Files/Paths | Change Summary |
|--------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `.env.example` | Updated `PEANUT_API_URL` and `NEXT_PUBLIC_PEANUT_API_URL` to use the `peanut.me` domain. Added a new `NEXT_PUBLIC_PEANUT_WS_URL` variable for the WebSocket endpoint. |
| `src/components/Home/HomeHistory.tsx` | Modified `HomeHistory` to integrate real-time transaction updates via WebSocket, merging live entries with fetched history and updating filtering, rendering, and state logic accordingly. |
| `src/hooks/useWebSocket.ts` | Added new `useWebSocket` React hook to manage WebSocket connection lifecycle, track status, handle incoming messages, and provide control functions and history state to components. |
| `src/services/websocket.ts` | Introduced `PeanutWebSocket` class implementing WebSocket connection with automatic reconnection, heartbeat pings, event-driven message handling, and a singleton accessor for shared usage. |
## Suggested reviewers
- kushagrasaratheNote ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
.env.example (1)
17-37: 🛠️ Refactor suggestionDuplicate
NEXT_PUBLIC_INFURA_API_KEYentry may cause subtle config bugs
NEXT_PUBLIC_INFURA_API_KEYis declared twice (line 17 and line 37).
When developers source this file withbash -aor similar, the second assignment silently overrides the first one, which can be confusing and lead to environment–specific “works-on-my-machine” issues.-export NEXT_PUBLIC_INFURA_API_KEY="" ... -export NEXT_PUBLIC_INFURA_API_KEY=""Remove the duplicate or add an inline comment clarifying why two assignments are necessary.
🧹 Nitpick comments (7)
src/services/websocket.ts (2)
145-160: Exponential back-off grows without an upper cap
this.reconnectDelay * 2^ndoubles indefinitely until the hard limit ofmaxReconnectAttempts, but ifmaxReconnectAttemptsis increased in the future you may quickly exceed browser limits (~24 days). Consider clamping to, say, 30 s.Not critical for
maxReconnectAttempts = 5, just a maintainability note.
67-79: Memory leak risk when many components add listeners
on/offstore callbacks in aSet, butdisconnect()never purgeseventListeners. If the singleton lives for the entire session, stale callbacks accumulate as pages mount/unmount.Either:
- Clear
eventListenersinsidedisconnect(), or- Document that consumers must call
offinuseEffectclean-ups.src/components/Home/HomeHistory.tsx (2)
40-48: MutatingwsEntrybreaks immutability & may trigger unexpected re-rendersYou directly mutate objects from the hook (
wsEntry.extraData = …). If another component relies on referential equality of that array, this introduces subtle bugs.const cloned = { ...wsEntry, extraData: { ...wsEntry.extraData, usdAmount: wsEntry.amount.toString() } } entries.unshift(cloned)Avoid in-place mutation.
145-153:filteredEntriesis recomputed inside every iteration
filteredEntrieshas the same value for each map pass; compute it once outside the loop to avoid unnecessary array allocations.const displayEntries = combinedEntries.filter( (entry) => !pendingRequests.some((r) => r.uuid === entry.uuid) ) displayEntries.map((item, index) => { // … })Tiny optimisation but keeps the render path clean.
src/hooks/useWebSocket.ts (3)
20-20: Consider adding pagination or limit for history entriesThe current implementation stores all history entries in memory without any limit, which could lead to excessive memory usage if the connection stays open for a long time and receives many entries.
Consider either:
- Implementing a max length for the history entries array
- Adding pagination support
- Using a more memory-efficient data structure
Example implementation with a max length:
-const [historyEntries, setHistoryEntries] = useState<HistoryEntry[]>([]) +const [historyEntries, setHistoryEntries] = useState<HistoryEntry[]>([]) +const MAX_HISTORY_ENTRIES = 50 // Or any appropriate numberThen in the handleHistoryEntry function:
-setHistoryEntries((prev) => [entry, ...prev]) +setHistoryEntries((prev) => [entry, ...prev].slice(0, MAX_HISTORY_ENTRIES))
29-41: Potential duplication in WebSocket initializationThe WebSocket instance is initialized both in the
connectfunction and at the beginning of the main effect. While this approach works becausegetWebSocketInstancereturns a singleton, it could be confusing and potentially lead to issues if the implementation ofgetWebSocketInstancechanges.Consider refactoring to initialize the WebSocket only once in the main effect, and have the connect function use the ref:
const connect = useCallback(() => { try { - const ws = getWebSocketInstance(username) - wsRef.current = ws + if (!wsRef.current) { + wsRef.current = getWebSocketInstance(username) + } + const ws = wsRef.current setStatus('connecting') ws.connect() } catch (error) { console.error('Failed to connect to WebSocket:', error) setStatus('error') } }, [username])Also applies to: 51-55
16-115: Add support for testing with mocksThe current implementation doesn't have an easy way to mock the WebSocket for testing purposes. This could make unit testing components that use this hook challenging.
Consider adding an optional parameter to inject a mock WebSocket instance for testing:
-export const useWebSocket = (options: UseWebSocketOptions = {}) => { +export const useWebSocket = ( + options: UseWebSocketOptions = {}, + websocketFactory: (username?: string) => PeanutWebSocket = getWebSocketInstance +) => { const { autoConnect = true, username, onHistoryEntry, onConnect, onDisconnect, onError } = options // ... // Then replace all instances of getWebSocketInstance with websocketFactory // For example: const connect = useCallback(() => { try { - const ws = getWebSocketInstance(username) + const ws = websocketFactory(username) wsRef.current = ws // ...This allows tests to inject a mock implementation:
// In a test file test('useWebSocket handles history entries correctly', () => { const mockWs = { connect: jest.fn(), disconnect: jest.fn(), on: jest.fn(), off: jest.fn(), // other methods }; const mockFactory = () => mockWs; const { result } = renderHook(() => useWebSocket({}, mockFactory)); // Now you can test the hook with the mock });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.env.example(1 hunks)src/components/Home/HomeHistory.tsx(6 hunks)src/hooks/useWebSocket.ts(1 hunks)src/services/websocket.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/hooks/useWebSocket.ts (2)
src/hooks/useTransactionHistory.ts (1)
HistoryEntry(31-67)src/services/websocket.ts (5)
PeanutWebSocket(8-187)connect(24-44)getWebSocketInstance(192-200)disconnect(46-65)handleError(126-129)
src/services/websocket.ts (1)
src/hooks/useTransactionHistory.ts (1)
HistoryEntry(31-67)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (2)
.env.example (1)
1-3: Confirm the staging URL & protocol are correctMoving from
peanut.to→peanut.meand adding awss://endpoint looks intentional, but please make sure:
- DNS + TLS are already configured for
api.staging.peanut.me.- The backend actually accepts WebSocket traffic on that port / path; otherwise initial socket connections will fail silently at runtime.
No action needed if you already verified this.
src/hooks/useWebSocket.ts (1)
1-115: Overall implementation is well-structuredThe hook is implemented with good practices:
- Uses appropriate React hooks (useState, useCallback, useEffect, useRef)
- Properly manages closures with refs to prevent stale callbacks
- Has clear separation of concerns
- Includes helpful comments
- Carefully manages WebSocket connection lifecycle
- Allows connection sharing between components
jjramirezn
force-pushed
the
feat/charges-websocket
branch
from
95e9ad7 to
78435a5
Compare
| // WebSocket for real-time updates | ||
| const { historyEntries: wsHistoryEntries } = useWebSocket({ | ||
| username, // Pass the username to the WebSocket hook | ||
| onHistoryEntry: useCallback(() => { |
| export class PeanutWebSocket { | ||
| private socket: WebSocket | null = null | ||
| private pingInterval: NodeJS.Timeout | null = null | ||
| private reconnectTimeout: NodeJS.Timeout | null = null | ||
| private eventListeners: Map<string, Set<(data: any) => void>> = new Map() | ||
| private isConnected = false | ||
| private reconnectAttempts = 0 | ||
| private readonly maxReconnectAttempts = 5 | ||
| private readonly reconnectDelay = 3000 // 3 seconds | ||
| private readonly pingIntervalTime = 30000 // 30 seconds |
There was a problem hiding this comment.
would be nice to justify these vars. is 3s normal? 5 attempts? whats reasonable
| case 'pong': | ||
| // Server responded to our ping | ||
| this.emit('pong', null) | ||
| break |
There was a problem hiding this comment.
to anyone that is listening to this evenemitter, I emit that I received a pong
| export const getWebSocketInstance = (username?: string): PeanutWebSocket => { | ||
| if (!websocketInstance && typeof window !== 'undefined') { | ||
| const wsUrl = process.env.NEXT_PUBLIC_PEANUT_WS_URL || '' | ||
| const path = username ? `/ws/charges/${username}` : '/ws/charges' |
There was a problem hiding this comment.
thought: we should also have an endpoint for /ws/ that just gives us all events we need for a single user
No description provided.