-
Notifications
You must be signed in to change notification settings - Fork 2
모달 내 소켓 통신을 위한 useSocketStore 생성 및 적용 #238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: GRPHI-227-refactor/game-component-branching
Are you sure you want to change the base?
모달 내 소켓 통신을 위한 useSocketStore 생성 및 적용 #238
Conversation
Summary of ChangesHello @kim-hyunjoo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 모달 내 소켓 통신 방식의 근본적인 문제를 해결하고, 전반적인 컴포넌트 구조를 개선하여 코드의 유지보수성과 확장성을 향상시키는 것을 목표로 합니다. 기존에는 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
이번 PR은 useSocketStore
를 도입하여 모달 내에서 직접 소켓 통신을 하도록 구조를 개선한 점이 인상적입니다. 이를 통해 불필요한 useEffect
를 제거하고 의도하지 않은 소켓 통신 문제를 해결한 것은 매우 좋은 접근입니다. 또한, 컴포넌트 구조를 Common
디렉토리 중심으로 대대적으로 리팩터링하여 코드의 재사용성과 유지보수성을 높인 점도 훌륭합니다.
전반적으로 코드 품질을 크게 향상시키는 변경이라고 생각합니다. 다만, 리팩터링 과정에서 한 가지 심각한 문제가 발견되었습니다. GameActionButtonsController
컴포넌트에서 React 훅을 조건부로 호출하고 있어 런타임 에러를 유발할 수 있습니다. 이 부분에 대한 수정 제안을 포함하여 몇 가지 개선 사항을 리뷰 코멘트로 남겼으니 확인 부탁드립니다.
const GameActionButtonsController = ({ | ||
game, | ||
isRoomManager, | ||
sendMessage, | ||
}: GameActionButtonsControllerProps) => { | ||
const gameType = gameToType(game); | ||
const { roomStatus } = useRoomStore(); | ||
|
||
const gameControl = gameType ? GAME_CONTROL_MAP[gameType] : null; | ||
|
||
const { round } = gameControl?.useStore() || { round: null }; | ||
|
||
if (!gameControl || !round || !isRoomManager) return null; | ||
|
||
const roundEndActionText = | ||
round.currentRound === round.totalRounds | ||
? '최종 결과 보기' | ||
: '다음 라운드로 이동'; | ||
const nextDestination = gameControl.nextDestination; | ||
const endDestination = gameControl.endDestination; | ||
|
||
const handleEnterNextRound = () => { | ||
sendMessage({ destination: nextDestination }); | ||
}; | ||
|
||
const handleMoveToWaitingRoom = () => { | ||
sendMessage({ destination: endDestination }); | ||
}; | ||
|
||
return ( | ||
<GameActionButtonsView | ||
roomStatus={roomStatus} | ||
buttonText={roundEndActionText} | ||
handleEnterNextRound={handleEnterNextRound} | ||
handleMoveToWaitingRoom={handleMoveToWaitingRoom} | ||
/> | ||
); | ||
}; |
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.
현재 구현에서는 gameControl?.useStore()
를 통해 조건부로 훅을 호출하고 있습니다. 이는 React의 훅 규칙(Rules of Hooks)에 위배되며, 렌더링 시점에 따라 훅 호출 순서가 달라져 런타임 에러를 발생시킬 수 있습니다. 훅은 항상 컴포넌트의 최상위 레벨에서 호출되어야 합니다. 이 문제를 해결하기 위해 훅 호출 로직을 별도의 내부 컴포넌트로 분리하고, 부모 컴포넌트에서는 조건부로 이 내부 컴포넌트를 렌더링하는 방식으로 리팩터링하는 것을 제안합니다.
const GameActionButtonsContent = ({
gameControl,
isRoomManager,
sendMessage,
}: {
gameControl: (typeof GAME_CONTROL_MAP)[keyof typeof GAME_CONTROL_MAP];
isRoomManager: boolean;
sendMessage: GameActionButtonsControllerProps['sendMessage'];
}) => {
const { roomStatus } = useRoomStore();
const { round } = gameControl.useStore();
if (!round || !isRoomManager) return null;
const roundEndActionText =
round.currentRound === round.totalRounds
? '최종 결과 보기'
: '다음 라운드로 이동';
const nextDestination = gameControl.nextDestination;
const endDestination = gameControl.endDestination;
const handleEnterNextRound = () => {
sendMessage({ destination: nextDestination });
};
const handleMoveToWaitingRoom = () => {
sendMessage({ destination: endDestination });
};
return (
<GameActionButtonsView
roomStatus={roomStatus}
buttonText={roundEndActionText}
handleEnterNextRound={handleEnterNextRound}
handleMoveToWaitingRoom={handleMoveToWaitingRoom}
/>
);
};
const GameActionButtonsController = ({
game,
isRoomManager,
sendMessage,
}: GameActionButtonsControllerProps) => {
const gameType = gameToType(game);
const gameControl = gameType ? GAME_CONTROL_MAP[gameType] : null;
if (!gameControl) {
return null;
}
return (
<GameActionButtonsContent
gameControl={gameControl}
isRoomManager={isRoomManager}
sendMessage={sendMessage}
/>
);
};
useEffect(() => { | ||
if (sendMessage) { | ||
setSendMessage(sendMessage); | ||
} | ||
}, [sendMessage, setSendMessage]); |
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.
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.
완료
useEffect(() => { | ||
if (roomDetail.players.length > 0) { | ||
const host = roomDetail.players.find((player) => player.isHost); | ||
if (host) { | ||
setHostName(host.name); | ||
} | ||
} | ||
}, [roomDetail.players, setHostName]); |
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.
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.
완료
📝작업 내용
기존 구조의 문제점
optionNumberprops
의 타입은string | number
이기 때문에, 모달 내부에서 직접 sendMessage를 사용할 수 없었습니다.GameRoom
에서 useEffect의 의존성 배열에myName
,gamdId
를 넣어 간접적으로 변경을 감지하여 sendMessage를 통해 소켓 통신을 하도록 설계했습니다.현재 발견된 에러 상황
게임 변경하기
를 통해 카드 게임으로 변경sendMessage
가 유효함GamePanel
에서 에러 발생GameRoom
컴포넌트는 상위 컴포넌트이므로 렌더링에 문제x, 정상적으로 렌더링됨GameRoom
렌더링 중 useEffect문의sendMessage
가 발동하여게임 변경
,닉네임 변경
로직 각각 2번씩 수행(react의 strictMode때문)개선된 구조
useSocketStore
생성, 소켓 연결 시sendMessage
함수 저장GameListCarousel
), 닉네임 변경 모달(CreateUserNameModal
)에서 직접 store에 저장된sendMessage
를 통해 핸들러 함수에서 소켓 통신하도록 변경useEffect
문 삭제✨PR Point