feat: subscribe to notification SSE#16
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 51 minutes and 50 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 Walkthrough개요SSE(Server-Sent Events) 기반 실시간 알림 구독 기능을 추가합니다. 클라이언트 훅이 EventSource를 통해 알림 스트림에 연결되고, notification 이벤트를 수신하면 React Query 캐시를 무효화하여 즉시 갱신됩니다. 제공자 컴포넌트가 인증 상태에 따라 구독을 조건부로 활성화하며, 루트 레이아웃에 통합됩니다. 변경사항SSE 알림 구독
시퀀스 다이어그램sequenceDiagram
participant RootLayout as 루트 레이아웃
participant NotificationSSEProvider
participant useNotificationSSE as useNotificationSSE 훅
participant EventSource
participant ReactQuery as React Query
RootLayout->>NotificationSSEProvider: 제공자 마운트
NotificationSSEProvider->>NotificationSSEProvider: useAuthStore에서 인증 상태 구독
alt 인증됨 또는 userId 존재
NotificationSSEProvider->>useNotificationSSE: enabled = true로 호출
useNotificationSSE->>EventSource: /api/notifications/stream 연결
EventSource-->>useNotificationSSE: 연결 확립
else 미인증
NotificationSSEProvider->>useNotificationSSE: enabled = false로 호출
useNotificationSSE->>useNotificationSSE: 즉시 정리하고 반환
end
loop 연결 활성 상태
EventSource-->>useNotificationSSE: notification 이벤트
useNotificationSSE->>ReactQuery: notificationKeys.all 무효화
ReactQuery-->>useNotificationSSE: 캐시 갱신
end
예상 코드 리뷰 노력🎯 2 (단순) | ⏱️ ~12분 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/features/notification/model/use-notification-sse.ts (1)
33-33: ⚡ Quick winuseEffect 의존성 배열에서 queryClient를 제거하는 것을 고려하세요.
queryClient가 의존성 배열에 포함되어 있어 참조가 변경될 경우 EventSource가 불필요하게 재연결됩니다. React Query의 QueryClient는 일반적으로 안정적인 참조를 제공하지만, 명시적으로 보장되지는 않습니다.♻️ 제안하는 리팩토링
export function useNotificationSSE({ enabled = true, }: UseNotificationSSEOptions = {}) { const queryClient = useQueryClient(); + const queryClientRef = useRef(queryClient); + + useEffect(() => { + queryClientRef.current = queryClient; + }); useEffect(() => { if (!enabled || typeof EventSource === 'undefined') return; const es = new EventSource( backendProxyEndpoint(endpoints.notifications.subscribe), ); es.addEventListener('notification', () => { - queryClient.invalidateQueries({ queryKey: notificationKeys.all }); + queryClientRef.current.invalidateQueries({ queryKey: notificationKeys.all }); }); es.addEventListener('ping', () => { // keep-alive 이벤트는 연결 유지용이므로 별도 처리하지 않는다. }); return () => es.close(); - }, [enabled, queryClient]); + }, [enabled]); }필요한 import 추가:
-import { useEffect } from 'react'; +import { useEffect, useRef } from 'react';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/notification/model/use-notification-sse.ts` at line 33, useNotificationSSE의 useEffect 의존성 배열에 있는 queryClient 때문에 참조 변경 시 EventSource가 불필요하게 재연결됩니다. 이 훅에서 실제로 재실행이 필요한 값만 남기고 queryClient를 의존성에서 제거하도록 정리하세요. useNotificationSSE 내부의 useEffect와 EventSource 생성/정리 로직을 기준으로 수정하면 됩니다.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/notification/model/use-notification-sse.ts`:
- Around line 17-33: The current useEffect hook in use-notification-sse.ts lacks
error handling and reconnection logic for the EventSource instance. To fix this,
add an 'error' event listener to the EventSource object to handle connection
errors gracefully. Implement a reconnection strategy, such as retrying the
connection with exponential backoff, when an error occurs or the connection
closes unexpectedly. Ensure this logic is contained within the useEffect hook
managing the EventSource lifecycle to maintain a stable subscription.
---
Nitpick comments:
In `@src/features/notification/model/use-notification-sse.ts`:
- Line 33: useNotificationSSE의 useEffect 의존성 배열에 있는 queryClient 때문에 참조 변경 시
EventSource가 불필요하게 재연결됩니다. 이 훅에서 실제로 재실행이 필요한 값만 남기고 queryClient를 의존성에서 제거하도록
정리하세요. useNotificationSSE 내부의 useEffect와 EventSource 생성/정리 로직을 기준으로 수정하면 됩니다.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 48c7a237-86d4-46cc-aa56-17fe4861174b
📒 Files selected for processing (4)
src/app/layout.tsxsrc/app/providers/notification-sse-provider.tsxsrc/features/notification/index.tssrc/features/notification/model/use-notification-sse.ts
seoJing
force-pushed
the
feat/notification-sse
branch
from
99374af to
010b7cb
Compare
1 participant
Summary
Test Plan
Notes
Summary by CodeRabbit
릴리스 노트