Merged
Conversation
- getMeSkipRedirect 대신 getMe API 로 수정 - page.tsx 에서 useEffect(redirect) 구문 삭제
Contributor
Walkthrough사용자 서비스에서 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
🎨 Storybook Report✅ 변경 사항이 없습니다 모든 Story가 이전 빌드와 동일합니다.
|
Member
Author
|
@coderabbitai review |
🎭 Playwright Report✨ E2E Test가 성공적으로 완료되었습니다. Test 요약 내용을 확인해주세요.
📊 Test Summary
📜 Test Details✅ Passed Tests (3)
|
🚀 PR Preview Report✨ Build가 성공적으로 완료되었습니다. Preview에서 변경사항을 확인하세요.
|
📊 Coverage Report
📉 #390을 main에 병합하면 coverage가 Coverage 요약@@ Coverage Diff @@
## main #390 +/- ##
===========================================
- Coverage 35.47% 35.40% -0.07%
===========================================
Files 262 262 0
Lines 12143 12119 -24
Branches 468 467 -1
===========================================
- Hits 4308 4291 -17
- Misses 7835 7828 -7 영향받은 파일
|
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/message/chat/[roomId]/page.tsx (1)
12-19:⚠️ Potential issue | 🟡 Minor
me?.userId의 불필요한 옵셔널 체이닝이userId: undefined를 하위에 전달할 수 있음
getMe()는 인증 실패 시 예외를 던지므로, 실행이 Line 13에 도달했을 때me는 항상 유효한User객체입니다. 따라서me?.userId는 불필요하며, 타입을number | undefined로 넓혀ChatRoomPage의userIdprop이number를 기대하는 경우 TypeScript 타입 불일치를 유발합니다. 최악의 경우userId={undefined}로 렌더링되어 채팅 룸 로직이 깨질 수 있습니다.🛡️ 수정 제안
- const me = await API.userService.getMe(); - const userId = me?.userId; + const me = await API.userService.getMe(); + const userId = me.userId;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/message/chat/`[roomId]/page.tsx around lines 12 - 19, Remove the unnecessary optional chaining on me when passing userId to ChatRoomPage: since API.userService.getMe() throws on auth failure, me is guaranteed to be a User at the point after await API.userService.getMe(), so replace me?.userId with me.userId and ensure the prop type matches (ChatRoomPage userId expects number); update any surrounding null/undefined handling if needed so userId is a number when calling ChatRoomPage (refer to getMe(), me, userId, and ChatRoomPage in this file).src/hooks/use-user/use-user-get/index.ts (1)
7-18:⚠️ Potential issue | 🟠 Major
useSuspenseQuery는enabled옵션을 지원하지 않으므로options파라미터 시그니처가 부정확함
useSuspenseQuery는 TanStack Query v5에서enabled옵션을 명시적으로 제외합니다. 현재 훅이options?: { enabled?: boolean }을 수용하고...options로 스프레드하면, 호출 측에서enabled: false를 전달했을 때 런타임에서 묵묵히 무시되어 쿼리가 항상 실행됩니다.현재 코드에서는
useGetUser({ userId })로 호출되며 누구도enabled옵션을 전달하지 않지만, 훅 시그니처가enabled를 허용하도록 되어 있어 향후 호출 측에서 오용할 가능성이 있습니다. TypeScript도 변수 스프레드(...options)에 대해서는 여분의 프로퍼티를 타입 에러로 잡지 않으므로 컴파일 단계에서 감지되지 않습니다.♻️ 수정 예시 (options 파라미터 제거)
-export const useGetUser = ({ userId }: GetUserPathParams, options?: { enabled?: boolean }) => { +export const useGetUser = ({ userId }: GetUserPathParams) => { const query = useSuspenseQuery({ queryKey: userKeys.item(userId), queryFn: () => API.userService.getUser({ userId }), - ...options, select: (data) => ({ ...data, profileImage: data.profileImage ?? '', profileMessage: data.profileMessage ?? '', mbti: data.mbti ?? '', }), }); return query; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/use-user/use-user-get/index.ts` around lines 7 - 18, The hook useGetUser declares an options parameter that includes enabled, but useSuspenseQuery does not support enabled; remove the options parameter (and the spread ...options) from useGetUser to avoid suggesting callers can pass enabled, or alternatively replace it with a properly typed options object that excludes enabled; update the useGetUser implementation (the call to useSuspenseQuery and its signature) so it no longer accepts or spreads an options that may contain enabled, keeping the select mapping and other logic intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/message/page.tsx`:
- Around line 26-27: Replace optional chaining on the current user ID so the
prop types align: where you call API.userService.getMe() (function getMe) and
currently use me?.userId, change it to me.userId; update the usage passed into
ChatRoomPage (prop userId: number) so it receives a definite number, not number
| undefined, ensuring the variable from getMe() (me.userId) is used consistently
across chat/[roomId]/page.tsx and src/app/message/page.tsx.
---
Outside diff comments:
In `@src/app/message/chat/`[roomId]/page.tsx:
- Around line 12-19: Remove the unnecessary optional chaining on me when passing
userId to ChatRoomPage: since API.userService.getMe() throws on auth failure, me
is guaranteed to be a User at the point after await API.userService.getMe(), so
replace me?.userId with me.userId and ensure the prop type matches (ChatRoomPage
userId expects number); update any surrounding null/undefined handling if needed
so userId is a number when calling ChatRoomPage (refer to getMe(), me, userId,
and ChatRoomPage in this file).
In `@src/hooks/use-user/use-user-get/index.ts`:
- Around line 7-18: The hook useGetUser declares an options parameter that
includes enabled, but useSuspenseQuery does not support enabled; remove the
options parameter (and the spread ...options) from useGetUser to avoid
suggesting callers can pass enabled, or alternatively replace it with a properly
typed options object that excludes enabled; update the useGetUser implementation
(the call to useSuspenseQuery and its signature) so it no longer accepts or
spreads an options that may contain enabled, keeping the select mapping and
other logic intact.
Chiman2937
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 변경 사항
프로필 페이지 리팩토링 변경사항
커밋 히스토리
692f496be1ffe88746c2e1. getMeSkipRedirect 삭제
user-service의getMeSkipRedirect메서드 및 관련 훅(useUserGetMeSkipRedirect)을 완전히 삭제.삭제 배경
기존에
getMeSkipRedirect가 필요했던 이유는, 서버 컴포넌트에서 발생한 API 요청에 대해 refresh가 정상 동작하지 않아 별도의 API 함수를 만들어 인증 실패 시 리다이렉트를 건너뛰도록 처리했기 때문. 이전 리팩토링에서 refresh 로직이proxy.ts(미들웨어)로 이관되면서 서버 요청에서도 refresh가 정상 동작하게 되었으므로,getMeSkipRedirect의 존재 이유가 사라짐.삭제된 코드
페이지별 대응
message/chat/[roomId])getMeSkipRedirect()getMe()message/page)getMeSkipRedirect()getMe()profile/[userId]/layout)getMeSkipRedirect()getMe().catch(() => null)유지2. 프로필 페이지 로직 간소화
src/app/(user)/profile/[userId]/page.tsx에서 본인 프로필 접근 시/mypage로 리다이렉트하던 로직을 삭제.Before
After
삭제된 항목
useRouterimport 및 사용useEffect내 본인 프로필 →/mypage리다이렉트 로직useUserGetMeSkipRedirect훅 사용if (!user) return null가드 (useSuspenseQuery 사용으로 불필요)3. useQuery → useSuspenseQuery 변경
useGetUser,useUserGetMe훅에서useQuery를useSuspenseQuery로 변경.영향받은 훅
useUserGetMesrc/hooks/use-user/use-user-get-me/index.tsuseGetUsersrc/hooks/use-user/use-user-get/index.ts변경된 파일 목록
src/api/service/user-service/index.tssrc/app/(user)/profile/[userId]/layout.tsxsrc/app/(user)/profile/[userId]/page.tsxsrc/app/message/chat/[roomId]/page.tsxsrc/app/message/page.tsxsrc/hooks/use-user/use-user-get-me/index.tssrc/hooks/use-user/use-user-get/index.ts🔗 관련 이슈
Closes #
🧪 테스트 방법
📸 스크린샷 (선택)
📋 체크리스트
💬 추가 코멘트
CodeRabbit Review는 자동으로 실행되지 않습니다.
Review를 실행하려면 comment에 아래와 같이 작성해주세요
Summary by CodeRabbit
릴리스 노트
버그 수정
개선 사항