Refactor: Context API로 BottomSheet onClose props drilling 제거 - #200
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 0 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
전체 요약개요BottomSheet의 변경 사항
코드 리뷰 노력 추정🎯 3 (보통) | ⏱️ ~20분 시 🐰
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook 배포 완료📚 Storybook: https://6960ec095e9394ddeaa0f9f3-glqgbntsra.chromatic.com/
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/web/src/shared/ui/bottomSheet/BottomSheetContext.tsx`:
- Line 1: This file uses React client-only hooks createContext and useContext
but lacks the Next.js Client Component directive; add the string directive "use
client" as the very first line of the BottomSheetContext module (before any
imports) so createContext/useContext are executed on the client.
In `@apps/web/src/shared/ui/bottomSheet/templates/BaseBottomSheetTemplate.tsx`:
- Line 15: BaseBottomSheetTemplate currently always calls
useBottomSheetContext(), causing errors when rendering the header with
type='add' where onClose isn't needed; move the hook call out of the top-level
render and into the close-button-specific component (e.g., the CloseButton or
headerCloseHandler) so useBottomSheetContext() is only invoked when a closable
header is rendered and onClose is required; update BaseBottomSheetTemplate to
render the close-button component conditionally for non-'add' types and remove
any unconditional useBottomSheetContext() usage, ensuring onClose is passed down
from the hook into that close-button component only when present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a5190b07-58a8-4f4e-a768-7af3bed2b43b
📒 Files selected for processing (17)
apps/web/src/features/datePickerModal/ui/DatePickerBottomSheet.tsxapps/web/src/features/datePickerModal/ui/DatePickerFeature.tsxapps/web/src/features/expense/ui/ExpenseEditBottomSheet.tsxapps/web/src/features/expense/ui/expenseBottomSheet/DatePickerBottomSheet.stories.tsxapps/web/src/features/expense/ui/expenseBottomSheet/DatePickerBottomSheet.tsxapps/web/src/features/expense/ui/expenseBottomSheet/ExpenseFormBottomSheet.stories.tsxapps/web/src/features/expense/ui/expenseBottomSheet/ExpenseFormBottomSheet.tsxapps/web/src/features/expense/ui/expenseBottomSheet/IconPickerBottomSheet.stories.tsxapps/web/src/features/expense/ui/expenseBottomSheet/IconPickerBottomSheet.tsxapps/web/src/features/expense/ui/steps/AddCategoryStep/AddCategoryStep.tsxapps/web/src/features/expense/ui/steps/AmountDateStep/AmountDateStep.tsxapps/web/src/features/expense/ui/steps/AmountDateStep/CalendarBottomSheet.tsxapps/web/src/shared/ui/bottomSheet/BottomSheet.tsxapps/web/src/shared/ui/bottomSheet/BottomSheetContext.tsxapps/web/src/shared/ui/bottomSheet/index.tsapps/web/src/shared/ui/bottomSheet/templates/BaseBottomSheetTemplate.tsxapps/web/src/widgets/addCategory/ui/AddCategory.tsx
💤 Files with no reviewable changes (8)
- apps/web/src/widgets/addCategory/ui/AddCategory.tsx
- apps/web/src/features/expense/ui/steps/AmountDateStep/AmountDateStep.tsx
- apps/web/src/features/datePickerModal/ui/DatePickerFeature.tsx
- apps/web/src/features/expense/ui/expenseBottomSheet/IconPickerBottomSheet.stories.tsx
- apps/web/src/features/expense/ui/ExpenseEditBottomSheet.tsx
- apps/web/src/features/expense/ui/steps/AddCategoryStep/AddCategoryStep.tsx
- apps/web/src/features/expense/ui/expenseBottomSheet/DatePickerBottomSheet.stories.tsx
- apps/web/src/features/expense/ui/expenseBottomSheet/ExpenseFormBottomSheet.stories.tsx
| import { vars } from '../../theme.css'; | ||
| import { IcClear, IcPlusSimple } from 'public/icons'; | ||
| import { Button } from '../../button'; | ||
| import { useBottomSheetContext } from '../BottomSheetContext'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: BaseBottomSheetTemplate.Header의 add 타입 사용처를 확인합니다.
rg -nP -C3 "<BaseBottomSheetTemplate\\.Header[^>]*type=['\"]add['\"]" --glob '*.tsx'Repository: Central-MakeUs/Monit_Front
Length of output: 929
🏁 Script executed:
#!/bin/bash
# 1. BaseBottomSheetTemplate.tsx 파일 크기 확인
wc -l apps/web/src/shared/ui/bottomSheet/templates/BaseBottomSheetTemplate.tsx
# 2. 파일 내용 읽기
cat -n apps/web/src/shared/ui/bottomSheet/templates/BaseBottomSheetTemplate.tsxRepository: Central-MakeUs/Monit_Front
Length of output: 3438
🏁 Script executed:
#!/bin/bash
# 3. Header 컴포넌트의 직접 import 사용 여부 검색
rg -n "import.*Header.*from.*BottomSheetTemplate|from.*BaseBottomSheetTemplate.*Header" --glob '*.tsx'Repository: Central-MakeUs/Monit_Front
Length of output: 52
🏁 Script executed:
#!/bin/bash
# 4. 전체 type='add' 사용처 재검색 (더 광범위)
rg -n "type=['\"]add['\"]" apps/web/src/features --glob '*.tsx' | grep -i "header\|template"Repository: Central-MakeUs/Monit_Front
Length of output: 239
type='add' 헤더에서 불필요한 컨텍스트 의존성 제거 필요
Line 43-44에서 useBottomSheetContext()를 항상 호출하므로, onClose가 필요 없는 type='add' 헤더도 컨텍스트가 존재하지 않으면 에러가 발생합니다. 현재 사용처는 모두 BaseBottomSheetTemplate 내부에서 렌더링되어 안전하지만, 훅 호출을 onClose가 필요한 close 버튼 전용 컴포넌트로 이동하면 더 안전한 구조가 됩니다.
🐛 제안 수정
import { Button } from '../../button';
import { useBottomSheetContext } from '../BottomSheetContext';
@@
-const BottomSheetHeader = ({ text, type = 'close', onClickAddBtn }: BottomSheetHeaderProps) => {
+const BottomSheetCloseButton = () => {
const { onClose } = useBottomSheetContext();
+
+ return <IcClear className={headerIcon} color={vars.color.icon.subtle} onClick={onClose} />;
+};
+
+const BottomSheetHeader = ({ text, type = 'close', onClickAddBtn }: BottomSheetHeaderProps) => {
return (
<div className={bottomSheetHeaderWrapper}>
@@
{type === 'close' ? (
- <IcClear className={headerIcon} color={vars.color.icon.subtle} onClick={onClose} />
+ <BottomSheetCloseButton />
) : (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/shared/ui/bottomSheet/templates/BaseBottomSheetTemplate.tsx` at
line 15, BaseBottomSheetTemplate currently always calls useBottomSheetContext(),
causing errors when rendering the header with type='add' where onClose isn't
needed; move the hook call out of the top-level render and into the
close-button-specific component (e.g., the CloseButton or headerCloseHandler) so
useBottomSheetContext() is only invoked when a closable header is rendered and
onClose is required; update BaseBottomSheetTemplate to render the close-button
component conditionally for non-'add' types and remove any unconditional
useBottomSheetContext() usage, ensuring onClose is passed down from the hook
into that close-button component only when present.
|
🎉 구현한 기능 Preview: https://nitrogen-front-2zicl2jix-ssilver01s-projects.vercel.app |
hyun907
left a comment
There was a problem hiding this comment.
Context 경계와 데이터 소유권이 잘 보이는 리팩토링입니다! props-drilling도 깔끔하게 정리되고, 잘못 쓰면 throw로 바로 드러나는 점도 좋습니다! Good!
📝 PR 유형
🔔 관련된 이슈 넘버
✅ 작업 목록
문제 상황: Compound Component 패턴 사용 시 props drilling이 심화되어 유지보수성 저하 우려
BottomSheet가 이미onClose를 갖고 있음에도, 그 안에 렌더되는 컴포넌트들도 독립적으로onClose를 prop으로 받아서 아래로 전달해야 했습니다.변경 내용
1.
BottomSheetContext생성2.
BottomSheet가 Context 제공3.
Header가 Context에서 직접 읽음개선된 구조
onClose를 소유한BottomSheet와 실제로 사용하는Header사이의 중간 컴포넌트들이onClose를 전혀 알 필요가 없어졌습니다. 앞으로 새 템플릿을 만들 때도 props에onClose를 선언하지 않아도 됩니다.Summary by CodeRabbit
릴리스 노트