Skip to content

feat: event bus packages init#14

Open
citron03 wants to merge 1 commit intomainfrom
feat/event-bus
Open

feat: event bus packages init#14
citron03 wants to merge 1 commit intomainfrom
feat/event-bus

Conversation

@citron03
Copy link
Owner

No description provided.

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

콜백 안정성

useOn 훅의 callback 함수가 자주 변경될 경우, 불필요한 이벤트 구독 및 해제가 반복될 수 있습니다. useCallback을 사용하여 콜백 함수를 메모이제이션하거나, useRef를 활용하여 콜백 함수의 안정성을 높이는 방법을 고려해볼 수 있습니다.

export function useOn<Events extends Record<string, any>, K extends keyof Events>(
  bus: EventBus<Events>,
  key: K,
  callback: (data: Events[K]) => void,
) {
  useEffect(() => {
    // 구독 및 cleanup 자동 처리
    const unsubscribe = bus.on(key, callback);
    return () => unsubscribe();
  }, [bus, key, callback]);
}

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
이벤트 핸들러 제거 후 빈 셋 정리

EventBusoff 메서드에서 특정 이벤트 키에 대한 모든 핸들러가 제거된 후에도 해당 키가 listeners 맵에 남아있을 수 있습니다. 이는
Set 객체를 남겨두어 불필요한 메모리 사용을 초래할 수 있습니다. 모든 핸들러가 제거되면 listeners 맵에서 해당 키를 제거하여 리소스를
효율적으로 관리하는 것이 좋습니다.

apps/event-bus/src/event-bus.ts [29-34]

 off<Key extends keyof Events>(key: Key, handler: Handler<Events[Key]>) {
   const handlers = this.listeners.get(key);
   if (handlers) {
     handlers.delete(handler);
+    if (handlers.size === 0) { // Set이 비어있는지 확인
+      this.listeners.delete(key); // Map에서 키 제거
+    }
   }
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that an empty Set might remain in the listeners map after all handlers for a key are removed. Deleting the key from the map when its Set becomes empty improves memory management and resource efficiency.

Medium

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant