Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Git 설정 .gitignore |
.claude 항목의 줄바꿈 수정 및 dailyNote/ 무시 규칙 추가 |
배너 API 및 타입 frontend/src/apis/banner.ts, frontend/src/constants/queryKeys.ts |
BannerType, Banner 인터페이스, bannerApi.getBanners() 메서드 추가 및 배너 쿼리 키 그룹 정의 |
배너 데이터 페칭 훅 frontend/src/hooks/Queries/useBanner.ts |
React Query 기반 useGetBanners 훅 추가 (60초 staleTime 포함) |
배너 컴포넌트 구현 frontend/src/pages/MainPage/components/Banner/Banner.tsx |
useGetBanners 훅을 통한 동적 데이터 페칭, 폴백 배너 로직, 동적 렌더링 업데이트 |
배너 Storybook frontend/src/pages/MainPage/components/Banner/Banner.stories.tsx |
MSW 핸들러, Provider 래퍼 추가, Desktop/Mobile/Fallback 스토리 구현 |
Sequence Diagram(s)
sequenceDiagram
participant Client as Client (Browser)
participant Component as Banner Component
participant Hook as useGetBanners Hook
participant Cache as React Query Cache
participant API as bannerApi
participant Server as Backend API
Client->>Component: Mount with bannerType
Component->>Hook: useGetBanners(bannerType)
Hook->>Cache: Check queryKey ['banner', type]
alt Cache hit (valid)
Cache-->>Hook: Cached banners
Hook-->>Component: Loading=false, data=banners
else Cache miss or stale
Hook->>API: getBanners(bannerType)
API->>Server: GET /api/banners?type=WEB
Server-->>API: {statuscode, message, images: [...]}
API->>API: handleResponse extracts images
API-->>Hook: Promise<Banner[]>
Hook->>Cache: Store in queryKey ['banner', type]
Hook-->>Component: Loading=false, data=banners
end
Component->>Component: Compute displayBanners<br/>(fetched or fallback)
Component->>Component: Render banner carousel<br/>with displayBanners
Client->>Component: Click banner
Component->>Component: Navigate via banner.linkTo
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- [feature] 앱 출시 배너를 추가한다 #938: Banner 컴포넌트 클릭/네비게이션 로직 수정으로 banner.linkTo 및 imageUrl 사용 추가 관련
- [feature] 메인화면 컴포넌트 스토리북 추가 #1128: Banner.stories.tsx 파일 초기 생성 이후 MSW 핸들러, Provider, 추가 스토리로 대폭 확장된 관련 PR
- [feature] 2026 동아리 소개 한마당 페이지 완성 #1284: 배너 데이터에 linkTo 값 추가로 본 PR의 API 및 훅과 직접 연결
Suggested reviewers
- lepitaaar
- oesnuj
- suhyun113
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | 제목 '[feature] 배너 api 연동'은 풀 리퀘스트의 주요 변경 사항인 배너 API 연동 기능을 명확하게 요약하고 있습니다. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feature/#1330-banner-api-MOA-744
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 @coderabbitai help to get the list of available commands and usage tips.
🎨 UI 변경사항을 확인해주세요
5개 스토리 변경 · 전체 59개 스토리 · 23개 컴포넌트 |
![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: 2
🧹 Nitpick comments (1)
frontend/src/hooks/Queries/useBanner.ts (1)
6-8:AbortSignal을 fetch까지 전달해 주세요.지금 구현은 쿼리 키가 바뀌거나 컴포넌트가 언마운트돼도 기존 요청이 계속 진행됩니다. 배너 타입이
WEB/WEB_MOBILE사이에서 바뀔 수 있는 구조라서, TanStack Query v5가 제공하는signal을bannerApi.getBanners와fetch까지 넘겨 두는 편이 안전합니다.♻️ Proposed fix
export const useGetBanners = (type: BannerType = 'WEB') => { return useQuery<Banner[]>({ queryKey: queryKeys.banner.list(type), - queryFn: () => bannerApi.getBanners(type), + queryFn: ({ signal }) => bannerApi.getBanners(type, signal), staleTime: 60 * 1000, }); };// frontend/src/apis/banner.ts getBanners: async ( type: BannerType = 'WEB', signal?: AbortSignal, ): Promise<Banner[]> => { // ... const response = await fetch(url, { signal }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/Queries/useBanner.ts` around lines 6 - 8, The useQuery call in useBanner.ts isn't passing TanStack Query's AbortSignal to the API layer, so requests keep running after key changes/unmount; update bannerApi.getBanners to accept an optional signal: AbortSignal parameter and forward that signal into the underlying fetch call, and change the queryFn in useBanner (the useQuery invocation) to pass the signal from the queryFn args (i.e., call bannerApi.getBanners(type, signal)). Ensure function names mentioned: useQuery (in useBanner.ts), bannerApi.getBanners, and the internal fetch call all wire the AbortSignal through.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/pages/MainPage/components/Banner/Banner.stories.tsx`:
- Around line 8-9: The story redefines API_BASE_URL locally which can diverge
from the app and MSW handlers; remove the local fallback and instead import and
use the canonical API constant (e.g., API_BASE) from the shared API constants,
and update any references in this file (API_BASE_URL) and the bannerApi usage to
rely on that imported constant so MSW URL matching remains consistent with
bannerApi.
In `@frontend/src/pages/MainPage/components/Banner/Banner.tsx`:
- Around line 37-39: The current fallback logic in Banner.tsx uses isError to
choose fallbackBanners, which causes switching to fallback even when cached
banners exist; update the shouldUseFallback condition (used to compute
displayBanners) so it only falls back when there are no valid banners available
— e.g., require both isError (or isFetched false) and (banners == null ||
banners.length === 0) before using fallbackBanners; adjust the logic around the
shouldUseFallback and displayBanners variables (referencing shouldUseFallback,
displayBanners, banners, fallbackBanners, isFetched, isError) so a refetch error
preserves previously cached banners instead of flipping to the fallback.
---
Nitpick comments:
In `@frontend/src/hooks/Queries/useBanner.ts`:
- Around line 6-8: The useQuery call in useBanner.ts isn't passing TanStack
Query's AbortSignal to the API layer, so requests keep running after key
changes/unmount; update bannerApi.getBanners to accept an optional signal:
AbortSignal parameter and forward that signal into the underlying fetch call,
and change the queryFn in useBanner (the useQuery invocation) to pass the signal
from the queryFn args (i.e., call bannerApi.getBanners(type, signal)). Ensure
function names mentioned: useQuery (in useBanner.ts), bannerApi.getBanners, and
the internal fetch call all wire the AbortSignal through.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e7bd8b7-fed7-450c-8a66-b062d7d8b254
📒 Files selected for processing (6)
.gitignorefrontend/src/apis/banner.tsfrontend/src/constants/queryKeys.tsfrontend/src/hooks/Queries/useBanner.tsfrontend/src/pages/MainPage/components/Banner/Banner.stories.tsxfrontend/src/pages/MainPage/components/Banner/Banner.tsx
Comment on lines
+8
to
+9
| const API_BASE_URL = | ||
| import.meta.env.VITE_API_BASE_URL || 'http://localhost:3000'; |
Contributor
There was a problem hiding this comment.
API base URL을 여기서 다시 정의하지 마세요.
스토리에서만 별도 fallback을 두면 실제 bannerApi가 호출하는 URL과 MSW 핸들러의 매칭 기준이 쉽게 어긋납니다. 이 값은 공통 API 상수에서 import해서 한 곳만 진실 소스로 두는 편이 안전합니다.
Based on learnings, API 관련 상수(예: API_BASE)는 frontend/src/mocks/constants/api.ts에 정의되어 있으며, 다른 파일에서 재정의하지 말고 이 파일에서 import하여 사용해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/MainPage/components/Banner/Banner.stories.tsx` around
lines 8 - 9, The story redefines API_BASE_URL locally which can diverge from the
app and MSW handlers; remove the local fallback and instead import and use the
canonical API constant (e.g., API_BASE) from the shared API constants, and
update any references in this file (API_BASE_URL) and the bannerApi usage to
rely on that imported constant so MSW URL matching remains consistent with
bannerApi.
Comment on lines
+37
to
+39
| const shouldUseFallback = | ||
| isFetched && (isError || (banners?.length ?? 0) === 0); | ||
| const displayBanners = shouldUseFallback ? fallbackBanners : (banners ?? []); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TanStack Query v5에서 이전 성공 데이터가 있는 상태로 refetch가 실패하면, query.data는 유지되고 status/isError는 어떻게 바뀌나요?
💡 Result:
TanStack Query v5에서 이전에 한 번이라도 성공해서 캐시에 data가 있는 상태에서 refetch(백그라운드 재요청 포함)가 실패하면:
query.data: 유지됩니다. (data는 “마지막으로 성공한 데이터”로 정의됨) [1]status/isError:status: 'error'로 바뀌고,isError: true가 됩니다. (성공 데이터가 있어도 “이번 시도”가 에러면 status가 error가 될 수 있음) [1][2]- 추가로 구분 플래그:
isRefetchError: true(refetch 중 실패) [1]isLoadingError: false(첫 로드 실패가 아님) [1]
참고: 이때 보통 UI는 이전 data를 보여주면서, 에러 상태(예: 토스트/배너)를 함께 표시하는 패턴이 됩니다. [1]
출처:
[1] TanStack Query v5 useQuery API Reference (tanstack.com)
[2] TanStack Query v5 마이그레이션 가이드(백그라운드 refetch 에러 시 status가 error일 수 있음) (tanstack.com)
Citations:
- 1: https://tanstack.com/query/latest/docs/framework/react/reference/useQuery
- 2: https://tanstack.com/query/latest/docs/react/guides/migrating-to-v5
refetch 실패 시 유효한 캐시 데이터를 무시하고 있습니다.
현재 37-39번 라인의 조건은 isError로 폴백을 결정하는데, React Query v5에서 이전 성공 데이터가 있는 상태로 refetch가 실패하면:
banners데이터는 유지됩니다 (마지막 성공 데이터)isError는true로 설정됩니다
따라서 유효한 배너 데이터가 있어도 조건문이 true가 되어 갑자기 폴백 배너로 전환되므로, 사용자 경험이 저하됩니다.
수정 제안
- const shouldUseFallback =
- isFetched && (isError || (banners?.length ?? 0) === 0);
- const displayBanners = shouldUseFallback ? fallbackBanners : (banners ?? []);
+ const hasApiBanners = (banners?.length ?? 0) > 0;
+ const shouldUseFallback = isFetched && !hasApiBanners;
+ const displayBanners = hasApiBanners
+ ? banners
+ : shouldUseFallback
+ ? fallbackBanners
+ : [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/MainPage/components/Banner/Banner.tsx` around lines 37 -
39, The current fallback logic in Banner.tsx uses isError to choose
fallbackBanners, which causes switching to fallback even when cached banners
exist; update the shouldUseFallback condition (used to compute displayBanners)
so it only falls back when there are no valid banners available — e.g., require
both isError (or isFetched false) and (banners == null || banners.length === 0)
before using fallbackBanners; adjust the logic around the shouldUseFallback and
displayBanners variables (referencing shouldUseFallback, displayBanners,
banners, fallbackBanners, isFetched, isError) so a refetch error preserves
previously cached banners instead of flipping to the fallback.
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.
#️⃣연관된 이슈
📝작업 내용
api 작업
WEB,WEB_MOBILE)에 따라 쿼리 키를 분리해 조회하도록 구성했습니다.폴백 전략
다.
스켈레톤 ui가 없어서 새로고침을 빠르게 하면 높이가 새로 계산되는 것을 볼 수 있습니다. 추후에 스켈레톤을 추가하여
배너 높이 계산을 최적화할 수 있을 것 같은데 한 번 더 고민해 볼 예정입니다.
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
새로운 기능