-
Notifications
You must be signed in to change notification settings - Fork 5
Feature/와인 등록하기 #90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/와인 등록하기 #90
Conversation
|
@Luganic is attempting to deploy a commit to the 626-ju's projects Team on Vercel. A member of the Team first needs to authorize it. |
src/stores/searchStore.ts
Outdated
| setSearchTerm: (term: string) => void; | ||
| }; | ||
|
|
||
| const useSearchStore = create<SearchState>((set) => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'와인 검색을 위한 검색어' 상태를 다루기 위한 스토어군요. 사용하는 곳을 봤을 때, 전역으로 선언할 필요는 없어보입니다. WineListCard컴포넌트가 prop으로 searchKeyword: string을 받아서 사용해도 충분해보입니다. 아직은 prop으로 전달하기 어려운, 멀리 떨어진 컴포넌트에서 사용하거나 props drilling이 필요할정도로 깊고 넓게 활용되는 상태가 아니니까요. (그럴 필요가 생기면 그때 수정해도 늦지않겠죠(YAGNI), 단순한 구현으로 커버할 수 있는 경우에는 단순하게 하는 편이 유지보수에 좋습니다
유지하신다면, 도메인과 기능적 의미가 들어간 네이밍 추천드립니다. useWineSearchKeywordStore
src/stores/wineAddStore.ts
Outdated
| addWine: (newWine: Wine) => void; | ||
| } | ||
|
|
||
| const useWineStore = create<WineStoreState>((set) => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
목록에 표시될 와인목록 상태를 다루기 위한 스토어로 보이네요
- 파일이름과 달라서 어색하니 일치시길 추천드려요
- 결국 (tanstack/query로써)
와인목록상태를 서버(캐시)상태로 관리하면서 조회/추가 모두 api연동으로 처리된다면. 이 훅은 제거될거같긴하네요
📦 Pull Request
📝 요약(Summary)
💬 공유사항 to 리뷰어
🗂️ 관련 이슈
📸 스크린샷
✅ 체크리스트