Refactor: map-image 로딩 처리 및 지도 에러 처리#133
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough네 파일의 변경 사항은 이미지 로딩 스켈레톤 UI 추가, 세션 없을 시 인증 페이지로의 클라이언트 사이드 리다이렉트 구현, 그리고 지도 컴포넌트의 에러 핸들링 개선으로 구성됩니다. Next.js 설정도 정렬되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Map Page
participant FullMap
participant useCourseSession
participant Auth Service
Map Page->>FullMap: render with sessionId & onError()
FullMap->>useCourseSession: fetch with sessionId
alt Success Path
useCourseSession-->>FullMap: data, isError=false
FullMap->>FullMap: render map content
end
alt Error Path
useCourseSession-->>FullMap: data, isError=true
FullMap->>FullMap: useEffect detects isError
FullMap->>Map Page: onError callback
Map Page->>Auth Service: router.push('/auth')
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
next.config.ts(1 hunks)src/pages/map/location/[placeId].tsx(1 hunks)src/pages/map/result/Map.tsx(3 hunks)src/shared/components/map/result/components/FullMap.tsx(3 hunks)
🧰 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/map/result/Map.tsxsrc/pages/map/location/[placeId].tsx
🧬 Code graph analysis (3)
src/shared/components/map/result/components/FullMap.tsx (2)
src/shared/api/course/queries/useCourseSession.ts (1)
useCourseSession(5-12)src/shared/api/course/types/courseSession.ts (1)
CoursePlace(1-9)
src/pages/map/result/Map.tsx (1)
src/shared/components/map/result/components/FullMap.tsx (1)
FullMap(12-69)
src/pages/map/location/[placeId].tsx (1)
src/shared/lib/utils.ts (1)
cn(71-73)
🪛 GitHub Actions: Next.js CI/CD with Vercel
next.config.ts
[error] 31-31: Type error: An object literal cannot have multiple properties with the same name.
🔇 Additional comments (3)
next.config.ts (1)
26-31: remotePatterns 중복 병합 확인 완료 및 승인.설정 파일이 성공적으로 로드되었으며, 단일
remotePatterns배열로 올바르게 통합되었습니다. 이전의 "object literal cannot have multiple properties with the same name" 빌드 오류가 해결되었고,places.googleapis.com항목이 다른 원본 패턴들과 함께 통합되었습니다. 변경사항이 의도한 목표를 달성했습니다.src/pages/map/location/[placeId].tsx (1)
27-33: 스켈레톤 UI 구현이 적절합니다.기본
animate-pulse를 사용한 구현이 깔끔합니다. PR에 명시된 대로 Shimmer 애니메이션 및 공통 컴포넌트 분리는 추후 개선 사항으로 적절합니다.src/shared/components/map/result/components/FullMap.tsx (1)
7-10: Props 인터페이스가 명확합니다.
FullMapProps인터페이스 정의가 깔끔하고 적절합니다.onError를 선택적 prop으로 설계한 것이 좋습니다.
| const src = | ||
| typeof imageSrc === 'string' ? imageSrc.trim() : defaultImage; |
There was a problem hiding this comment.
trim() 후 빈 문자열 처리가 누락되었습니다.
imageSrc.trim()이 빈 문자열을 반환할 경우 이를 처리하지 않아 이미지 로딩이 실패할 수 있습니다.
다음 diff를 적용하여 빈 문자열 케이스를 처리하세요:
- const src =
- typeof imageSrc === 'string' ? imageSrc.trim() : defaultImage;
+ const src =
+ typeof imageSrc === 'string' && imageSrc.trim()
+ ? imageSrc.trim()
+ : defaultImage;📝 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 src = | |
| typeof imageSrc === 'string' ? imageSrc.trim() : defaultImage; | |
| const src = | |
| typeof imageSrc === 'string' && imageSrc.trim() | |
| ? imageSrc.trim() | |
| : defaultImage; |
🤖 Prompt for AI Agents
In src/pages/map/location/[placeId].tsx around lines 19 to 20, the code uses
imageSrc.trim() but doesn't handle the case where trim() returns an empty
string; update the logic to treat a trimmed empty string as missing and fall
back to defaultImage (e.g. compute a single trimmed value once, check
trimmedValue !== '' and use it, otherwise use defaultImage).
| <Image | ||
| src={typeof imageSrc === 'string' ? imageSrc : '/assets/board.svg'} | ||
| src={src} | ||
| alt={String(name)} | ||
| width={354} | ||
| height={436} | ||
| className="w-full h-auto object-cover block rounded-[16px]" | ||
| className={cn( | ||
| 'w-full h-auto object-cover block rounded-[16px] transition-opacity duration-300', | ||
| imgLoaded ? 'opacity-100' : 'opacity-0' | ||
| )} | ||
| onLoadingComplete={() => setImgLoaded(true)} | ||
| /> |
There was a problem hiding this comment.
이미지 로딩 실패 처리가 누락되었습니다.
onError 핸들러가 없어서 이미지 로딩이 실패하면 onLoadingComplete가 호출되지 않고, 사용자는 무한정 스켈레톤 UI만 보게 됩니다.
다음 diff를 적용하여 에러 핸들링을 추가하세요:
<Image
src={src}
alt={String(name)}
width={354}
height={436}
className={cn(
'w-full h-auto object-cover block rounded-[16px] transition-opacity duration-300',
imgLoaded ? 'opacity-100' : 'opacity-0'
)}
onLoadingComplete={() => setImgLoaded(true)}
+ onError={() => setImgLoaded(true)}
/>또는 에러 상태를 별도로 관리하여 fallback 이미지로 교체하는 방법도 고려할 수 있습니다.
📝 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.
| <Image | |
| src={typeof imageSrc === 'string' ? imageSrc : '/assets/board.svg'} | |
| src={src} | |
| alt={String(name)} | |
| width={354} | |
| height={436} | |
| className="w-full h-auto object-cover block rounded-[16px]" | |
| className={cn( | |
| 'w-full h-auto object-cover block rounded-[16px] transition-opacity duration-300', | |
| imgLoaded ? 'opacity-100' : 'opacity-0' | |
| )} | |
| onLoadingComplete={() => setImgLoaded(true)} | |
| /> | |
| <Image | |
| src={src} | |
| alt={String(name)} | |
| width={354} | |
| height={436} | |
| className={cn( | |
| 'w-full h-auto object-cover block rounded-[16px] transition-opacity duration-300', | |
| imgLoaded ? 'opacity-100' : 'opacity-0' | |
| )} | |
| onLoadingComplete={() => setImgLoaded(true)} | |
| onError={() => setImgLoaded(true)} | |
| /> |
🤖 Prompt for AI Agents
In src/pages/map/location/[placeId].tsx around lines 35-45, the Image component
lacks error handling so if the image fails to load the skeleton stays visible;
add an onError handler (or separate error state) that stops the skeleton and
shows a fallback image: when the image errors, set the imgLoaded state (or an
explicit errored state) so the skeleton is hidden, and replace the src with a
local/default fallback URL (or toggle to render an <img> fallback) to ensure a
visible image is shown; ensure any state update types are correct and avoid
infinite loops when swapping to the fallback.
| useEffect(() => { | ||
| if (router.isReady && !sessionId) { | ||
| router.push('/auth'); | ||
| } | ||
| }, [router, sessionId]); |
There was a problem hiding this comment.
리다이렉트 로직이 중복되어 있습니다.
useEffect의 리다이렉트 로직과 36번 라인의 조건부 렌더링이 중복됩니다. useEffect가 실행되기 전에 41-44번 라인의 "코스 정보가 없습니다" 메시지가 짧게 표시될 수 있어 사용자 경험이 일관되지 않습니다.
다음 중 하나를 선택하세요:
옵션 1: useEffect만 사용 (즉시 리다이렉트)
return (
<main
className="w-full h-[100vh] bg-mint-300 overflow-hidden"
role="main"
aria-label="코스 추천 지도 페이지"
>
<Header
title="코스 추천"
onClick={() => {
if (window.history.length > 1) {
router.back();
} else {
router.push('/map');
}
}}
/>
- {sessionId ? (
- <FullMap
- sessionId={sessionId}
- onError={() => router.push('/auth')}
- />
- ) : (
- <div className="flex h-full items-center justify-center text-gray-600 text-title-sm bg-gray-200">
- 코스 정보가 없습니다
- </div>
- )}
+ {sessionId && (
+ <FullMap
+ sessionId={sessionId}
+ onError={() => router.push('/auth')}
+ />
+ )}
</main>
);옵션 2: 조건부 렌더링만 사용 (메시지 표시)
- useEffect(() => {
- if (router.isReady && !sessionId) {
- router.push('/auth');
- }
- }, [router, sessionId]);
-
return (📝 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.
| useEffect(() => { | |
| if (router.isReady && !sessionId) { | |
| router.push('/auth'); | |
| } | |
| }, [router, sessionId]); |
🤖 Prompt for AI Agents
In src/pages/map/result/Map.tsx around lines 13 to 17, the redirect logic is
duplicated between the useEffect and the conditional rendering later, causing a
flash of the "코스 정보가 없습니다" message before redirect; choose one approach and
remove the other: either keep the useEffect-only approach (immediately
router.push('/auth') when router.isReady && !sessionId and remove the
conditional rendering that shows the message) or keep the
conditional-rendering-only approach (remove the useEffect and return the "코스 정보가
없습니다" UI when !sessionId, avoiding an automatic redirect). Ensure dependencies
remain correct and any related cleanup (e.g., preventing state updates after
unmount) is handled.
| <FullMap | ||
| sessionId={sessionId} | ||
| onError={() => router.push('/auth')} | ||
| /> |
There was a problem hiding this comment.
onError 콜백이 매 렌더링마다 재생성됩니다.
인라인 함수로 전달된 onError는 매 렌더링마다 새로운 참조를 생성하여, FullMap 컴포넌트의 useEffect 의존성 배열에 포함된 onError가 계속 변경되어 불필요한 effect 실행을 유발할 수 있습니다.
useCallback을 사용하여 콜백을 안정화하세요:
+ import { useEffect, useCallback } from 'react';
const MapPage = () => {
const router = useRouter();
const sessionId = Array.isArray(router.query.sessionId)
? router.query.sessionId[0]
: router.query.sessionId;
+ const handleError = useCallback(() => {
+ router.push('/auth');
+ }, [router]);
useEffect(() => {
if (router.isReady && !sessionId) {
router.push('/auth');
}
}, [router, sessionId]);
return (
<main
className="w-full h-[100vh] bg-mint-300 overflow-hidden"
role="main"
aria-label="코스 추천 지도 페이지"
>
<Header
title="코스 추천"
onClick={() => {
if (window.history.length > 1) {
router.back();
} else {
router.push('/map');
}
}}
/>
{sessionId ? (
<FullMap
sessionId={sessionId}
- onError={() => router.push('/auth')}
+ onError={handleError}
/>
) : (
<div className="flex h-full items-center justify-center text-gray-600 text-title-sm bg-gray-200">
코스 정보가 없습니다
</div>
)}
</main>
);
};📝 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.
| <FullMap | |
| sessionId={sessionId} | |
| onError={() => router.push('/auth')} | |
| /> | |
| import { useEffect, useCallback } from 'react'; | |
| const MapPage = () => { | |
| const router = useRouter(); | |
| const sessionId = Array.isArray(router.query.sessionId) | |
| ? router.query.sessionId[0] | |
| : router.query.sessionId; | |
| const handleError = useCallback(() => { | |
| router.push('/auth'); | |
| }, [router]); | |
| useEffect(() => { | |
| if (router.isReady && !sessionId) { | |
| router.push('/auth'); | |
| } | |
| }, [router, sessionId]); | |
| return ( | |
| <main | |
| className="w-full h-[100vh] bg-mint-300 overflow-hidden" | |
| role="main" | |
| aria-label="코스 추천 지도 페이지" | |
| > | |
| <Header | |
| title="코스 추천" | |
| onClick={() => { | |
| if (window.history.length > 1) { | |
| router.back(); | |
| } else { | |
| router.push('/map'); | |
| } | |
| }} | |
| /> | |
| {sessionId ? ( | |
| <FullMap | |
| sessionId={sessionId} | |
| onError={handleError} | |
| /> | |
| ) : ( | |
| <div className="flex h-full items-center justify-center text-gray-600 text-title-sm bg-gray-200"> | |
| 코스 정보가 없습니다 | |
| </div> | |
| )} | |
| </main> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In src/pages/map/result/Map.tsx around lines 37 to 40, the inline onError={() =>
router.push('/auth')} is recreated on every render causing dependent effects in
FullMap to retrigger; wrap the callback with React.useCallback to stable-ize its
reference (e.g. const onError = useCallback(() => router.push('/auth'),
[router])) and pass that onError to FullMap, and add the necessary import for
useCallback at the top of the file.
| useEffect(() => { | ||
| if (isError) { | ||
| onError?.(); | ||
| } | ||
| }, [isError, onError]); |
There was a problem hiding this comment.
🧩 Analysis chain
에러 핸들링 로직에 개선이 필요합니다.
현재 구현에는 다음 문제가 있습니다:
- 중복 실행 위험:
onError참조가 변경되면서isError가true인 상태에서 effect가 재실행되어 리다이렉트가 여러 번 발생할 수 있습니다. - 에러 타입 미구분:
useCourseSession의isError는 모든 에러(401/404/500/네트워크 오류)를 동일하게 처리합니다. 인증 에러(401)와 서버 에러(500)를 구분하여 처리하는 것이 사용자 경험에 더 좋습니다. - 사용자 피드백 부재: 리다이렉트 전에 어떤 에러가 발생했는지 사용자에게 알리지 않습니다.
제안 1: Ref를 사용하여 중복 호출 방지
export default function FullMap({ sessionId, onError }: FullMapProps) {
const mapRef = useRef<HTMLDivElement | null>(null);
+ const hasCalledError = useRef(false);
const { data, isError } = useCourseSession(sessionId);
const [selectedPlace, setSelectedPlace] = useState<CoursePlace | null>(null);
const [bubblePos, setBubblePos] = useState<{ x: number; y: number } | null>(null);
useEffect(() => {
- if (isError) {
+ if (isError && !hasCalledError.current) {
+ hasCalledError.current = true;
onError?.();
}
}, [isError, onError]);제안 2: 에러 타입 확인 후 조건부 처리
useCourseSession이 반환하는 error 객체를 활용하여 에러 타입을 구분하세요:
const { data, isError, error } = useCourseSession(sessionId);
useEffect(() => {
if (isError && error) {
// 401 에러만 인증 페이지로 리다이렉트
if (error.response?.status === 401) {
onError?.();
} else {
// 다른 에러는 사용자에게 메시지 표시
console.error('Course session error:', error);
}
}
}, [isError, error, onError]);다음 스크립트로 useCourseSession의 에러 처리 방식을 확인하세요:
🏁 Script executed:
#!/bin/bash
# useCourseSession의 에러 처리 및 error 객체 반환 여부 확인
rg -A 10 "export const useCourseSession" --type=tsLength of output: 725
---
🏁 Script executed:
```shell
#!/bin/bash
# Check FullMap.tsx actual destructuring from useCourseSession
rg -A 5 "useCourseSession" src/shared/components/map/result/components/FullMap.tsx
Length of output: 601
🏁 Script executed:
#!/bin/bash
# Check getCourseSession API implementation to understand error structure
rg -B 5 -A 15 "export const getCourseSession" --type=tsLength of output: 1072
---
🏁 Script executed:
```shell
#!/bin/bash
# Check apiAuth instance and error handling interceptors
fd -type f "instance" --type ts | head -20
Length of output: 170
🏁 Script executed:
#!/bin/bash
# More specific search for apiAuth definition
rg -B 5 -A 20 "export.*apiAuth" --type=tsLength of output: 1257
---
🏁 Script executed:
```shell
#!/bin/bash
# Get complete apiAuth interceptor configuration, especially error response handling
rg -A 50 "apiWithToken.interceptors.response" src/shared/api/instance.ts
Length of output: 1552
🏁 Script executed:
#!/bin/bash
# Also check if apiAuth has any error interceptor
rg -A 30 "apiAuth.interceptors" src/shared/api/instance.tsLength of output: 0
중복 호출 방지를 위해 ref 도입이 필수입니다.
현재 구현에서 onError 참조가 변경되면서 isError가 true인 상태에서 effect가 재실행되어 리다이렉트가 여러 번 발생할 수 있습니다.
다음과 같이 ref를 사용하여 수정하세요:
export default function FullMap({ sessionId, onError }: FullMapProps) {
const mapRef = useRef<HTMLDivElement | null>(null);
+ const hasErrorHandled = useRef(false);
const { data, isError } = useCourseSession(sessionId);
const [selectedPlace, setSelectedPlace] = useState<CoursePlace | null>(null);
const [bubblePos, setBubblePos] = useState<{ x: number; y: number } | null>(null);
useEffect(() => {
- if (isError) {
+ if (isError && !hasErrorHandled.current) {
+ hasErrorHandled.current = true;
onError?.();
}
}, [isError, onError]);추가 확인 사항: useCourseSession은 apiAuth 인스턴스를 사용하는데, 이는 에러 인터셉터가 없습니다. PR 요구사항에서 "401/404/500/504/네트워크 오류에 대해 자동으로 인증 페이지로 리다이렉트"를 언급했다면, apiWithToken(401 처리 + 리다이렉트 로직 있음) 사용 여부 또는 apiAuth에 에러 인터셉터 추가 여부를 검토하세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/shared/components/map/result/components/FullMap.tsx around lines 18 to
22, the effect calls onError whenever isError is true and onError reference
changes, causing duplicate redirects; introduce a ref like calledOnErrorRef to
track whether the error callback has already been invoked and only call onError
when isError is true and the ref is false, then set the ref to true; optionally
reset the ref when isError becomes false if retries/reenables should allow
future calls; also verify useCourseSession uses apiWithToken (or add an error
interceptor to apiAuth) so 401/404/500/504/network errors trigger the
centralized redirect logic as required by the PR.
1 participant
🔥 작업 내용
LocationPage)에 스켈레톤 UI 적용imgLoaded단일 상태로 통합)transition-opacity기반 페이드 인 적용으로 자연스러운 전환 제공infoLoaded등 불필요한 state 제거하여 렌더링 흐름 최적화onError전달 기능 추가useCourseSessionAPI 호출 중 에러 발생 시 자동으로 로그인 페이지(/auth)로 이동하도록 처리remotePatterns중복 선언 문제 해결🤔 추후 작업 사항
🔗 이슈
PR Point (To Reviewer)
📸 피그마 스크린샷 or 기능 GIF
(작업 내역 스크린샷)
Summary by CodeRabbit
주요 변경사항
새로운 기능
개선사항