Conversation
INSANE-P
left a comment
INSANE-P
left a comment
There was a problem hiding this comment.
수고 많으셨습니다.
앞으로 일정 변경사항을 훨씬 편하게 관리할 수 있을 것 같습니다. 버튼을 서비스 기간 별로 다르게 매핑한 방식도 좋은 것 같습니다.
간단한 리뷰 몇 가지 남겼으니 확인해주시면 감사하겠습니다.
| }; | ||
|
|
||
| //preSeat 데이터 업데이트 시 preSeatOpenDate를 변경해주어야합니다. | ||
| export const PRESEAT_OPEN_DATE = new Date('2026-02-29T00:00:00'); |
There was a problem hiding this comment.
2026년 2월 29일은 존재하지 않는 날짜라서 수정하면 좋을 것같습니다.
| return await fetchJsonOnPublic<IPreRealSeatsResponse>('/pre-seats.json?date=20260215'); | ||
| //preSeat 데이터 업데이트 시 preSeatOpenDate를 변경해주어야합니다. | ||
| const { preSeat } = useManagePeriod(); | ||
| return await fetchJsonOnPublic<IPreRealSeatsResponse>(`/pre-seats.json?date=${preSeat.preSeatOpenDate}`); |
There was a problem hiding this comment.
| return await fetchJsonOnPublic<IPreRealSeatsResponse>(`/pre-seats.json?date=${preSeat.preSeatOpenDate}`); | |
| return await fetchJsonOnPublic<IPreRealSeatsResponse>(`/pre-seats.json?date=${PRESEAT_OPEN_DATE.getTime()}`); |
preSeatOpenDate 상수 값을 변경시켜야 되는 요구사항이라면 내부에서 useManagePeriod 훅을 실행하지 않고 preSeatOpenDate 상수를 직접 적용해도 괜찮지 않을까요?
훅을 사용하지 않아도 동일하게 처리 가능할 것 같아서 의견 드립니다
There was a problem hiding this comment.
좋은 의견 주셔서 감사합니다!
아래 현우님의 코멘트와 겹치는 수정 내용이 될 것 같아서, 참고해서 설명드리겠습니다!
manageSchedule, useManagePeriod의 역할
우선, manageSchedule은 날짜를 관리하는 데이터를 모아두고, UI나 다른 도메인에서 useManagePeriod 훅을 통해서만 데이터에 접근할 수 있는 방향성을 생각해보았습니다.
이런 방향성을 생각한 이유는, 추후에 해당 기능이 API로 변경될 상황 과 각각의 역할(날짜 데이터 관리, 데이터 가공 훅)을 염두에 두었기 때문입니다. PRESEAT_OPEN_DATE는 추후 구조가 변경될 수 있는 데이터이기도 하고,
앞으로의 변경에 대한 최소화를 위해 useManagePeriod를 통해서만 접근하도록 구현하였습니다. 또한 모든 값들의 출처가 useManagePeriod를 가리키고 있을 때, 데이터 변경이나 훅 변경에 대한 대응을 빠르게 할 수 있을 것이라 생각합니다.
수정 방향성에 대해
찬빈님과 현우님께서 주신 코멘트를 각각 반영해 API 전환을 고려하여 manageSchedule의 데이터를 fetch하는 함수를 별도로 추가하고, manageSchedule에서의 직접 import는 금지하는 방향으로 컨벤션을 정리하겠습니다!!
그리고 CACHE_VERSION 값을 상수로 두어 혼란을 드린 것 같습니다. 이 부분도 같이 수정하겠습니다!
| const setIsSearchOpen = useAlarmModalStore(state => state.setIsSearchOpen); | ||
| const isMobile = useMobile(); | ||
| const { isPreSeatAvailable } = usePreSeatGate(); | ||
| const { shouldPreparePreSeat: isPreSeatAvailable } = useManagePeriod(); |
There was a problem hiding this comment.
| const { shouldPreparePreSeat: isPreSeatAvailable } = useManagePeriod(); | |
| const { preSeat: { shouldPreparePreSeat: isPreSeatAvailable } } = useManagePeriod(); |
useManagePeriod가 다음과 같은 객체 구조를 반환하고 있는데
return {
period: {
mode: currentMode,
period: currentPeriod,
displayPeriod: CURRENT_PERIOD.displayPeriod,
},
mainPageRouter: {
mainPageActions: mainActions,
},
preSeat: {
shouldPreparePreSeat,
preSeatCloseDate,
liveStartTime,
preSeatOpenDate: PRESEAT_OPEN_DATE,
},
basket: {
basketOpenDate: BASKET_OPEN_DATE,
},
};
shouldPreparePreSeat가 최상위 객체가 아닌 preSeat내부에 존재하는데 최상위 객체로 가정하고 파싱하고 있어서 오류가 발생하고 있는 것 같습니다.
There was a problem hiding this comment.
헉 오류가 발생하고 있었군요🫢 감사합니다 수정하겠습니다!
| function MainBanner() { | ||
| const { data } = useServiceSemester(); | ||
| const { mainPageRouter: {mainPageActions}, period :{displayPeriod} } = useManagePeriod(); | ||
|
|
There was a problem hiding this comment.
| const [mainAction, subAction] = mainPageActions; | |
mainPageActions[0], mainPageActions[1]과 같이 인덱스 기반으로 접근하면 각 버튼의 역할을 파악하기 어려울 수 있습니다. 구조 분해 할당을 사용하면 가독성이 높아질 수 있을 것 같습니다.
There was a problem hiding this comment.
오 꼼꼼하게 리뷰해주셔서 감사합니다! 반영하겠습니다.
hyunwoo0081
left a comment
hyunwoo0081
left a comment
There was a problem hiding this comment.
수정해야하는 정보들을 한 곳에 모아두어서 좋네요.
추후에 달력 업데이트와 동시에 진행되면 확실히 유용할 것 같습니다!
| end: '02월 13일(금)', | ||
| }, | ||
| }; | ||
|
|
There was a problem hiding this comment.
예전에 달력 업데이트 부분이 이 내용이었던 것 같아요.
이 상수 부분도, API 로 변경될 가능성이 있을 것 같아요. 직접 상수로 export 하기보다는 함수로 대신 export 해두면 좋을 것 같아요,
There was a problem hiding this comment.
이 파일이 APP 의 설정파일 역할을 하게 될 것 같습니다.
app 폴더로 이동해보는건 어떻게 생각하시나요?
추후 달력 기능이 업데이트 되면, 여기 두는게 좋을 수도 있을 것 같아요.
There was a problem hiding this comment.
예전에 달력 업데이트 부분이 이 내용이었던 것 같아요.
이 상수 부분도, API 로 변경될 가능성이 있을 것 같아요. 직접 상수로 export 하기보다는 함수로 대신 export 해두면 좋을 것 같아요,
넵! 저도 해당 방향성으로 데이터 처럼 관리하는게 좋을 거 같습니다. 좋은 의견 감사합니다!!
이 파일이 APP 의 설정파일 역할을 하게 될 것 같습니다.
app 폴더로 이동해보는건 어떻게 생각하시나요? 추후 달력 기능이 업데이트 되면, 여기 두는게 좋을 수도 있을 것 같아요.
앗 그러네요! 혹시.., 제가 이해한 방향성이 맞는지 모르겠어서,
특정 도메인과 연관 있다기 보다는 전체 서비스 관리와 연관성이 있어서 APP 설정 파일에 두는 것을 말씀하신 게 맞을까요?
맞다면 APP폴더에 두는 것도 좋을 것 같습니다!
다만, FSD 관점에서는 app의 파일을 하위 레이어 page, widget등에서 import하게 되는 방향성은 어색할 수 있을 것 같습니다.
| const hasPreSeats = wishes && 'seat' in wishes; | ||
| const { isPreSeatAvailable } = usePreSeatGate({ hasSeats: hasPreSeats }); | ||
| const { preSeat } = useManagePeriod(); | ||
| const isPreSeatAvailable = preSeat.shouldPreparePreSeat && Boolean(hasPreSeats); |
There was a problem hiding this comment.
파일을 확인해보니, hasPreSeats 가 boolean 으로써 사용되고 있습니다.
hasPreSeats = Boolean(...
변수에서 Boolean으로 바꾸긴 보다, hasPreSeats를 변경하는건 어떤가요?
There was a problem hiding this comment.
넵! 좋은 의견 감사합니다~! 수정하겠습니다.
|
3 participants
작업 내용
preSeat, baskets 없데이트 시 변경해야하는 코드가 응집되어 있지 않아 변경 사항을 놓치거나 코드상으로 찾아야하는 불편함이 존재했습니다.
PreSeat 업데이트
AS-IS TO-BEAS-IS
아래는 변경에 의존성이 있는 코드들입니다.
PRESEAT_CLOSE_DATE업데이트TO-BE
PRESEAT_CLOSE_DATE자동 업데이트PRESEAT_OPEN_DATE,BASKET_OPEN_DATE만 업데이트 하면 되도록하여 변경사항을 최소화합니다.🚧 앞으로 변경 사항이 있을 때 해야할 작업 🚧
우선, 간단히 앞으로 변경 사항을 적용할 때 해야할 작업을 정리해드리겠습니다.
학기 시작 전
CURRENT_PERIOD를 각 학기의 일정에 맞게 수정합니다.(현재는 2026_SPRING 기준데이터입니다.)학기 시작 후 변경사항 업데이트 시
PRESEAT_OPEN_DATE,BASKET_OPEN_DATE를 변경 날짜에 맞춰 수정합니다.구현 내용
변경 사항에 대한 의존성이 있는 데이터들을 정의하고, 구현한 내용입니다.
메인 배너
메인 배너의 바로가기 버튼을 각 서비스 기간별로 다르게 매핑합니다.
관련 훅
getMainActions에서 각 서비스에 맞는 라우팅을 반환합니다.CURRENT_PERIOD로 판별합니다.PreSeat 기간
preSeat 기간은 다음과 같이 정의하였습니다.
PRESEAT_CLOSE_DATE에 의존)캐시버스터
PRESEAT_OPEN_DATE를 참조하고 있습니다.변경 사항 및 리뷰 포인트
엄.. 일단 구현은 했는데, 맞는 방향성일지 확신이 들지는 않습니다. 더 좋은 방법이 있으면 의견 주세요!
엣지 케이스가 존재할 것 같아서, 발견하신다면 말씀해주시면 감사하겠습니다!