Refactor(audience): 관객 측 FestivalCard 위젯 구조 및 상태 관리 일관성 개선#266
Merged
jin-evergreen merged 17 commits intodevelopfrom Mar 1, 2026
Merged
Refactor(audience): 관객 측 FestivalCard 위젯 구조 및 상태 관리 일관성 개선#266jin-evergreen merged 17 commits intodevelopfrom
jin-evergreen merged 17 commits intodevelopfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough홈 페이지 축제 목록 UI와 관련 훅·API를 리팩터링했습니다. HomeFestivalList 제거 후 FestivalSection/FestivalCard로 교체되고, API·쿼리 옵션은 features에서 entities/festival로 이동하며, wishlist 토글 로직과 관련 prop 시그니처가 단순화되었습니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
검토 시 주의사항 Must:
Suggest:
🚥 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. 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/audience/src/features/notice-list/hooks/use-toggle-wishlist-mutation.ts (1)
49-61: 🧹 Nitpick | 🔵 Trivial[Suggest] 독립적인
invalidateQueries호출을Promise.all로 병렬화하면 어떨까요?세
invalidateQueries호출은 서로 의존성이 없어요. 순차await대신Promise.all을 사용하면 불필요한 직렬 대기를 줄일 수 있어요.♻️ Proposed refactor: Promise.all 병렬화
onSettled: async () => { - await queryClient.invalidateQueries({ - queryKey, - }); - await queryClient.invalidateQueries({ - queryKey: HOME_QUERY_OPTIONS.ALL_FESTIVALS({ page: 0, size: 20 }) - .queryKey, - }); - await queryClient.invalidateQueries({ - queryKey: HOME_QUERY_OPTIONS.PLANNED_FESTIVALS({ page: 0, size: 20 }) - .queryKey, - }); + await Promise.all([ + queryClient.invalidateQueries({ queryKey }), + queryClient.invalidateQueries({ + queryKey: HOME_QUERY_OPTIONS.ALL_FESTIVALS().queryKey, + }), + queryClient.invalidateQueries({ + queryKey: HOME_QUERY_OPTIONS.PLANNED_FESTIVALS().queryKey, + }), + ]); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/features/notice-list/hooks/use-toggle-wishlist-mutation.ts` around lines 49 - 61, The onSettled handler currently awaits three independent queryClient.invalidateQueries calls sequentially; change it to run them in parallel by collecting the three invalidateQueries promises (the one using queryKey, HOME_QUERY_OPTIONS.ALL_FESTIVALS({ page: 0, size: 20 }).queryKey, and HOME_QUERY_OPTIONS.PLANNED_FESTIVALS({ page: 0, size: 20 }).queryKey) and awaiting them together with Promise.all so the invalidations execute concurrently rather than serially.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/audience/src/features/notice-list/hooks/use-toggle-wishlist-mutation.ts`:
- Around line 53-60: Replace the hardcoded param-specific invalidations with
base-key invalidation to improve maintainability: instead of calling
queryClient.invalidateQueries with HOME_QUERY_OPTIONS.ALL_FESTIVALS({ page: 0,
size: 20 }).queryKey and HOME_QUERY_OPTIONS.PLANNED_FESTIVALS({ page: 0, size:
20 }).queryKey, call queryClient.invalidateQueries using the base keys for those
entries (e.g., the base key values exposed by HOME_QUERY_OPTIONS.ALL_FESTIVALS
and HOME_QUERY_OPTIONS.PLANNED_FESTIVALS or their root queryKey arrays) so all
paginated variants are invalidated; update the calls in
use-toggle-wishlist-mutation.ts where queryClient.invalidateQueries is used to
pass the base key instead of the param object.
- Line 5: Move the shared query-options out of the home feature into a shared
layer and stop importing HOME_QUERY_OPTIONS from the home feature in
use-toggle-wishlist-mutation.ts: create a shared module (e.g.,
`@shared/apis/query-options.ts`) that exports HOME_QUERY_OPTIONS and update both
home and notice-list usages (including where putWishList is used) to import from
that shared module; additionally, fix the query invalidation in the mutation by
invalidating the base queryKey (call HOME_QUERY_OPTIONS.ALL_FESTIVALS() with no
params) instead of using hard-coded params like {page:0,size:20} so the fuzzy
matching works and invalidation won't silently fail.
In
`@apps/audience/src/widgets/home/components/home-festival-card/home-festival-card.tsx`:
- Around line 7-10: The imports AllFestivalItem and UpcomingFestivalItem are
type-only and should use TypeScript's type-only import to avoid runtime
dependencies; update the import statement that currently brings in
AllFestivalItem and UpcomingFestivalItem (from '@shared/types/home-response') to
use "import type" for those symbols so they are erased at runtime.
- Around line 17-29: HomeFestivalCard is not passing the festival.status into
FestivalStatusGroup so the status chip never renders; update the component to
read the status from the incoming festival (either by adding status to the
destructuring or by checking 'status' in festival to handle the union type like
UpcomingFestivalItem vs AllFestivalItem) and pass it as the statusText prop to
FestivalStatusGroup (e.g., <FestivalStatusGroup dDay={formatDday(dDay)}
isWishlist={wishList} statusText={festival.status ?? undefined} />) so the "진행
중"/"upcoming" chip shows correctly.
- Line 26: The CardFestival.Image usage passes mainImageUrl ?? '' which can
produce src="" and trigger unwanted browser requests; update the
home-festival-card component to either conditionally render CardFestival.Image
only when mainImageUrl is truthy or replace the empty string fallback with a
meaningful placeholder constant (e.g., DEFAULT_FESTIVAL_IMAGE). Locate the
CardFestival.Image invocation and change it to conditional rendering (skip
rendering when mainImageUrl is missing) or use the placeholder constant so src
is never an empty string.
In
`@apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.ts`:
- Around line 10-16: The CSS properties for the exported style "content" are out
of Mozilla-style order; move Display & Layout properties (display,
flexDirection, flex, minHeight) before Box Model (padding) so the object lists
display, flexDirection, flex, minHeight, then padding to follow "Display &
Layout → Box Model → Visual → Typography → Content" ordering; update the style
definition in home-festival-section.css.ts for the content constant accordingly.
In
`@apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.tsx`:
- Around line 65-73: The festival card list uses a non-semantic <div> container;
change it to a semantic list by replacing the wrapper with a <ul> and wrapping
each HomeFestivalCard in an <li> (move the key from the HomeFestivalCard to the
<li>), keeping the onClick handler (handleMoveToFestival(festival.festivalId))
on the clickable element or the card as before, and update styles.cardList
selectors (or add a new class) to target the <ul>/<li> instead of the <div> so
layout and styling remain unchanged while improving accessibility for
targetFestivals rendering.
- Around line 37-39: Change handleMoveToFestival to a curried function that
returns the click handler so you can pass it directly to onClick instead of
creating an inline arrow each render; specifically, replace the current
handleMoveToFestival(festivalId: number) => { navigate(...) } with a factory
like handleMoveToFestival = (festivalId: number) => () => {
navigate(ROUTE_PATH.NOTICE_LIST.replace(':eventId', String(festivalId))); } and
update usages in the map (currently onClick={() =>
handleMoveToFestival(festival.festivalId)}) to
onClick={handleMoveToFestival(festival.festivalId)} so the map body has no
inline closures.
In `@packages/ads-ui/src/components/button/flag-button/flag-button.tsx`:
- Line 22: The FlagButton component forwards the disabled prop to the <button>
element but there are no :disabled styles in flag-button.css.ts; add a :disabled
selector for the button (or the component's root class) in flag-button.css.ts
that reduces opacity (e.g., 0.5), sets cursor: not-allowed, and adjusts
backgroundColor/borderColor to the disabled tokens from your design system so
the button provides clear visual feedback when disabled; ensure the selector
targets the same class/element used by FlagButton to take effect.
In `@packages/compositions/src/festival-status-group/festival-status-group.tsx`:
- Around line 3-6: The FestivalStatusGroupProps interface is currently internal;
export it so external consumers can reference the props type (change declaration
to export interface FestivalStatusGroupProps) and update any places that should
import this type; ensure named export matches existing component usage
(FestivalStatusGroup) to avoid breaking imports.
- Line 23: The component currently hard-codes an app-domain string check
(statusText === '진행 중') in FestivalStatusGroup; replace that by switching the
component to accept a typed status prop (e.g., status: 'current' | 'upcoming' |
'ended') instead of relying on statusText, or move the mapping logic into the
caller (HomeFestivalCard) so HomeFestivalCard resolves API strings to the typed
status and passes it in; update FestivalStatusGroup's props signature and usage
points to use the new status value and remove any direct comparisons to
domain-specific strings like '진행 중'.
---
Outside diff comments:
In
`@apps/audience/src/features/notice-list/hooks/use-toggle-wishlist-mutation.ts`:
- Around line 49-61: The onSettled handler currently awaits three independent
queryClient.invalidateQueries calls sequentially; change it to run them in
parallel by collecting the three invalidateQueries promises (the one using
queryKey, HOME_QUERY_OPTIONS.ALL_FESTIVALS({ page: 0, size: 20 }).queryKey, and
HOME_QUERY_OPTIONS.PLANNED_FESTIVALS({ page: 0, size: 20 }).queryKey) and
awaiting them together with Promise.all so the invalidations execute
concurrently rather than serially.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
apps/audience/src/features/notice-list/hooks/use-toggle-wishlist-mutation.tsapps/audience/src/pages/home/home.tsxapps/audience/src/pages/home/model/use-home-festivals.tsapps/audience/src/widgets/home/components/home-festival-card/home-festival-card.tsxapps/audience/src/widgets/home/components/home-festival-list/home-festival-list.css.tsapps/audience/src/widgets/home/components/home-festival-list/home-festival-list.tsxapps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.tsapps/audience/src/widgets/home/components/home-festival-section/home-festival-section.tsxpackages/ads-ui/src/components/button/flag-button/flag-button.tsxpackages/compositions/src/festival-status-group/festival-status-group.tsxpackages/compositions/src/index.ts
💤 Files with no reviewable changes (3)
- apps/audience/src/pages/home/home.tsx
- apps/audience/src/widgets/home/components/home-festival-list/home-festival-list.css.ts
- apps/audience/src/widgets/home/components/home-festival-list/home-festival-list.tsx
apps/audience/src/features/notice-list/hooks/use-toggle-wishlist-mutation.ts
Outdated
Show resolved
Hide resolved
apps/audience/src/features/usecase/toggle-wishlist/use-toggle-wishlist-mutation.ts
Show resolved
Hide resolved
apps/audience/src/widgets/home/components/home-festival-card/home-festival-card.tsx
Outdated
Show resolved
Hide resolved
apps/audience/src/widgets/home/components/home-festival-card/home-festival-card.tsx
Outdated
Show resolved
Hide resolved
apps/audience/src/widgets/home/components/home-festival-card/home-festival-card.tsx
Outdated
Show resolved
Hide resolved
apps/audience/src/widgets/home/components/festival-section/festival-section.tsx
Outdated
Show resolved
Hide resolved
apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.tsx
Outdated
Show resolved
Hide resolved
Comment on lines
+3
to
+6
| interface FestivalStatusGroupProps { | ||
| dDay: string; | ||
| statusText?: string; | ||
| isWishlist?: boolean; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Suggest: FestivalStatusGroupProps 타입 export 고려
현재 FestivalStatusGroupProps가 파일 내부에만 선언되어 있어요. 외부 소비자가 props 타입을 참조해야 할 때 불편할 수 있으니, 필요 시 export 추가를 고려해 보세요.
-interface FestivalStatusGroupProps {
+export interface FestivalStatusGroupProps {📝 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.
Suggested change
| interface FestivalStatusGroupProps { | |
| dDay: string; | |
| statusText?: string; | |
| isWishlist?: boolean; | |
| export interface FestivalStatusGroupProps { | |
| dDay: string; | |
| statusText?: string; | |
| isWishlist?: boolean; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/compositions/src/festival-status-group/festival-status-group.tsx`
around lines 3 - 6, The FestivalStatusGroupProps interface is currently
internal; export it so external consumers can reference the props type (change
declaration to export interface FestivalStatusGroupProps) and update any places
that should import this type; ensure named export matches existing component
usage (FestivalStatusGroup) to avoid breaking imports.
packages/compositions/src/festival-status-group/festival-status-group.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/audience/src/shared/hooks/use-toggle-wishlist-mutation.ts (1)
48-57: 🧹 Nitpick | 🔵 TrivialSuggest: 독립적인 invalidation 호출을
Promise.all로 병렬화해요.
onSettled내의 세invalidateQueries호출은 서로 의존성이 없어요. 순차await대신Promise.all을 사용하면 불필요한 직렬화를 피하고 전체 소요 시간을 줄일 수 있어요.♻️ 제안: Promise.all로 병렬 invalidation
onSettled: async () => { - await queryClient.invalidateQueries({ - queryKey, - }); - await queryClient.invalidateQueries({ - queryKey: HOME_QUERY_OPTIONS.ALL_FESTIVALS().queryKey, - }); - await queryClient.invalidateQueries({ - queryKey: HOME_QUERY_OPTIONS.PLANNED_FESTIVALS().queryKey, - }); + await Promise.all([ + queryClient.invalidateQueries({ queryKey }), + queryClient.invalidateQueries({ + queryKey: HOME_QUERY_OPTIONS.ALL_FESTIVALS().queryKey, + }), + queryClient.invalidateQueries({ + queryKey: HOME_QUERY_OPTIONS.PLANNED_FESTIVALS().queryKey, + }), + ]); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/shared/hooks/use-toggle-wishlist-mutation.ts` around lines 48 - 57, The onSettled handler is performing three independent sequential await calls to queryClient.invalidateQueries; replace them with a parallel Promise.all of the three invalidateQueries calls (the one using queryKey, HOME_QUERY_OPTIONS.ALL_FESTIVALS().queryKey, and HOME_QUERY_OPTIONS.PLANNED_FESTIVALS().queryKey) so invalidations run concurrently and avoid unnecessary serialization in the use-toggle-wishlist-mutation onSettled block.
♻️ Duplicate comments (2)
apps/audience/src/widgets/home/components/home-festival-card/home-festival-card.tsx (1)
27-29:⚠️ Potential issue | 🟠 MajorMust:
UpcomingFestivalItem.status가 여전히FestivalStatusGroup에 전달되지 않아요
FestivalStatusGroup에statusTextprop이 전달되지 않고 있어요.UpcomingFestivalItem에는status필드가 있지만, 현재 구조 분해에서 누락되어 상태 칩이 렌더링되지 않아요.🐛 Proposed fix
const { festivalId, title, period, mainImageUrl, wishList, dDay } = festival; + const statusText = 'status' in festival ? festival.status : undefined;- <FestivalStatusGroup dDay={formatDday(dDay)} isWishlist={wishList} /> + <FestivalStatusGroup + dDay={formatDday(dDay)} + statusText={statusText} + isWishlist={wishList} + />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/widgets/home/components/home-festival-card/home-festival-card.tsx` around lines 27 - 29, The FestivalStatusGroup is missing its statusText prop: update the component usage in home-festival-card.tsx (where FestivalStatusGroup is rendered) to pass the festival's status (e.g., statusText={status}) and ensure the parent destructuring (UpcomingFestivalItem) includes the status field so status is available for passing; keep the existing dDay and isWishlist props intact.apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.ts (1)
10-16: ✅ 이전 리뷰 코멘트 반영 확인
content스타일의 CSS 속성 순서가display → flexDirection → flex → minHeight → padding으로 올바르게 수정됐어요. Mozilla Style 가이드라인에 맞게 잘 반영해 주셨습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.ts` around lines 10 - 16, The content style object (export const content) already has the CSS properties ordered per the Mozilla Style guideline (display, flexDirection, flex, minHeight, padding), so no code changes are required; keep the current ordering and commit as-is or mark the change approved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/audience/src/shared/apis/festival.ts`:
- Around line 43-59: The constant HOME_QUERY_OPTIONS in
apps/audience/src/shared/apis/festival.ts is domain-scoped and violates the
Shared layer naming guideline; rename HOME_QUERY_OPTIONS to
FESTIVAL_QUERY_OPTIONS (or move it into the home page model if it is truly
home-specific) and update all references (e.g., use-home-festivals.ts,
use-toggle-wishlist-mutation.ts and any imports that use HOME_QUERY_OPTIONS) to
the new symbol name so the shared API reflects a domain-agnostic, reusable
festival query options export.
- Around line 3-4: 현재 코드에서 '@amp/apis'를 두 번 import하고 있어요; 중복된 import 구문을 하나로
합치세요: '@amp/apis'에서 put과 get을 한 import 문으로 병합하여 가져오도록 수정하고, 관련된 중복된 import 라인(현재
get과 put을 각각 가져오는 구문)을 제거하세요. 이 변경은 파일의 상단 import 블록(특히 get, put 심볼을 사용하는 부분)에만
적용하면 됩니다.
In `@apps/audience/src/shared/hooks/use-toggle-wishlist-mutation.ts`:
- Around line 5-6: 현재 use-toggle-wishlist-mutation.ts에서 같은
모듈('@shared/apis/festival')을 두 번 임포트하고 있으니 HOME_QUERY_OPTIONS와 putWishList를 하나의
import 문으로 합치세요; 파일의 상단 import들을 찾아 HOME_QUERY_OPTIONS, putWishList를 단일 중괄호
import로 결합하면 됩니다.
In
`@apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.ts`:
- Line 15: home-festival-section.css.ts 에서 style 객체의 padding: '2rem' 과 gap:
'1rem' 같은 하드코딩된 spacing 값을 디자인 토큰(vars)으로 교체하세요; 해당 파일 상단에 프로젝트의 디자인 토큰 모듈에서
vars를 임포트한 뒤 padding과 gap에 각각 적절한 토큰(예: vars.spacing.xxl 또는 vars.space[?], 프로젝트
규약에 맞는 이름)을 할당하고 타입이 string 인지 확인해 두 군데(현재의 padding, gap 사용 위치)를 모두 수정하세요.
- Around line 18-23: Rename the CSS export emptyWrapper to follow project
convention by using the container suffix (e.g., emptyContainer) and update all
usages accordingly; specifically change the exported constant name in
home-festival-section.css.ts from emptyWrapper to emptyContainer and update the
reference in home-festival-section.tsx to import/use the new name so the wrapper
naming is consistent with the project's container convention.
---
Outside diff comments:
In `@apps/audience/src/shared/hooks/use-toggle-wishlist-mutation.ts`:
- Around line 48-57: The onSettled handler is performing three independent
sequential await calls to queryClient.invalidateQueries; replace them with a
parallel Promise.all of the three invalidateQueries calls (the one using
queryKey, HOME_QUERY_OPTIONS.ALL_FESTIVALS().queryKey, and
HOME_QUERY_OPTIONS.PLANNED_FESTIVALS().queryKey) so invalidations run
concurrently and avoid unnecessary serialization in the
use-toggle-wishlist-mutation onSettled block.
---
Duplicate comments:
In
`@apps/audience/src/widgets/home/components/home-festival-card/home-festival-card.tsx`:
- Around line 27-29: The FestivalStatusGroup is missing its statusText prop:
update the component usage in home-festival-card.tsx (where FestivalStatusGroup
is rendered) to pass the festival's status (e.g., statusText={status}) and
ensure the parent destructuring (UpcomingFestivalItem) includes the status field
so status is available for passing; keep the existing dDay and isWishlist props
intact.
In
`@apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.ts`:
- Around line 10-16: The content style object (export const content) already has
the CSS properties ordered per the Mozilla Style guideline (display,
flexDirection, flex, minHeight, padding), so no code changes are required; keep
the current ordering and commit as-is or mark the change approved.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
apps/audience/src/features/home/apis/query.tsapps/audience/src/pages/home/model/use-home-festivals.tsapps/audience/src/pages/notice-list/notice-list.tsxapps/audience/src/shared/apis/festival.tsapps/audience/src/shared/apis/notice.tsapps/audience/src/shared/hooks/use-festival-notification.tsapps/audience/src/shared/hooks/use-toggle-wishlist-mutation.tsapps/audience/src/widgets/home/components/home-festival-card/home-festival-card.tsxapps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.ts
💤 Files with no reviewable changes (1)
- apps/audience/src/features/home/apis/query.ts
| flexDirection: 'column', | ||
| flex: 1, | ||
| minHeight: 0, | ||
| padding: '2rem', |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Suggest: 디자인 토큰(vars)을 사용해 하드코딩된 spacing 값을 대체하는 것을 고려해 주세요
padding: '2rem', gap: '1rem'이 리터럴로 선언되어 있어요. 컴포넌트 로컬 스타일 파일에서는 디자인 토큰(vars)을 직접 참조하는 것이 권장돼요. 프로젝트의 spacing 토큰이 있다면 활용해 주세요.
As per coding guidelines, "디자인 토큰(vars)을 직접 사용하고 있는지 확인해요"
Also applies to: 28-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.ts`
at line 15, home-festival-section.css.ts 에서 style 객체의 padding: '2rem' 과 gap:
'1rem' 같은 하드코딩된 spacing 값을 디자인 토큰(vars)으로 교체하세요; 해당 파일 상단에 프로젝트의 디자인 토큰 모듈에서
vars를 임포트한 뒤 padding과 gap에 각각 적절한 토큰(예: vars.spacing.xxl 또는 vars.space[?], 프로젝트
규약에 맞는 이름)을 할당하고 타입이 string 인지 확인해 두 군데(현재의 padding, gap 사용 위치)를 모두 수정하세요.
apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.ts
Outdated
Show resolved
Hide resolved
eunkr82
approved these changes
Feb 25, 2026
Contributor
eunkr82
left a comment
eunkr82
left a comment
There was a problem hiding this comment.
대규모 수정... 너무 고생 많으셨습니다 👍👍 전보다 훨씬 깔끔하고 읽기도 쉬워진 것 같아요! 크리티컬한 이슈는 아니고 궁금한 게 하나 있어서 코멘트 하나 남기고 어푸합니다!! 개발천재 진석님 ^ ^ 🌹
Comment on lines
64
to
65
| return { | ||
| toggleWishList: () => mutate(!isWishlist), |
Contributor
There was a problem hiding this comment.
지금은 훅이 호출된 시점에 캡처된 isWishList 값을 뒤집어 보내고 있는데, UI는 바뀌었지만 부모가 아직 isWIshList를 갱신하지 못한 상태거나 동시 요청이 겹치면 !wishList 계산이 이전 값 기준으로 서버에 전달되어 사용자의 의도와 다르게 남는 이슈가 발생할 수도 있을 것 같아요. 연타가 빈번한 요소라고 생각하기는 애매하지만, 현재 상태를 기준으로 다음 상태를 계산해 업데이트에 사용하면 UX 개선의 여지가 있을 것 같은데 어떻게 생각하시는지 궁금합니다!!
저는 FlagButton의 onChange를 변경 이전처럼
onChange: (nextSelected: boolean) => void로 바꾸고,
<FlagButton
selected={wishList}
onChange={(nextSelected) => toggleWishList(nextSelected)}
disabled={isTogglePending}
/>처럼 클릭 시점에 컴포넌트가 다음 상태를 계산해 전달하도록 해 클로저 이슈를 방지하는 쪽을 생각하긴 했는데, 현재는 () => void 로 수정이 되었더라고요!! 수정하신 이유가 있는지도 궁금합니당... 수정하느라 너무 고생 많으셨습니다 👍👍👍
Contributor
There was a problem hiding this comment.
nextSelected 형태가 일반적인 컨트롤 패턴이라 안정성은 좋아지지만,
현재처럼 홈에서만 사용되고 disabled로 충분히 방어된다면 변경 비용 대비 효과가 크지 않을 수도 있을 것 같습니다.
이번 PR에서는 진석님이 수정하신 것처럼 disabled로 대응하고, 이후 사용 범위가 넓어지면 그때 nextSelected 패턴 도입을 다시 논의해보면 어떨까요?
Contributor
There was a problem hiding this comment.
추가로 FlagButton의 위치에 대해 논의해보고 싶습니다.
현재 사용처가 홈 화면으로 한정되어 보이는데, 어떤 이유로 공용 UI 패키지(ads-ui)에 포함되어 있는지 궁금합니다.
만약 재사용 계획이 없다면 widgets/home으로 내려두는 편이 책임/의도 측면에서 더 명확할 것 같습니다!
Contributor
There was a problem hiding this comment.
좋습니다! FlagButton 위치 이동 관련 건도 동의합니다 👍
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/audience/src/shared/apis/festival.ts (1)
42-58:⚠️ Potential issue | 🟠 MajorMust: Shared 레이어에서
HOME_QUERY_OPTIONS명칭/역할은 범용성 원칙에 맞게 정리해 주세요.
HOME_접두사는 페이지 종속 의미가 강해서 shared 레이어 의도와 충돌합니다.FESTIVAL_QUERY_OPTIONS같은 범용 명칭으로 변경하거나, 정말 홈 전용이면 홈 모델 레이어로 이동이 필요합니다.As per coding guidelines, "
apps/**/src/shared/**: 특정 도메인(entity/feature)에 종속되면 안 돼요."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/shared/apis/festival.ts` around lines 42 - 58, The constant HOME_QUERY_OPTIONS in apps/audience/src/shared/apis/festival.ts is page-specific and should be made domain-agnostic or moved: either rename HOME_QUERY_OPTIONS to FESTIVAL_QUERY_OPTIONS and update all references to the new name (keeping the inner entries ALL_FESTIVALS, PLANNED_FESTIVALS, UPCOMING_FESTIVAL, queryFn calls getAllFestivals/getPlannedFestivals/getUpcomingFestival and queryKey uses USERS_QUERY_KEY.* as-is), or if these options are truly home-page-only, move the entire constant into the home model layer and update callers to import from the home module instead of the shared folder. Ensure imports/exports and any tests or usages are updated to match the chosen change.apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.tsx (1)
65-73: 🧹 Nitpick | 🔵 TrivialSuggest: 카드 반복 렌더링은
<ul>/<li>로 바꾸시면 접근성이 더 좋아집니다.Line 65-73은 목록성 데이터라 시맨틱 리스트 구조가 더 적합합니다. (스크린리더 내비게이션 개선)
♻️ 제안 코드
- <div className={styles.cardList}> + <ul className={styles.cardList}> {targetFestivals.map((festival) => ( - <HomeFestivalCard - key={festival.festivalId} - festival={festival} - onClick={() => handleMoveToFestival(festival.festivalId)} - /> + <li key={festival.festivalId}> + <HomeFestivalCard + festival={festival} + onClick={() => handleMoveToFestival(festival.festivalId)} + /> + </li> ))} - </div> + </ul>As per coding guidelines, "Avoid using meaningless div tags; use semantic HTML or custom components instead".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.tsx` around lines 65 - 73, Replace the non-semantic wrapper div used for list rendering with a semantic <ul> and wrap each HomeFestivalCard in an <li>: locate the targetFestivals map that renders HomeFestivalCard (symbols: targetFestivals, HomeFestivalCard, handleMoveToFestival, styles.cardList) and change the outer <div className={styles.cardList}> to <ul className={styles.cardList}>, then render each item inside an <li key={festival.festivalId}> while preserving the festival prop and onClick handler (handleMoveToFestival(festival.festivalId)); ensure styling expects a ul/li structure and adjust CSS if needed.apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.ts (1)
15-15: 🛠️ Refactor suggestion | 🟠 MajorSuggest: spacing 하드코딩 대신 디자인 토큰으로 교체해 주세요.
Line 15의
padding: '2rem', Line 28의gap: '1rem'은 토큰 미사용 상태입니다. 컴포넌트 로컬 스타일 파일에서는 spacing vars로 맞춰 주시는 게 좋겠습니다.As per coding guidelines, "디자인 토큰(vars)을 직접 사용하고 있는지 확인해요".
Also applies to: 28-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.ts` at line 15, Replace the hardcoded spacing in home-festival-section.css.ts: change the padding declaration that currently uses '2rem' and the gap declaration that uses '1rem' to use the design spacing tokens/vars instead; update the padding and gap entries in the exported styles (look for the padding: '2rem' and gap: '1rem' entries) to reference the appropriate spacing vars (e.g., the project's spacing tokens such as vars.spacing.* or tokens.spacing.*) so the component uses the design system spacing tokens.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/audience/src/shared/apis/festival.ts`:
- Around line 45-56: The queryKey currently embeds the params object as part of
the fixed key which prevents invalidate calls that use the base key from
matching (e.g., USERS_QUERY_KEY.HOME_FESTIVALS_ALL() vs
USERS_QUERY_KEY.HOME_FESTIVALS_ALL() + params); update the query construction so
the base key (USERS_QUERY_KEY.HOME_FESTIVALS_ALL / HOME_FESTIVALS_PLANNED /
HOME_FESTIVAL_UPCOMING) is kept as a stable array and params are appended as a
separate, optional element only when calling queryOptions (e.g., queryKey:
[...USERS_QUERY_KEY.HOME_FESTIVALS_ALL(), params] but ensure the USERS_QUERY_KEY
helpers return just the base array), and then change invalidation in
useToggleWishListMutation to call invalidateQueries with the base key
(USERS_QUERY_KEY.HOME_FESTIVALS_ALL()) so cache invalidation matches.
---
Duplicate comments:
In `@apps/audience/src/shared/apis/festival.ts`:
- Around line 42-58: The constant HOME_QUERY_OPTIONS in
apps/audience/src/shared/apis/festival.ts is page-specific and should be made
domain-agnostic or moved: either rename HOME_QUERY_OPTIONS to
FESTIVAL_QUERY_OPTIONS and update all references to the new name (keeping the
inner entries ALL_FESTIVALS, PLANNED_FESTIVALS, UPCOMING_FESTIVAL, queryFn calls
getAllFestivals/getPlannedFestivals/getUpcomingFestival and queryKey uses
USERS_QUERY_KEY.* as-is), or if these options are truly home-page-only, move the
entire constant into the home model layer and update callers to import from the
home module instead of the shared folder. Ensure imports/exports and any tests
or usages are updated to match the chosen change.
In
`@apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.ts`:
- Line 15: Replace the hardcoded spacing in home-festival-section.css.ts: change
the padding declaration that currently uses '2rem' and the gap declaration that
uses '1rem' to use the design spacing tokens/vars instead; update the padding
and gap entries in the exported styles (look for the padding: '2rem' and gap:
'1rem' entries) to reference the appropriate spacing vars (e.g., the project's
spacing tokens such as vars.spacing.* or tokens.spacing.*) so the component uses
the design system spacing tokens.
In
`@apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.tsx`:
- Around line 65-73: Replace the non-semantic wrapper div used for list
rendering with a semantic <ul> and wrap each HomeFestivalCard in an <li>: locate
the targetFestivals map that renders HomeFestivalCard (symbols: targetFestivals,
HomeFestivalCard, handleMoveToFestival, styles.cardList) and change the outer
<div className={styles.cardList}> to <ul className={styles.cardList}>, then
render each item inside an <li key={festival.festivalId}> while preserving the
festival prop and onClick handler (handleMoveToFestival(festival.festivalId));
ensure styling expects a ul/li structure and adjust CSS if needed.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
apps/audience/src/shared/apis/festival.tsapps/audience/src/shared/hooks/use-toggle-wishlist-mutation.tsapps/audience/src/widgets/home/components/home-festival-section/home-festival-section.css.tsapps/audience/src/widgets/home/components/home-festival-section/home-festival-section.tsx
Comment on lines
+45
to
+56
| queryKey: [...USERS_QUERY_KEY.HOME_FESTIVALS_ALL(), params], | ||
| queryFn: () => getAllFestivals(params), | ||
| }), | ||
| PLANNED_FESTIVALS: (params: PageSizeParams = {}) => | ||
| queryOptions({ | ||
| queryKey: [...USERS_QUERY_KEY.HOME_FESTIVALS_PLANNED(), params], | ||
| queryFn: () => getPlannedFestivals(params), | ||
| }), | ||
| UPCOMING_FESTIVAL: (params: PageSizeParams = {}) => | ||
| queryOptions({ | ||
| queryKey: [...USERS_QUERY_KEY.HOME_FESTIVAL_UPCOMING(), params], | ||
| queryFn: () => getUpcomingFestival(params), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the festival.ts file and examine its full content
find apps -name "festival.ts" -type f 2>/dev/null | head -20Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 190
🏁 Script executed:
# Check the festival.ts file around the target lines
cat -n apps/audience/src/shared/apis/festival.ts | head -80Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 2315
🏁 Script executed:
# Search for calls with non-empty params
rg -nP --type=ts --type=tsx 'HOME_QUERY_OPTIONS\.(ALL_FESTIVALS|PLANNED_FESTIVALS|UPCOMING_FESTIVAL)\(\s*\{[^)]*\}\s*\)' apps/audience/src -C2Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 94
🏁 Script executed:
# Search for invalidateQueries calls without params
rg -nP --type=ts --type=tsx 'invalidateQueries\(\{\s*queryKey:\s*HOME_QUERY_OPTIONS\.(ALL_FESTIVALS|PLANNED_FESTIVALS|UPCOMING_FESTIVAL)\(\)\.queryKey' apps/audience/src -C2Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 94
🏁 Script executed:
# Also check for more general invalidateQueries patterns that might target these keys
rg -nP --type=ts --type=tsx 'invalidateQueries.*HOME_QUERY_OPTIONS\.(ALL_FESTIVALS|PLANNED_FESTIVALS|UPCOMING_FESTIVAL)' apps/audience/src -C2Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 94
🏁 Script executed:
# Find files that use HOME_QUERY_OPTIONS
rg -l 'HOME_QUERY_OPTIONS' apps/audience/src --type tsRepository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 228
🏁 Script executed:
# Find useQuery or useSuspenseQuery calls with the festival options
rg -n 'useSuspenseQuery\|useQuery.*HOME_QUERY_OPTIONS\.(ALL_FESTIVALS|PLANNED_FESTIVALS|UPCOMING_FESTIVAL)' apps/audience/src -A3 -B1Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 49
🏁 Script executed:
# Search for invalidateQueries calls on these query keys
rg -n 'invalidateQueries' apps/audience/src -A5 -B1 | head -100Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 3075
🏁 Script executed:
# Check USERS_QUERY_KEY structure to understand base keys
rg -n 'HOME_FESTIVALS_ALL\|HOME_FESTIVALS_PLANNED\|HOME_FESTIVAL_UPCOMING' apps/audience/src --type ts -A2 -B2Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 49
🏁 Script executed:
# Check use-home-festivals.ts to see how queries are actually called
cat -n apps/audience/src/pages/home/model/use-home-festivals.tsRepository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 1861
🏁 Script executed:
# Check use-toggle-wishlist-mutation.ts to see full context of invalidation
cat -n apps/audience/src/shared/hooks/use-toggle-wishlist-mutation.ts | head -70Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 2271
🏁 Script executed:
# Search for where HOME_QUERY_OPTIONS calls are made with parameters
rg -n 'HOME_QUERY_OPTIONS\.(ALL_FESTIVALS|PLANNED_FESTIVALS|UPCOMING_FESTIVAL)' apps/audience/src -B2 -A2Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 2220
🏁 Script executed:
# Find the USERS_QUERY_KEY definition to understand base key structure
find apps -name "*query-key*" -type fRepository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 158
🏁 Script executed:
# Check the query-key definition
cat -n apps/audience/src/shared/constants/query-key.ts | head -100Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 2011
Must: params 객체를 queryKey의 고정 요소로 두면 invalidate 범위가 누락됩니다.
useHomeFestivals에서는 ALL_FESTIVALS와 PLANNED_FESTIVALS를 { page: 0, size: 20 }으로 호출하는데, useToggleWishListMutation에서는 이들을 무인자로 invalidate하고 있어요.
현재 구조에서는:
- 쿼리 생성:
['users', 'home-festivals-all', { page: 0, size: 20 }] - 무효화:
['users', 'home-festivals-all', {}](기본값)
두 키가 다르기 때문에 무효화가 적용되지 않아 캐시가 남아있게 돼요. base key와 param key를 분리하는 것이 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/audience/src/shared/apis/festival.ts` around lines 45 - 56, The queryKey
currently embeds the params object as part of the fixed key which prevents
invalidate calls that use the base key from matching (e.g.,
USERS_QUERY_KEY.HOME_FESTIVALS_ALL() vs USERS_QUERY_KEY.HOME_FESTIVALS_ALL() +
params); update the query construction so the base key
(USERS_QUERY_KEY.HOME_FESTIVALS_ALL / HOME_FESTIVALS_PLANNED /
HOME_FESTIVAL_UPCOMING) is kept as a stable array and params are appended as a
separate, optional element only when calling queryOptions (e.g., queryKey:
[...USERS_QUERY_KEY.HOME_FESTIVALS_ALL(), params] but ensure the USERS_QUERY_KEY
helpers return just the base array), and then change invalidation in
useToggleWishListMutation to call invalidateQueries with the base key
(USERS_QUERY_KEY.HOME_FESTIVALS_ALL()) so cache invalidation matches.
Contributor
There was a problem hiding this comment.
여기 부분도 짚어볼 필요가 있어 보여요. 현재 queryKey에 { page, size } params가 포함되어 있는데, invalidate는 무인자(base key)로 호출되고 있어 invalidate가 누락될 수 있어요.
더불어 페이지와 사이즈를 고정으로 불러오고 있는 걸로 알고 있어요!
앱잼 기간 내에는 데이터가 많지 않아 문제가 안되었지만, 스프린트 기간에는 페이지네이션(or 무한스크롤)의 처리가 필요하다고 생각해요! 기디 회의를 통해서 명확히 정하는 시간이 필요할 거 같습니다.
There was a problem hiding this comment.
@Sohyunnnn, 맞아요! 정확히 짚어주셨어요.
TanStack Query의 invalidateQueries는 부분 매칭을 지원하지만, 현재 구조에서는 다음과 같은 불일치가 발생해요:
실제 쿼리:
['users', 'home-festivals-all', { page: 0, size: 20 }]
무효화 호출:
['users', 'home-festivals-all', {}] // 기본값
빈 객체 {}와 { page: 0, size: 20 }는 정확히 일치하지 않기 때문에 무효화가 적용되지 않아요.
단기 해결 방안:
현재는 page/size가 고정값이므로, 무효화 시 base key만 사용하면 모든 하위 키를 매칭할 수 있어요:
queryClient.invalidateQueries({
queryKey: USERS_QUERY_KEY.HOME_FESTIVALS_ALL()
})이렇게 하면 파라미터와 무관하게 ['users', 'home-festivals-all']로 시작하는 모든 쿼리가 무효화돼요.
장기 계획:
페이지네이션이나 무한 스크롤을 도입할 때는 useInfiniteQuery나 별도의 쿼리 키 구조 설계가 필요할 거예요. 이 부분은 팀 회의에서 결정하시는 게 좋겠어요!
필요하시면 이슈로 등록해서 추적하는 것도 도와드릴 수 있어요. 어떻게 진행하시겠어요?
🧠 Learnings used
Learnt from: Sohyunnnn
Repo: TEAM-DECIBEL/AMP-CLIENT PR: 264
File: apps/audience/src/features/notice-list/hooks/use-toggle-wishlist-mutation.ts:6-6
Timestamp: 2026-02-19T15:19:18.439Z
Learning: The putWishList API is used across multiple features (e.g., home, notice-list). Place its implementation in the Shared layer rather than a feature module to promote reuse and reduce duplication. Update imports to reference the shared path (e.g., apps/audience/src/shared/...) and adjust any feature-specific hooks to consume the shared API.
Sohyunnnn
reviewed
Feb 26, 2026
Contributor
Sohyunnnn
left a comment
Sohyunnnn
left a comment
There was a problem hiding this comment.
수고 많으셨습니다!! 전반적으로 구조가 잘 정리된 것 같아요.
궁금한 부분은 코멘트로 남겨두었으니 같이 논의해봅시다 :)
추가로 PR을 보면서, 현재처럼 features를 도메인별로 구성하고 있는 상황(예: home / notice-list)에서는
교차로 사용되는 유즈케이스(예: 위시 토글)가 생길 때 위치가 애매해질 수 있어 보였습니다.
그래서 컨벤션을 아래처럼 잡아보는 건 어떨까요?
- 도메인별 feature(home/notice-list 등)는 화면 조립과 도메인 전용 로직을 담는다.
- 여러 도메인에서 사용하는 유즈케이스(예: 위시 토글)는
features/usecase/<usecase>로 분리한다.
이 기준이면 이번 useToggleWishListMutation도 shared로 이동하기보다는
features/usecase/toggle-wishlist 같은 형태로 분리하는 쪽이 더 자연스러울 것 같습니다.
Comment on lines
+45
to
+56
| queryKey: [...USERS_QUERY_KEY.HOME_FESTIVALS_ALL(), params], | ||
| queryFn: () => getAllFestivals(params), | ||
| }), | ||
| PLANNED_FESTIVALS: (params: PageSizeParams = {}) => | ||
| queryOptions({ | ||
| queryKey: [...USERS_QUERY_KEY.HOME_FESTIVALS_PLANNED(), params], | ||
| queryFn: () => getPlannedFestivals(params), | ||
| }), | ||
| UPCOMING_FESTIVAL: (params: PageSizeParams = {}) => | ||
| queryOptions({ | ||
| queryKey: [...USERS_QUERY_KEY.HOME_FESTIVAL_UPCOMING(), params], | ||
| queryFn: () => getUpcomingFestival(params), |
Contributor
There was a problem hiding this comment.
여기 부분도 짚어볼 필요가 있어 보여요. 현재 queryKey에 { page, size } params가 포함되어 있는데, invalidate는 무인자(base key)로 호출되고 있어 invalidate가 누락될 수 있어요.
더불어 페이지와 사이즈를 고정으로 불러오고 있는 걸로 알고 있어요!
앱잼 기간 내에는 데이터가 많지 않아 문제가 안되었지만, 스프린트 기간에는 페이지네이션(or 무한스크롤)의 처리가 필요하다고 생각해요! 기디 회의를 통해서 명확히 정하는 시간이 필요할 거 같습니다.
Comment on lines
64
to
65
| return { | ||
| toggleWishList: () => mutate(!isWishlist), |
Contributor
There was a problem hiding this comment.
nextSelected 형태가 일반적인 컨트롤 패턴이라 안정성은 좋아지지만,
현재처럼 홈에서만 사용되고 disabled로 충분히 방어된다면 변경 비용 대비 효과가 크지 않을 수도 있을 것 같습니다.
이번 PR에서는 진석님이 수정하신 것처럼 disabled로 대응하고, 이후 사용 범위가 넓어지면 그때 nextSelected 패턴 도입을 다시 논의해보면 어떨까요?
Comment on lines
64
to
65
| return { | ||
| toggleWishList: () => mutate(!isWishlist), |
Contributor
There was a problem hiding this comment.
추가로 FlagButton의 위치에 대해 논의해보고 싶습니다.
현재 사용처가 홈 화면으로 한정되어 보이는데, 어떤 이유로 공용 UI 패키지(ads-ui)에 포함되어 있는지 궁금합니다.
만약 재사용 계획이 없다면 widgets/home으로 내려두는 편이 책임/의도 측면에서 더 명확할 것 같습니다!
apps/audience/src/widgets/home/components/festival-card/festival-card.tsx
Show resolved
Hide resolved
apps/audience/src/widgets/home/components/home-festival-section/home-festival-section.tsx
Outdated
Show resolved
Hide resolved
Comment on lines
4
to
+6
| export { default as CategorySection } from './category-section/category-section'; | ||
| export { type CategoryType } from './category-section/category-section'; | ||
| export { default as FestivalStatusGroup } from './festival-status-group/festival-status-group'; |
Contributor
There was a problem hiding this comment.
해당 PR 이슈는 아니지만, CategoryType을 추가하신 이유가 궁금합니다!
현재는 type CategoryType = string 형태라 타입 안정성 측면에서 string과 차이가 크지 않아 보이는데, 추후 union 타입/객체 형태로 확장 계획이 있으셨을까요?
Member
Author
There was a problem hiding this comment.
지금 상태로는 말씀해주신 것처럼 string과 타입 안정성 측면에서 차이가 크지 않습니다..!
다만 단순히 string으로 선언하는 것보다 CategoryType으로 사용할 때 어떤 역할인지 직관적으로 이해 가능하고, 추후 확장 가능성 측면에서도 좋다고 판단했습니다!
jin-evergreen
changed the title
jin-evergreen
changed the title
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/audience/src/features/usecase/toggle-wishlist/use-toggle-wishlist-mutation.ts (1)
49-59: 🧹 Nitpick | 🔵 TrivialSuggest: 쿼리 무효화 시 정확한 prefix 매칭을 위해 base key 배열만 사용하는 것을 고려해 주세요.
현재
FESTIVAL_QUERY_OPTIONS.ALL_FESTIVALS().queryKey는 기본 파라미터{ page: 0, size: 20 }가 포함된 전체 키를 반환해요. TanStack Query v5의invalidateQueries는 기본적으로 prefix 매칭을 하지만, 다른 페이지네이션 파라미터로 캐시된 쿼리가 있다면 무효화되지 않을 수 있어요.
USERS_QUERY_KEY.HOME_FESTIVALS_ALL()같은 base key 배열을 직접 사용하면 모든 관련 쿼리가 확실히 무효화돼요.#!/bin/bash # USERS_QUERY_KEY 정의 확인 echo "=== USERS_QUERY_KEY 구조 확인 ===" rg -A 3 "HOME_FESTIVALS_ALL|HOME_FESTIVALS_PLANNED" apps/audience/src/shared/constants/query-key.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/features/usecase/toggle-wishlist/use-toggle-wishlist-mutation.ts` around lines 49 - 59, The invalidate logic uses full queryKey objects (FESTIVAL_QUERY_OPTIONS.ALL_FESTIVALS().queryKey and .PLANNED_FESTIVALS().queryKey) which include pagination params and may miss other cached variants; update the onSettled handler to call queryClient.invalidateQueries with the base key array (prefix) instead of the full key so all related pages are invalidated — replace the two FESTIVAL_QUERY_OPTIONS.*.queryKey usages with their corresponding base key arrays (e.g., the shared base key like USERS_QUERY_KEY.HOME_FESTIVALS_ALL or the base part of FESTIVAL_QUERY_OPTIONS.*), keeping the existing invalidate for the local queryKey variable and still using queryClient.invalidateQueries to perform the invalidation.apps/audience/src/shared/types/home-response.ts (2)
41-49: 🧹 Nitpick | 🔵 TrivialSuggest:
dday필드명 케이스 일관성을 확인해 주세요.Line 8의
AllFestivalItem에서는dDay(camelCase)를 사용하고, Line 48의UpcomingFestivalResponse에서는dday(lowercase)를 사용하고 있어요. API 응답과 일치하는지 확인하고, 가능하다면 일관된 네이밍을 적용해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/shared/types/home-response.ts` around lines 41 - 49, The two interfaces use inconsistent casing for the same field: UpcomingFestivalResponse declares dday while AllFestivalItem declares dDay; confirm the actual API response field casing and make them consistent (either rename UpcomingFestivalResponse.dday to dDay or change AllFestivalItem.dDay to dday) and update any related mappers/serializers/deserializers and usages of UpcomingFestivalResponse or AllFestivalItem to match the chosen name (ensure exported types, JSON parsing, and tests reflect the change).
22-29: 🧹 Nitpick | 🔵 TrivialSuggest: PaginationResponse 중복 정의를 통합해 주세요.
PaginationResponse인터페이스가apps/audience/src/shared/types/notice-response.ts에도 동일하게 정의되어 있어요. 하나의 공통 위치에서 export하고 다른 파일에서 import하는 방식으로 중복을 제거하면 유지보수성이 향상돼요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/shared/types/home-response.ts` around lines 22 - 29, PaginationResponse is duplicated across modules; extract the PaginationResponse interface into a single shared module (a dedicated pagination/type module), export it from there, then remove the local interface declarations and import PaginationResponse where currently defined (including the other module that had the duplicate) so both files reference the one exported type; update any exports that previously re-exported the local interface to forward the shared export.apps/audience/src/shared/types/viewed-festival.ts (1)
10-17: 🧹 Nitpick | 🔵 TrivialSuggest: Pagination 인터페이스 중복을 정리해 주세요.
Pagination인터페이스가PaginationResponse와 동일한 구조로 여러 파일에 정의되어 있어요. 공통 타입으로 추출하면 일관성과 유지보수성이 개선돼요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/shared/types/viewed-festival.ts` around lines 10 - 17, The Pagination interface defined here duplicates PaginationResponse elsewhere; extract a single shared type (e.g., export interface Pagination { currentPage:number; totalPages:number; totalElements:number; size:number; hasNext:boolean; hasPrevious:boolean }) into a common types module (e.g., a shared/types index) and replace local definitions with an import of that shared Pagination in files that currently declare Pagination or PaginationResponse (including the viewed-festival.ts type and any other duplicated files), update exports/imports accordingly, and remove the duplicated definitions so all code uses the single shared Pagination symbol.apps/audience/src/pages/home/model/use-home-festivals.ts (1)
39-39:⚠️ Potential issue | 🟡 MinorMust: API 응답 타입의 케이싱 일관성 확인이 필요해요
UpcomingFestivalResponse에서dday(소문자)로 정의되어 있는데,AllFestivalItem과UpcomingFestivalItem에서는dDay(camelCase)로 사용하고 있어서 타입 정의가 불일치하는 상황이에요.실제 API 응답이
dday로 오는 게 맞다면UpcomingFestivalResponse는 그대로 두되, 이를 변환할 때 명시적으로 문서화하는 게 좋겠어요. 아니면 API 응답도dDay로 통일하는 게 나을 것 같습니다. 코딩 가이드라인상 camelCase를 권장하니까요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/pages/home/model/use-home-festivals.ts` at line 39, The API response uses upcomingFestivalData.dday while your app types AllFestivalItem and UpcomingFestivalItem expect dDay; update the mapping in use-home-festivals.ts so you explicitly convert the response field to the app shape (e.g., set dDay: upcomingFestivalData.dday when constructing AllFestivalItem/UpcomingFestivalItem) and update or add a comment explaining the normalization; alternatively, if you prefer to change the API type, rename UpcomingFestivalResponse.dday to dDay and ensure all places that consume UpcomingFestivalResponse use the same camelCase field to keep types consistent.
♻️ Duplicate comments (2)
apps/audience/src/widgets/home/components/festival-section/festival-section.css.ts (1)
15-15: 🧹 Nitpick | 🔵 TrivialSuggest: 디자인 토큰(vars)을 사용해 하드코딩된 spacing 값을 대체하는 것을 고려해 주세요
padding: '2rem',gap: '1rem'이 리터럴로 선언되어 있어요. 프로젝트의 spacing 토큰이 있다면 활용하시는 것이 일관성 유지에 도움이 될 거예요.As per coding guidelines, "디자인 토큰(vars)을 직접 사용하고 있는지 확인해요"
Also applies to: 28-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/widgets/home/components/festival-section/festival-section.css.ts` at line 15, Replace hardcoded spacing literals in festival-section styles with the project's design tokens: swap padding: '2rem' and gap: '1rem' to use the spacing vars (e.g., vars.space.* or the project's spacing token object) so spacing is consistent; update the style object in festival-section.css.ts where the keys padding and gap are defined to reference the appropriate token names and ensure imports include the vars/token module.apps/audience/src/widgets/home/components/festival-section/festival-section.tsx (1)
37-39: 🧹 Nitpick | 🔵 TrivialSuggest:
handleMoveToFestival을 커링 형태로 개선하면 인라인 화살표 함수를 줄일 수 있어요Line 70의
onClick={() => handleMoveToFestival(festival.festivalId)}는map반복마다 새 클로저를 생성해요. 목록이 작아 실질적 성능 영향은 적지만, 아래처럼 커링 형태로 정리하면map내부를 간결하게 만들 수 있어요.♻️ Proposed refactor
- const handleMoveToFestival = (festivalId: number) => { - navigate(ROUTE_PATH.NOTICE_LIST.replace(':eventId', String(festivalId))); - }; + const handleMoveToFestival = (festivalId: number) => () => { + navigate(ROUTE_PATH.NOTICE_LIST.replace(':eventId', String(festivalId))); + };- onClick={() => handleMoveToFestival(festival.festivalId)} + onClick={handleMoveToFestival(festival.festivalId)}Also applies to: 70-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/widgets/home/components/festival-section/festival-section.tsx` around lines 37 - 39, The current handleMoveToFestival creates a navigation closure per item when used as onClick={() => handleMoveToFestival(festival.festivalId)}; change handleMoveToFestival to a curried handler that returns an event callback (e.g., handleMoveToFestival = (festivalId: number) => () => { navigate(ROUTE_PATH.NOTICE_LIST.replace(':eventId', String(festivalId))) }) and update usages in the festival-section component to onClick={handleMoveToFestival(festival.festivalId)} to avoid creating inline arrow functions inside map.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/audience/src/entities/notice/model/query-options.ts`:
- Around line 16-21: The BANNER query's queryFn (in the queryOptions for
USERS_QUERY_KEY.FESTIVAL_BANNER) currently doesn't accept or forward an
AbortSignal, unlike LIST; change the queryFn to accept the standard queryFn
param (e.g. ({ signal })) and pass that signal into getFestivalBanner so
requests can be cancelled (e.g. call getFestivalBanner with the festivalId and
the signal in its options). If getFestivalBanner does not yet accept an options
object with signal, update getFestivalBanner's signature to accept an options
param ({ signal }) and use it for fetch/request cancellation.
In `@apps/audience/src/features/mypage/apis/query.ts`:
- Line 8: The import of ViewedFestivals is used only as a type and should be
changed to a type-only import to remove runtime dependency; update the import
statement that references ViewedFestivals to use "import type" (so the
ViewedFestivals type is imported as type-only) and ensure any usages remain type
positions only (e.g., in function signatures, type aliases, or generics) so the
compiler emits no runtime import for ViewedFestivals.
In `@apps/audience/src/shared/hooks/use-festival-notification.ts`:
- Line 3: The hook use-festival-notification in `@shared/hooks` currently imports
postNotificationsSubscribe from `@entities/notice/api/notice` which violates FSD
(shared should not depend on entities); either move the API function
postNotificationsSubscribe into a shared API module (e.g., `@shared/apis`) and
update the import in use-festival-notification, or move the hook
use-festival-notification into the entities domain (e.g.,
`@entities/notice/hooks`) and change its import path accordingly; update any
affected imports and tests to reference the new location and ensure
build/exports are adjusted so postNotificationsSubscribe and
use-festival-notification resolve correctly.
In
`@apps/audience/src/widgets/home/components/festival-section/festival-section.css.ts`:
- Around line 3-8: The CSS property order in the exported style named "section"
is incorrect: move Display & Layout properties before Box Model properties to
follow Mozilla-style ordering; specifically reorder the keys in the section
style so display and flexDirection come first, then flex (box model), and
finally minHeight (content/visual), preserving the same values.
In
`@apps/audience/src/widgets/home/components/festival-section/festival-section.tsx`:
- Around line 55-57: The wrapper element in the FestivalSection component uses a
non-semantic <div className={styles.section}> but this block represents an
independent section; replace the div with a semantic <section> element
(preserving the className={styles.section}) in festival-section.tsx so
HomeFestivalTabs and its props (selectedTab, onTabChange) remain unchanged and
styling continues to apply via styles.section to improve accessibility and
document semantics.
---
Outside diff comments:
In
`@apps/audience/src/features/usecase/toggle-wishlist/use-toggle-wishlist-mutation.ts`:
- Around line 49-59: The invalidate logic uses full queryKey objects
(FESTIVAL_QUERY_OPTIONS.ALL_FESTIVALS().queryKey and
.PLANNED_FESTIVALS().queryKey) which include pagination params and may miss
other cached variants; update the onSettled handler to call
queryClient.invalidateQueries with the base key array (prefix) instead of the
full key so all related pages are invalidated — replace the two
FESTIVAL_QUERY_OPTIONS.*.queryKey usages with their corresponding base key
arrays (e.g., the shared base key like USERS_QUERY_KEY.HOME_FESTIVALS_ALL or the
base part of FESTIVAL_QUERY_OPTIONS.*), keeping the existing invalidate for the
local queryKey variable and still using queryClient.invalidateQueries to perform
the invalidation.
In `@apps/audience/src/pages/home/model/use-home-festivals.ts`:
- Line 39: The API response uses upcomingFestivalData.dday while your app types
AllFestivalItem and UpcomingFestivalItem expect dDay; update the mapping in
use-home-festivals.ts so you explicitly convert the response field to the app
shape (e.g., set dDay: upcomingFestivalData.dday when constructing
AllFestivalItem/UpcomingFestivalItem) and update or add a comment explaining the
normalization; alternatively, if you prefer to change the API type, rename
UpcomingFestivalResponse.dday to dDay and ensure all places that consume
UpcomingFestivalResponse use the same camelCase field to keep types consistent.
In `@apps/audience/src/shared/types/home-response.ts`:
- Around line 41-49: The two interfaces use inconsistent casing for the same
field: UpcomingFestivalResponse declares dday while AllFestivalItem declares
dDay; confirm the actual API response field casing and make them consistent
(either rename UpcomingFestivalResponse.dday to dDay or change
AllFestivalItem.dDay to dday) and update any related
mappers/serializers/deserializers and usages of UpcomingFestivalResponse or
AllFestivalItem to match the chosen name (ensure exported types, JSON parsing,
and tests reflect the change).
- Around line 22-29: PaginationResponse is duplicated across modules; extract
the PaginationResponse interface into a single shared module (a dedicated
pagination/type module), export it from there, then remove the local interface
declarations and import PaginationResponse where currently defined (including
the other module that had the duplicate) so both files reference the one
exported type; update any exports that previously re-exported the local
interface to forward the shared export.
In `@apps/audience/src/shared/types/viewed-festival.ts`:
- Around line 10-17: The Pagination interface defined here duplicates
PaginationResponse elsewhere; extract a single shared type (e.g., export
interface Pagination { currentPage:number; totalPages:number;
totalElements:number; size:number; hasNext:boolean; hasPrevious:boolean }) into
a common types module (e.g., a shared/types index) and replace local definitions
with an import of that shared Pagination in files that currently declare
Pagination or PaginationResponse (including the viewed-festival.ts type and any
other duplicated files), update exports/imports accordingly, and remove the
duplicated definitions so all code uses the single shared Pagination symbol.
---
Duplicate comments:
In
`@apps/audience/src/widgets/home/components/festival-section/festival-section.css.ts`:
- Line 15: Replace hardcoded spacing literals in festival-section styles with
the project's design tokens: swap padding: '2rem' and gap: '1rem' to use the
spacing vars (e.g., vars.space.* or the project's spacing token object) so
spacing is consistent; update the style object in festival-section.css.ts where
the keys padding and gap are defined to reference the appropriate token names
and ensure imports include the vars/token module.
In
`@apps/audience/src/widgets/home/components/festival-section/festival-section.tsx`:
- Around line 37-39: The current handleMoveToFestival creates a navigation
closure per item when used as onClick={() =>
handleMoveToFestival(festival.festivalId)}; change handleMoveToFestival to a
curried handler that returns an event callback (e.g., handleMoveToFestival =
(festivalId: number) => () => {
navigate(ROUTE_PATH.NOTICE_LIST.replace(':eventId', String(festivalId))) }) and
update usages in the festival-section component to
onClick={handleMoveToFestival(festival.festivalId)} to avoid creating inline
arrow functions inside map.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (20)
apps/audience/src/entities/.gitkeepapps/audience/src/entities/festival/api/festival.tsapps/audience/src/entities/festival/model/query-options.tsapps/audience/src/entities/notice/api/notice.tsapps/audience/src/entities/notice/model/query-options.tsapps/audience/src/features/mypage/apis/query.tsapps/audience/src/features/usecase/toggle-wishlist/use-toggle-wishlist-mutation.tsapps/audience/src/pages/home/home.tsxapps/audience/src/pages/home/model/use-home-festivals.tsapps/audience/src/pages/notice-list/notice-list.tsxapps/audience/src/shared/apis/festival.tsapps/audience/src/shared/hooks/use-festival-notification.tsapps/audience/src/shared/types/home-response.tsapps/audience/src/shared/types/viewed-festival.tsapps/audience/src/widgets/home/components/festival-card/festival-card.tsxapps/audience/src/widgets/home/components/festival-section/festival-section.css.tsapps/audience/src/widgets/home/components/festival-section/festival-section.tsxapps/audience/src/widgets/home/components/flag-button/flag-button.css.tsapps/audience/src/widgets/home/components/flag-button/flag-button.tsxpackages/ads-ui/src/components/index.ts
💤 Files with no reviewable changes (3)
- apps/audience/src/shared/apis/festival.ts
- apps/audience/src/entities/notice/api/notice.ts
- packages/ads-ui/src/components/index.ts
Comment on lines
+16
to
+21
| BANNER: (festivalId: number) => | ||
| queryOptions({ | ||
| queryKey: USERS_QUERY_KEY.FESTIVAL_BANNER(festivalId), | ||
| queryFn: () => getFestivalBanner(festivalId), | ||
| enabled: Number.isFinite(festivalId), | ||
| }), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Suggest: BANNER queryFn에도 signal을 전달하면 일관성이 높아져요.
LIST queryFn은 { signal }을 전달하여 요청 취소를 지원하지만, BANNER는 signal을 전달하지 않아요. 일관된 패턴을 위해 BANNER에도 signal을 전달하는 것을 고려해 주세요.
♻️ 수정 제안
BANNER: (festivalId: number) =>
queryOptions({
queryKey: USERS_QUERY_KEY.FESTIVAL_BANNER(festivalId),
- queryFn: () => getFestivalBanner(festivalId),
+ queryFn: ({ signal }) => getFestivalBanner(festivalId, { signal }),
enabled: Number.isFinite(festivalId),
}),참고: getFestivalBanner 함수가 options 파라미터를 지원하는지 확인이 필요해요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/audience/src/entities/notice/model/query-options.ts` around lines 16 -
21, The BANNER query's queryFn (in the queryOptions for
USERS_QUERY_KEY.FESTIVAL_BANNER) currently doesn't accept or forward an
AbortSignal, unlike LIST; change the queryFn to accept the standard queryFn
param (e.g. ({ signal })) and pass that signal into getFestivalBanner so
requests can be cancelled (e.g. call getFestivalBanner with the festivalId and
the signal in its options). If getFestivalBanner does not yet accept an options
object with signal, update getFestivalBanner's signature to accept an options
param ({ signal }) and use it for fetch/request cancellation.
| import { useMutation } from '@tanstack/react-query'; | ||
|
|
||
| import { postNotificationsSubscribe } from '@features/notice-list/apis/query'; | ||
| import { postNotificationsSubscribe } from '@entities/notice/api/notice'; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Suggest: Shared 레이어에서 Entity 레이어 의존이 아키텍처 위반일 수 있어요.
현재 @shared/hooks 파일이 @entities/notice/api/notice를 import하고 있어요. FSD 아키텍처에서 Shared 레이어는 상위 레이어(entities, features 등)에 의존하지 않아야 해요.
Retrieved learnings에 따르면 putWishList API는 공유 레이어에 위치해야 한다고 했는데, 동일한 원칙이 postNotificationsSubscribe에도 적용될 수 있어요. 이 API도 @shared/apis로 이동하거나, 이 훅 자체를 @entities/notice/hooks로 이동하는 것을 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/audience/src/shared/hooks/use-festival-notification.ts` at line 3, The
hook use-festival-notification in `@shared/hooks` currently imports
postNotificationsSubscribe from `@entities/notice/api/notice` which violates FSD
(shared should not depend on entities); either move the API function
postNotificationsSubscribe into a shared API module (e.g., `@shared/apis`) and
update the import in use-festival-notification, or move the hook
use-festival-notification into the entities domain (e.g.,
`@entities/notice/hooks`) and change its import path accordingly; update any
affected imports and tests to reference the new location and ensure
build/exports are adjusted so postNotificationsSubscribe and
use-festival-notification resolve correctly.
apps/audience/src/widgets/home/components/festival-section/festival-section.css.ts
Show resolved
Hide resolved
apps/audience/src/widgets/home/components/festival-section/festival-section.tsx
Show resolved
Hide resolved
Member
Author
|
리뷰해주신 내용들 확인하여 반영하였습니다! 아마 가장 주요한 이슈는 API 로직의 위치와 두 개 이상의 도메인에서 사용되는 유즈케이스 관련 커스텀 훅의 폴더 구조인 것 같습니다. 기존 저희 구조를 보면 이를 해결하고자 처음에는 추가로 말씀해주신 것처럼 두 개 이상 도메인에서 활용되는 유즈케이스 또한 다만 아직 이번 PR 범위 외 API 로직은 |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/audience/src/widgets/home/components/festival-section/festival-section.tsx (1)
37-39: 🧹 Nitpick | 🔵 TrivialSuggest:
handleMoveToFestival을 커링 형태로 개선하면 인라인 화살표 함수를 줄일 수 있어요.Line 70의
onClick={() => handleMoveToFestival(festival.festivalId)}는map반복마다 새 클로저를 생성해요. 목록이 작아 실질적 성능 영향은 적지만, 아래처럼 커링 형태로 정리하면map내부를 간결하게 만들 수 있어요.♻️ 제안 코드
- const handleMoveToFestival = (festivalId: number) => { - navigate(ROUTE_PATH.NOTICE_LIST.replace(':eventId', String(festivalId))); - }; + const handleMoveToFestival = (festivalId: number) => () => { + navigate(ROUTE_PATH.NOTICE_LIST.replace(':eventId', String(festivalId))); + };- onClick={() => handleMoveToFestival(festival.festivalId)} + onClick={handleMoveToFestival(festival.festivalId)}Also applies to: 70-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/audience/src/widgets/home/components/festival-section/festival-section.tsx` around lines 37 - 39, The current handleMoveToFestival creates a new closure each time it's invoked from the map via onClick={() => handleMoveToFestival(festival.festivalId)}; refactor handleMoveToFestival into a curried form that takes festivalId and returns an event handler function so you can pass it directly to onClick (e.g., onClick={handleMoveToFestival(festival.festivalId)}), update the function signature and body accordingly to call navigate(ROUTE_PATH.NOTICE_LIST.replace(':eventId', String(festivalId))) inside the returned handler, and replace all inline arrow onClick usages that call handleMoveToFestival to use the curried form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/audience/src/widgets/home/components/festival-section/festival-section.tsx`:
- Around line 37-39: The current handleMoveToFestival creates a new closure each
time it's invoked from the map via onClick={() =>
handleMoveToFestival(festival.festivalId)}; refactor handleMoveToFestival into a
curried form that takes festivalId and returns an event handler function so you
can pass it directly to onClick (e.g.,
onClick={handleMoveToFestival(festival.festivalId)}), update the function
signature and body accordingly to call
navigate(ROUTE_PATH.NOTICE_LIST.replace(':eventId', String(festivalId))) inside
the returned handler, and replace all inline arrow onClick usages that call
handleMoveToFestival to use the curried form.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
apps/audience/src/features/mypage/apis/query.tsapps/audience/src/features/usecase/toggle-wishlist/use-toggle-wishlist-mutation.tsapps/audience/src/widgets/home/components/festival-section/festival-section.css.tsapps/audience/src/widgets/home/components/festival-section/festival-section.tsx
Sohyunnnn
reviewed
Mar 1, 2026
Contributor
Sohyunnnn
left a comment
Sohyunnnn
left a comment
There was a problem hiding this comment.
수고하셨습니다! 전체적으로 훨씬 깔끔해진 거 같습니다 👍
코멘트 한 번 확인 부탁드려요!!!
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
3 participants
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.
📌 Summary
관람객 홈 뷰에서 사용되는
HomeFestivalCard관련 위젯들의 구조 및 상태 관리 일관성을 개선했습니다.📚 Tasks
HomeFestivalCardprops 단순화 및 이벤트 관련 로직을 위한 불필요 태그 제거HomeFestivalCard내부로 이동HomeFestivalList컴포넌트 제거 및HomeFestivalSection으로 병합useHomeFestivals내부useState,useEffect활용 서버 데이터 수동 동기화 및 배열 조작 함수 삭제🔍 Describe
리팩토링 배경
기존 홈 화면의 공연 리스트를 렌더링하는 과정에서 불필요한 Props Drilling(
HomeFestivalSection->HomeFestivalList->HomeFestivalCard)이 발생하고 있었어요. 관람 예정 토글과 관련한 도메인 로직과 핸들러 함수가 최상위 훅(useHomeFestivals)에 집중되어 있어, 이를 계속 Props로 전달하며 하위 컴포넌트들이 부모에게 과도하게 의존하고 있어요.뿐만 아니라, 이
useHomeFestivals커스텀 훅도 Tanstack Query로 가져온 서버 데이터를 로컬 상태로 복사한 뒤useEffect로 수동 동기화하는 구조적 문제가 있었어요. 추가로 해당 페이지에서는 공연 목록을 배열로 관리하고 있었는데, 낙관적 업데이트를 위한 매우 길고 복잡한 코드가 훅 내부에 있어 유지보수성이 저하된 상태였어요.리팩토링 과정
관람 예정 도메인 로직 내재화 및 Props 단순화
가장 기본이 되는 위젯인
HomeFestivalCard가 부모로부터 props로 토글 함수를 전달 받는 대신, 이전 PR에서 구현해둔useToggleWishListMutation훅을 활용하여 내부에서 직접 관람 예정 설정을 할 수 있도록 변경했어요. 동시에 넘겨 받는 Props를festival객체 하나와onClick함수 하나로 단순화했어요.Props Drilling 해소
단순 데이터, 이벤트 핸들러를 전달하는 역할만 수행하며 불필요한 계층을 만들었던
HomeFestivalList컴포넌트를 삭제했어요. 리스트 렌더링 로직은HomeFestivalSection으로 통합하며 계층을 한 단계 줄였고, 동시에 탭에 따라 분기되어 2번 작성되는map순회 로직을targetFestivals라는 변수에서 미리 구분하여 코드 중복을 제거했어요.상태 관리 단일화
useHomeFestivals훅 내부에 있던useState,useEffect를 활용한 서버 데이터를 로컬 데이터에 복사하고 수동으로 동기화하는 로직과 관람 예정 설정 시 강제로 배열을 조작하는 함수, 약 100줄에 달하는 길고 복잡한 로직을 전면 삭제했어요. 그 대신 Tanstack Query의 캐시 데이터를 직접 참조하도록 하고,mutation완료 시 기존 배너 관련 쿼리 외에 홈 화면 관련 쿼리를 일괄invalidate처리하는 로직을 추가하여 전역에서 관련 데이터 동기화를 보장하도록 했어요.👀 To Reviewer
기존
features/notice-list에 정의되어 있던useToggleWishListMutation이 Home에서도 공통으로 사용됨에 따라, 특정 피처에 종속되지 않도록 Shared 레이어로 이동했어요.이 과정에서 단순히 훅만 이동하면, 하위 레이어(
shared)가 상위 레이어(features)에 존재하는 쿼리 옵션들을 참조하게 돼요. 이는 FSD 구조에 적합하지 않다고 판단하여 쿼리 옵션 관련 파일들도shared/apis경로로 옮겼는데 괜찮은지 한번씩만 확인 부탁드려요! 폴더 구조 문제 관련하여 더 나은 방안이 있다면 알려주세요...🙇♂️📸 Screenshot
2026-03-02.12.54.46.mov