feat: implement real-time event streaming #7
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| POSTGRES_URL=postgres://postgres:postgres@localhost:5432/forge | ||
| BACKEND_URL=http://localhost:8080 | ||
| API_KEY=1234567890 | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,66 +1,73 @@ | ||||||||||||||
| { | ||||||||||||||
| "name": "riven-frontend", | ||||||||||||||
| "version": "1.0.0", | ||||||||||||||
| "private": false, | ||||||||||||||
| "scripts": { | ||||||||||||||
| "dev": "next dev --turbopack", | ||||||||||||||
| "build": "next build", | ||||||||||||||
| "start": "next start", | ||||||||||||||
| "lint": "next lint", | ||||||||||||||
| "lint:fix": "next lint --fix", | ||||||||||||||
| "typecheck": "tsc --noEmit", | ||||||||||||||
| "format": "prettier --check \"**/*.{ts,tsx}\" --cache", | ||||||||||||||
| "format:fix": "prettier --write \"**/*.{ts,tsx}\" --cache", | ||||||||||||||
| "db:push": "drizzle-kit push", | ||||||||||||||
| "db:studio": "drizzle-kit studio", | ||||||||||||||
| "generate-client": "openapi-ts", | ||||||||||||||
| "prepare": "next-ws patch" | ||||||||||||||
| }, | ||||||||||||||
| "dependencies": { | ||||||||||||||
| "@formkit/auto-animate": "^0.8.2", | ||||||||||||||
|
There was a problem hiding this comment. 🧹 Nitpick (assertive) Use more specific version constraints. The caret ( Consider using exact versions or tilde ( - "@formkit/auto-animate": "^0.8.2",
- "@leo1305/custom-next-ws": "^1.0.0",
- "@types/ws": "^8.5.14",
- "ws": "^8.18.0",
+ "@formkit/auto-animate": "~0.8.2",
+ "@leo1305/custom-next-ws": "1.0.0",
+ "@types/ws": "~8.5.14",
+ "ws": "~8.18.0",Also applies to: 20-20, 26-26, 42-42 |
||||||||||||||
| "@hey-api/client-fetch": "^0.8.1", | ||||||||||||||
| "@leo1305/custom-next-ws": "^1.0.0", | ||||||||||||||
| "@radix-ui/react-checkbox": "^1.1.3", | ||||||||||||||
| "@radix-ui/react-dropdown-menu": "^2.1.5", | ||||||||||||||
| "@radix-ui/react-label": "^2.1.2", | ||||||||||||||
| "@radix-ui/react-select": "^2.1.5", | ||||||||||||||
| "@radix-ui/react-toast": "^1.2.5", | ||||||||||||||
| "@tailwindcss/postcss": "^4.0.3", | ||||||||||||||
| "@types/ws": "^8.5.14", | ||||||||||||||
| "bcryptjs": "^2.4.3", | ||||||||||||||
| "class-variance-authority": "^0.7.1", | ||||||||||||||
| "drizzle-orm": "^0.39.3", | ||||||||||||||
| "jose": "^5.9.6", | ||||||||||||||
| "lucide-react": "^0.475.0", | ||||||||||||||
| "next": "^15.2.0-canary.54", | ||||||||||||||
| "next-themes": "^0.4.4", | ||||||||||||||
| "postgres": "^3.4.5", | ||||||||||||||
| "react": "19.0.0", | ||||||||||||||
| "react-dom": "19.0.0", | ||||||||||||||
|
Comment on lines
+38
to
+39
There was a problem hiding this comment. ❓ Verification inconclusiveUpdate React version to match Next.js requirements. The React version (19.0.0) is outdated and may not be compatible with Next.js 15. 🌐 Web query: 💡 Result: Next.js 15's compatible React version depends on which routing system you use: App Router Pages Router Key Details
¹React 19 reached stable status in Next.js 15.1[5] Migration Considerations
The architecture split reflects Vercel's strategy to push React innovations through App Router while maintaining enterprise stability via Pages Router[1][4]. Developers should choose based on project requirements: bleeding-edge features vs dependency stability[1][4]. Citations:
React Version Compatibility – Verify Your Next.js Routing Setup The current React version is set to 19.0.0 in package.json which aligns with Next.js 15's requirements if you are using the App Router (which mandates React 19+). However, if your project relies on the Pages Router, you should consider downgrading to React 18 to ensure optimal compatibility and stability.
|
||||||||||||||
| "sonner": "^1.7.4", | ||||||||||||||
| "tailwind-merge": "^3.0.1", | ||||||||||||||
| "tailwindcss": "^4.0.3", | ||||||||||||||
| "vaul": "^1.1.2", | ||||||||||||||
| "ws": "^8.18.0", | ||||||||||||||
| "zod": "^3.24.2" | ||||||||||||||
| }, | ||||||||||||||
| "devDependencies": { | ||||||||||||||
| "@eslint/js": "^9.19.0", | ||||||||||||||
| "@hey-api/openapi-ts": "^0.64.1", | ||||||||||||||
| "@ianvs/prettier-plugin-sort-imports": "^4.4.1", | ||||||||||||||
|
There was a problem hiding this comment. 🛠️ Refactor suggestion Remove duplicate import sorting plugins. Two different import sorting plugins are included:
Choose one plugin to avoid conflicts: "@hey-api/openapi-ts": "^0.64.1",
"@ianvs/prettier-plugin-sort-imports": "^4.4.1",
"@next/bundle-analyzer": "^15.1.6",
"@next/eslint-plugin-next": "^15.1.6",
- "@trivago/prettier-plugin-sort-imports": "^5.2.2",
"@types/bcryptjs": "^2.4.6",Also applies to: 53-53 |
||||||||||||||
| "@next/bundle-analyzer": "^15.1.6", | ||||||||||||||
| "@next/eslint-plugin-next": "^15.1.6", | ||||||||||||||
| "@trivago/prettier-plugin-sort-imports": "^5.2.2", | ||||||||||||||
| "@types/bcryptjs": "^2.4.6", | ||||||||||||||
| "@types/node": "^22.13.0", | ||||||||||||||
| "@types/react": "19.0.8", | ||||||||||||||
| "@types/react-dom": "19.0.3", | ||||||||||||||
| "drizzle-kit": "^0.30.4", | ||||||||||||||
| "eslint": "^9.19.0", | ||||||||||||||
| "eslint-config-next": "15.1.6", | ||||||||||||||
| "eslint-config-prettier": "^10.0.1", | ||||||||||||||
| "eslint-plugin-import": "^2.31.0", | ||||||||||||||
| "eslint-plugin-promise": "^7.2.1", | ||||||||||||||
| "eslint-plugin-react": "^7.37.4", | ||||||||||||||
| "eslint-plugin-tailwindcss": "^3.18.0", | ||||||||||||||
| "globals": "^15.14.0", | ||||||||||||||
| "postcss": "^8.5.1", | ||||||||||||||
| "prettier": "^3.4.2", | ||||||||||||||
| "prettier-plugin-tailwindcss": "^0.6.11", | ||||||||||||||
| "typescript": "^5.7.3", | ||||||||||||||
| "typescript-eslint": "^8.22.0" | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,37 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { env } from 'node:process' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const websocketUrl = (env.BACKEND_URL || 'http://localhost:8080').replace(/^http/, 'ws') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const apiKey = env.API_KEY | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+3
to
+4
There was a problem hiding this comment. Add validation for required environment variables. The code assumes Add environment variable validation: const websocketUrl = (env.BACKEND_URL || 'http://localhost:8080').replace(/^http/, 'ws')
-const apiKey = env.API_KEY
+const apiKey = env.API_KEY
+if (!apiKey) {
+ throw new Error('API_KEY environment variable is required')
+}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function SOCKET( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client: import('ws').WebSocket, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request: import('http').IncomingMessage, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| server: import('ws').WebSocketServer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { params }: { params: { topic: string } } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+6
to
+11
There was a problem hiding this comment. Add input validation for topic parameter. The topic parameter is used directly without validation, which could lead to security vulnerabilities. Add input validation to prevent injection attacks and ensure the topic follows expected format: export function SOCKET(
client: import('ws').WebSocket,
request: import('http').IncomingMessage,
server: import('ws').WebSocketServer,
{ params }: { params: { topic: string } }
) {
+ // Validate topic parameter
+ if (!/^[a-zA-Z0-9_-]+$/.test(params.topic)) {
+ client.close(1008, 'Invalid topic format');
+ return;
+ }📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log('A client connected') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. 🧹 Nitpick (assertive) Enhance logging with structured data. Basic console logging doesn't provide enough context for production monitoring. Add structured logging with relevant metadata: - console.log('A client connected')
+ console.log('WebSocket client connected', {
+ topic: params.topic,
+ timestamp: new Date().toISOString(),
+ clientId: request.headers['x-client-id'] || 'unknown'
+ })📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const nativeWebSocket = new WebSocket(`${websocketUrl}/api/v1/ws/${params.topic}?api_key=${apiKey}`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client.onmessage = (message) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nativeWebSocket.send(message.data.toString()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+16
to
+18
There was a problem hiding this comment. Add message size validation. The code forwards messages without validating their size, which could lead to memory issues. Add message size validation: client.onmessage = (message) => {
+ const MAX_MESSAGE_SIZE = 1024 * 1024; // 1MB
+ if (message.data.length > MAX_MESSAGE_SIZE) {
+ client.close(1009, 'Message too large');
+ return;
+ }
nativeWebSocket.send(message.data.toString())
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nativeWebSocket.onmessage = (message) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client.send(message.data) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+14
to
+22
There was a problem hiding this comment. Implement error handling for WebSocket connection. The native WebSocket connection lacks proper error handling for connection failures and invalid API key responses. Add error handling for the native WebSocket connection: - const nativeWebSocket = new WebSocket(`${websocketUrl}/api/v1/ws/${params.topic}?api_key=${apiKey}`)
+ const nativeWebSocket = new WebSocket(`${websocketUrl}/api/v1/ws/${params.topic}?api_key=${apiKey}`)
+
+ // Add connection timeout
+ const connectionTimeout = setTimeout(() => {
+ if (nativeWebSocket.readyState !== WebSocket.OPEN) {
+ nativeWebSocket.close();
+ client.close(1006, 'Connection timeout');
+ }
+ }, 5000);
+
+ nativeWebSocket.onopen = () => {
+ clearTimeout(connectionTimeout);
+ }
client.onmessage = (message) => {
+ if (nativeWebSocket.readyState !== WebSocket.OPEN) {
+ client.close(1006, 'Backend connection lost');
+ return;
+ }
nativeWebSocket.send(message.data.toString())
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nativeWebSocket.onerror = (error) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error('WebSocket error:', error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
+26
There was a problem hiding this comment. 🛠️ Refactor suggestion Enhance error handling and logging. The error handling is basic and might miss important error details. Improve error handling with more detailed logging: nativeWebSocket.onerror = (error) => {
- console.error('WebSocket error:', error)
+ const errorMessage = error instanceof Error ? error.message : 'Unknown error';
+ console.error('WebSocket error:', {
+ error: errorMessage,
+ topic: params.topic,
+ timestamp: new Date().toISOString()
+ });
+ client.close(1006, errorMessage);
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client.onclose = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log('client closed') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nativeWebSocket.close() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nativeWebSocket.onclose = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log('nativeWebSocket closed') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client.close() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,103 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||
| 'use client' | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| import { useEffect, useState } from 'react' | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| import { Badge } from '@/components/ui/badge' | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { useAutoAnimate } from '@formkit/auto-animate/react' | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| type EventData = { | ||||||||||||||||||||||||||||||||||||||||||||||||
| [key: string]: string[] | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| function getNewWebSocket() { | ||||||||||||||||||||||||||||||||||||||||||||||||
| return new WebSocket('/api/ws/event_update/socket') | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+14
There was a problem hiding this comment. 🧹 Nitpick (assertive) Configure WebSocket URL based on environment. The WebSocket URL is hardcoded which could cause issues in different environments (development, staging, production). Consider using environment variables: +const WS_URL = process.env.NEXT_PUBLIC_WS_URL || '/api/ws/event_update/socket'
+
function getNewWebSocket() {
- return new WebSocket('/api/ws/event_update/socket')
+ return new WebSocket(WS_URL)
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| export default function EventsPage() { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const [events, setEvents] = useState<EventData | undefined>() | ||||||||||||||||||||||||||||||||||||||||||||||||
| const [status, setStatus] = useState('Disconnected') | ||||||||||||||||||||||||||||||||||||||||||||||||
| const [ws, setWs] = useState<WebSocket | undefined>() | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+17
to
+19
There was a problem hiding this comment. 🧹 Nitpick (assertive) Consider using a reducer for complex state management. Multiple related state variables could be better managed using useReducer. Consider refactoring to use useReducer: - const [events, setEvents] = useState<EventData | undefined>()
- const [status, setStatus] = useState('Disconnected')
- const [ws, setWs] = useState<WebSocket | undefined>()
+ const [state, dispatch] = useReducer(reducer, {
+ events: undefined,
+ status: 'Disconnected',
+ ws: undefined
+ })
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const [listRef] = useAutoAnimate() | ||||||||||||||||||||||||||||||||||||||||||||||||
| const [itemsRef] = useAutoAnimate() | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const websocket = getNewWebSocket() | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| websocket.onopen = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| setStatus('Connected') | ||||||||||||||||||||||||||||||||||||||||||||||||
| console.log('Connected to WebSocket') | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| websocket.onclose = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| setStatus('Disconnected') | ||||||||||||||||||||||||||||||||||||||||||||||||
| console.log('Disconnected from WebSocket') | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| // Attempt to reconnect after 3 seconds | ||||||||||||||||||||||||||||||||||||||||||||||||
| setTimeout(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| setStatus('Reconnecting...') | ||||||||||||||||||||||||||||||||||||||||||||||||
| setWs(undefined) | ||||||||||||||||||||||||||||||||||||||||||||||||
| const newWebsocket = getNewWebSocket() | ||||||||||||||||||||||||||||||||||||||||||||||||
| setWs(newWebsocket) | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, 3000) | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| websocket.onerror = (error) => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| console.error('WebSocket error:', error) | ||||||||||||||||||||||||||||||||||||||||||||||||
| setStatus('Error') | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| websocket.onmessage = (event) => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const data = JSON.parse(event.data) | ||||||||||||||||||||||||||||||||||||||||||||||||
| console.log('Received data:', data) | ||||||||||||||||||||||||||||||||||||||||||||||||
| setEvents(data.data) | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| setWs(websocket) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| // Cleanup on component unmount | ||||||||||||||||||||||||||||||||||||||||||||||||
| return () => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (websocket) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| websocket.close() | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+58
to
+62
There was a problem hiding this comment. 🧹 Nitpick (assertive) Enhance WebSocket cleanup. The cleanup function could be more robust by handling additional edge cases. Consider enhancing the cleanup: return () => {
if (websocket) {
+ // Remove all event listeners to prevent memory leaks
+ websocket.onclose = null;
+ websocket.onerror = null;
+ websocket.onmessage = null;
+ websocket.onopen = null;
+
+ // Only close if the connection is still open
+ if (websocket.readyState === WebSocket.OPEN) {
websocket.close()
+ }
}
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
| }, []) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||
| <> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div className='absolute top-4 right-4'> | ||||||||||||||||||||||||||||||||||||||||||||||||
| {status === 'Connected' && <Badge variant={'secondary'}>Connected</Badge>} | ||||||||||||||||||||||||||||||||||||||||||||||||
| {status === 'Disconnected' && <Badge variant={'destructive'}>Disconnected</Badge>} | ||||||||||||||||||||||||||||||||||||||||||||||||
| {status === 'Reconnecting...' && <Badge variant={'gray'}>Reconnecting...</Badge>} | ||||||||||||||||||||||||||||||||||||||||||||||||
| {status === 'Error' && <Badge variant={'destructive'}>Error</Badge>} | ||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+67
to
+72
There was a problem hiding this comment. 🧹 Nitpick (assertive) Add initial connecting state. The component lacks a visual indicator for the initial connection attempt. Add a connecting state: <div className='absolute top-4 right-4'>
+ {status === 'Connecting...' && (
+ <Badge variant={'secondary'}>
+ <span className="animate-pulse">Connecting...</span>
+ </Badge>
+ )}
{status === 'Connected' && <Badge variant={'secondary'}>Connected</Badge>}
{status === 'Disconnected' && <Badge variant={'destructive'}>Disconnected</Badge>}
{status === 'Reconnecting...' && <Badge variant={'gray'}>Reconnecting...</Badge>}
{status === 'Error' && <Badge variant={'destructive'}>Error</Badge>}
</div>📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
| <div className='container mx-auto p-6'> | ||||||||||||||||||||||||||||||||||||||||||||||||
| {events === undefined && <h1 className='text-xl font-bold'>Loading...</h1>} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| {events === null && <h1 className='text-xl font-bold text-red-600'>Error</h1>} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+74
to
+76
There was a problem hiding this comment. 🛠️ Refactor suggestion Add loading state timeout. The loading state could persist indefinitely if the WebSocket connection fails. Add a timeout for the loading state: + const LOADING_TIMEOUT = 10000; // 10 seconds
+ useEffect(() => {
+ if (events === undefined) {
+ const timer = setTimeout(() => {
+ setStatus('Error');
+ setEvents(null);
+ }, LOADING_TIMEOUT);
+ return () => clearTimeout(timer);
+ }
+ }, [events]);
+
return (
<>
<div className='container mx-auto p-6'>
{events === undefined && <h1 className='text-xl font-bold'>Loading...</h1>}
{events === null && <h1 className='text-xl font-bold text-red-600'>Error</h1>}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
| {events && ( | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div className='space-y-6'> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <h1 className='mb-6 text-2xl font-bold'>Events</h1> | ||||||||||||||||||||||||||||||||||||||||||||||||
| {/* Apply animation to the container of event groups */} | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div ref={listRef} className='space-y-6'> | ||||||||||||||||||||||||||||||||||||||||||||||||
| {Object.entries(events).map(([key, value]) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div key={key} className='space-y-4'> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <h2 className='text-xl font-semibold'>{key}</h2> | ||||||||||||||||||||||||||||||||||||||||||||||||
| {/* Apply animation to the container of individual events */} | ||||||||||||||||||||||||||||||||||||||||||||||||
| <ul ref={itemsRef} className='flex flex-wrap space-y-4 space-x-4'> | ||||||||||||||||||||||||||||||||||||||||||||||||
| {value.map((event) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||
| <li key={event} className='list-none'> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div className='rounded-lg border-2 border-black p-4 dark:border-white'> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <p>{event}</p> | ||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||
| </li> | ||||||||||||||||||||||||||||||||||||||||||||||||
| ))} | ||||||||||||||||||||||||||||||||||||||||||||||||
| </ul> | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+86
to
+94
There was a problem hiding this comment. 🧹 Nitpick (assertive) Enhance accessibility. The list elements could benefit from improved accessibility attributes. Add ARIA attributes and semantic markup: -<ul ref={itemsRef} className='flex flex-wrap space-y-4 space-x-4'>
+<ul ref={itemsRef}
+ className='flex flex-wrap space-y-4 space-x-4'
+ role="list"
+ aria-label={`Events for ${key}`}>
{value.map((event) => (
- <li key={event} className='list-none'>
+ <li key={event}
+ className='list-none'
+ role="listitem">
<div className='rounded-lg border-2 border-black p-4 dark:border-white'>
- <p>{event}</p>
+ <p role="text">{event}</p>
</div>
</li>
))}
</ul>
|
||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||
| ))} | ||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||
| </> | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Strengthen API key security.
The example API key is too simple and predictable. While this is just an example file, it might lead developers to use similarly weak keys in production.
Replace with a more secure example and add guidelines:
📝 Committable suggestion