Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough기존 [id] 동적 라우트를 [placeId]로 변경하면서, JWT 토큰에서 memberId를 추출하는 유틸리티를 추가하고, 장소 상세 정보 API 및 React Query 훅을 새로 도입합니다. Changes
Sequence DiagramsequenceDiagram
participant Page as Node Page<br/>[placeId]
participant Token as Token Util
participant Router as Next Router
participant Query as React Query
participant API as API Client
participant Backend as Backend API
Page->>Router: 라우터에서 placeId 추출
Page->>Token: getMemberIdFromToken() 호출
Token->>Token: localStorage 액세스 토큰 읽기
Token->>Token: jwtDecode로 디코딩
Token-->>Page: memberId 반환
Page->>Query: useGetPlaceDetail(placeId, memberId)
Query->>API: queryFn 실행
API->>Backend: GET /api/places/{placeId}?memberId=...
Backend-->>API: 장소 상세 정보
API-->>Query: ApiResponse 반환
Query-->>Page: 데이터 상태 업데이트
Page->>Page: LocationCard, AddressCopy 렌더링
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
|
🏷️ Labeler has automatically applied labels based on your PR title, branch name, or commit message. |
|
🏷️ Labeler has automatically applied labels based on your PR title, branch name, or commit message. |
skyblue1232
left a comment
There was a problem hiding this comment.
말씀하신 것 참고해서 구현하도록 하겠습니다!!
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/shared/utils/token.ts (1)
25-39: JWT 디코딩 로직 개선 제안함수 로직은 전반적으로 올바르지만, 다음 사항들을 고려해주세요:
- 타입 안정성: Line 32-33의 타입 정의가 느슨합니다. JWT 페이로드 구조에 대한 명확한 인터페이스 정의를 고려해주세요.
- 폴백 체인 문서화:
memberId ?? sub ?? id폴백 순서에 대한 주석이나 문서화가 있으면 유지보수에 도움이 됩니다.- 에러 로깅: Line 36의 에러 로그가 운영 환경에서도 노출됩니다. 개발 환경에서만 로그를 출력하도록 조건부 처리를 고려해주세요.
다음과 같이 개선할 수 있습니다:
+interface JwtPayload { + memberId?: number; + sub?: number; + id?: number; +} + export const getMemberIdFromToken = (): number | null => { if (!isBrowser) return null; const token = localStorage.getItem(ACCESS_TOKEN_KEY); if (!token) return null; try { - const decoded: { memberId?: number; sub?: number; id?: number } = - jwtDecode(token); + const decoded: JwtPayload = jwtDecode(token); + // 우선순위: memberId > sub > id return decoded.memberId ?? decoded.sub ?? decoded.id ?? null; } catch (err) { - console.error('JWT decode 실패:', err); + if (process.env.NODE_ENV === 'development') { + console.error('JWT decode 실패:', err); + } return null; } };src/pages/main/node/[placeId].tsx (1)
40-41: 디버깅 로그 제거 필요개발용 console.log가 남아있습니다. 프로덕션 배포 전에 제거하거나 개발 환경에서만 실행되도록 조건부 처리가 필요합니다.
다음과 같이 개선할 수 있습니다:
- console.log('📍 장소 ID:', placeId); - console.log('👤 사용자 ID:', memberId); + if (process.env.NODE_ENV === 'development') { + console.log('📍 장소 ID:', placeId); + console.log('👤 사용자 ID:', memberId); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
package.json(1 hunks)src/pages/main/node/[id].tsx(0 hunks)src/pages/main/node/[placeId].tsx(1 hunks)src/shared/main/api/getPlaceDetail.ts(1 hunks)src/shared/main/queries/useGetPlaceDetail.ts(1 hunks)src/shared/utils/token.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/pages/main/node/[id].tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-31T17:34:04.352Z
Learnt from: jjangminii
Repo: geulDa/FE PR: 64
File: src/pages/map/result/components/ResultMap.tsx:3-3
Timestamp: 2025-10-31T17:34:04.352Z
Learning: The geulDa/FE project uses Next.js Pages Router (not App Router), so components should import useRouter from 'next/router' and should not use the 'use client' directive.
Applied to files:
src/pages/main/node/[placeId].tsx
🧬 Code graph analysis (3)
src/shared/main/queries/useGetPlaceDetail.ts (1)
src/shared/main/api/getPlaceDetail.ts (1)
getPlaceDetail(14-22)
src/pages/main/node/[placeId].tsx (5)
src/shared/hooks/useUserStatus.ts (1)
useUserStatus(7-66)src/shared/utils/token.ts (1)
getMemberIdFromToken(25-39)src/shared/utils/handleGetLocation.ts (1)
getLocation(1-15)src/shared/lib/utils.ts (1)
cn(71-73)src/shared/components/set/PopupSet.tsx (1)
PopupSet(11-35)
src/shared/main/api/getPlaceDetail.ts (2)
src/shared/api/instance.ts (1)
apiAuth(20-23)src/shared/types/authtypes.ts (1)
ApiResponse(1-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-deploy
🔇 Additional comments (8)
src/shared/utils/token.ts (1)
1-1: LGTM!
jwtDecode를 명명된 import로 올바르게 가져오고 있습니다.src/shared/main/api/getPlaceDetail.ts (2)
4-11: LGTM!
PlaceDetailResponse인터페이스가 장소 상세 정보의 모든 필요한 필드를 명확하게 정의하고 있습니다.
14-22: LGTM!API 호출 구조가 올바르며,
memberId를 쿼리 파라미터로 전달하는 방식이 적절합니다. 타입 안정성도 확보되어 있습니다.src/pages/main/node/[placeId].tsx (4)
24-29: 임시 데이터 활성화 계획 확인현재 하드코딩된 임시 데이터를 사용하고 있습니다. Lines 35-38에 주석 처리된
useGetPlaceDetail훅을 언제 활성화할 계획인지 확인이 필요합니다.실제 API 연동을 활성화하기 전에, 다음 사항들을 확인해주세요:
- 백엔드 API 엔드포인트가 준비되었는지
- API 응답 구조가
PlaceDetailResponse인터페이스와 일치하는지- 에러 처리 및 로딩 상태 UI가 준비되었는지 (현재 Lines 46-48에 주석 처리됨)
74-101: LGTM!이미지 섹션과 스탬프 버튼의 조건부 렌더링 로직이 잘 구현되어 있습니다.
isCompleted상태에 따라 적절한 아이콘, 크기, 스타일이 적용되고 있으며, 접근성(aria-label)도 고려되어 있습니다.
103-111: LGTM!
LocationCard와AddressCopy컴포넌트를 적절하게 사용하여 장소 정보를 표시하고 있습니다.
114-122: LGTM!로그인 팝업 처리 로직이 올바르게 구현되어 있으며, 팝업 닫기 시
/auth로 리다이렉트하는 흐름이 적절합니다.package.json (1)
18-18: 검증 완료: jwt-decode 버전 안전확인 결과,
jwt-decode ^4.0.0은 최신 버전이며 GitHub Advisory Database에서 보안 취약점이 발견되지 않았습니다. 해당 의존성은 안전하게 사용 가능합니다.
| @@ -0,0 +1,127 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
'use client' 지시어 제거 필요
이 프로젝트는 Next.js Pages Router를 사용하므로 'use client' 지시어가 필요하지 않습니다. Pages Router에서는 이 지시어를 사용하지 않으며, 페이지 컴포넌트는 기본적으로 서버/클라이언트 하이브리드로 동작합니다.
Based on learnings
다음과 같이 제거해주세요:
-'use client';
import { useState } from 'react';🤖 Prompt for AI Agents
src/pages/main/node/[placeId].tsx around lines 1 to 1: Remove the `'use client'`
directive at the top of the file; this page uses the Next.js Pages Router so the
client directive is unnecessary—delete the first line containing `'use client';`
so the component remains as a Pages Router page and can operate as server/client
hybrid.
| const handleStampClick = () => { | ||
| if (!isLoggedIn) { | ||
| setShowLoginPopup(true); | ||
| return; | ||
| } | ||
|
|
||
| getLocation( | ||
| (pos) => console.log('📍 현재 위치:', pos.coords), | ||
| (err) => console.error('⚠️ 위치 에러:', err.message), | ||
| ); | ||
| router.push('/main/HiddenReward'); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
handleStampClick 로직 개선 제안
현재 구현에는 다음과 같은 개선 사항을 고려해주세요:
- 위치 권한 에러 처리: Line 60의 에러 핸들러가 console.error만 호출합니다. 사용자에게 명확한 피드백(예: 팝업 또는 토스트)을 제공해야 합니다.
- 위치 확인 후 페이지 이동: Line 62에서 위치 확인 성공 여부와 관계없이 페이지를 이동합니다. 위치 확인이 성공한 경우에만 이동하도록 수정이 필요합니다.
- 디버깅 로그: Line 59의 console.log도 프로덕션에서 제거가 필요합니다.
다음과 같이 개선해주세요:
const handleStampClick = () => {
if (!isLoggedIn) {
setShowLoginPopup(true);
return;
}
getLocation(
(pos) => {
- console.log('📍 현재 위치:', pos.coords);
+ if (process.env.NODE_ENV === 'development') {
+ console.log('📍 현재 위치:', pos.coords);
+ }
+ // 위치 확인 성공 시에만 페이지 이동
+ router.push('/main/HiddenReward');
},
- (err) => console.error('⚠️ 위치 에러:', err.message),
+ (err) => {
+ console.error('⚠️ 위치 에러:', err.message);
+ // TODO: 사용자에게 위치 권한 필요 안내 팝업 표시
+ alert('위치 정보 접근 권한이 필요합니다.');
+ },
);
- router.push('/main/HiddenReward');
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleStampClick = () => { | |
| if (!isLoggedIn) { | |
| setShowLoginPopup(true); | |
| return; | |
| } | |
| getLocation( | |
| (pos) => console.log('📍 현재 위치:', pos.coords), | |
| (err) => console.error('⚠️ 위치 에러:', err.message), | |
| ); | |
| router.push('/main/HiddenReward'); | |
| }; | |
| const handleStampClick = () => { | |
| if (!isLoggedIn) { | |
| setShowLoginPopup(true); | |
| return; | |
| } | |
| getLocation( | |
| (pos) => { | |
| if (process.env.NODE_ENV === 'development') { | |
| console.log('📍 현재 위치:', pos.coords); | |
| } | |
| // 위치 확인 성공 시에만 페이지 이동 | |
| router.push('/main/HiddenReward'); | |
| }, | |
| (err) => { | |
| console.error('⚠️ 위치 에러:', err.message); | |
| // TODO: 사용자에게 위치 권한 필요 안내 팝업 표시 | |
| alert('위치 정보 접근 권한이 필요합니다.'); | |
| }, | |
| ); | |
| }; |
| export const useGetPlaceDetail = (placeId?: number, memberId?: number) => { | ||
| return useQuery({ | ||
| queryKey: ['placeDetail', placeId, memberId], | ||
| queryFn: () => getPlaceDetail(placeId!, memberId!), | ||
| enabled: !!placeId && !!memberId, // 둘 다 있어야 실행 | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
non-null assertion 안전성 개선 필요
Line 8의 placeId!, memberId! non-null assertion은 Line 9의 enabled 가드에 의존하고 있지만, 향후 enabled 로직이 변경되면 런타임 에러가 발생할 수 있습니다.
더 안전한 패턴으로 개선해주세요:
export const useGetPlaceDetail = (placeId?: number, memberId?: number) => {
return useQuery({
queryKey: ['placeDetail', placeId, memberId],
- queryFn: () => getPlaceDetail(placeId!, memberId!),
+ queryFn: () => {
+ if (!placeId || !memberId) {
+ throw new Error('placeId and memberId are required');
+ }
+ return getPlaceDetail(placeId, memberId);
+ },
enabled: !!placeId && !!memberId, // 둘 다 있어야 실행
});
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const useGetPlaceDetail = (placeId?: number, memberId?: number) => { | |
| return useQuery({ | |
| queryKey: ['placeDetail', placeId, memberId], | |
| queryFn: () => getPlaceDetail(placeId!, memberId!), | |
| enabled: !!placeId && !!memberId, // 둘 다 있어야 실행 | |
| }); | |
| export const useGetPlaceDetail = (placeId?: number, memberId?: number) => { | |
| return useQuery({ | |
| queryKey: ['placeDetail', placeId, memberId], | |
| queryFn: () => { | |
| if (!placeId || !memberId) { | |
| throw new Error('placeId and memberId are required'); | |
| } | |
| return getPlaceDetail(placeId, memberId); | |
| }, | |
| enabled: !!placeId && !!memberId, // 둘 다 있어야 실행 | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In src/shared/main/queries/useGetPlaceDetail.ts around lines 5 to 10, remove the
unsafe non-null assertions (placeId! and memberId!) and make queryFn validate
inputs at runtime: keep enabled: !!placeId && !!memberId but implement queryFn
as a small guard that checks if placeId or memberId are null/undefined and
either returns a rejected Promise with a clear error or a resolved empty value,
otherwise call getPlaceDetail(placeId, memberId); this eliminates the reliance
on enabled remaining unchanged and prevents potential runtime crashes if callers
change enabled logic later.
🔥 작업 내용
🤔 추후 작업 사항
🔗 이슈
PR Point (To Reviewer)
📸 피그마 스크린샷 or 기능 GIF
(작업 내역 스크린샷)