feat(time): 타임테이블 모달 api 연동 #211
Conversation
|
| getLectureList: 'api/v1/lecture/retrieve', | ||
| getMajorList: 'api/v1/lecture/retrieve/major', |
There was a problem hiding this comment.
get 요청 말고 다른 경우에도 동일한 엔드포인트를 사용해야 할 수도 있기 때문에 접두사를 빼고 LecturList로만 작성해도 괜찮을 것 같아요.
환경변수 설정 시에 /api가 있는 API_BASE_URL을 별도로 생성 후 사용하는 건 어떨까요? 엔드 포인트마다 api를 붙이지 않아도 되므로 편리할 것 같아서 제안드려요.
또, 엔드포인트 값은 변경되지 않을 상수이므로 TIMETABLE_ENDPOINT처럼 전체 대문자로 아름을 설정해도 좋을 것 같아요!
ex)LECTURE_LIST
There was a problem hiding this comment.
저는 개인적으로 앞에 END_POINT로 앞에 대문자 스네이크 케이스를 사용했다면, 객체 내부의 key에는 따로 대문자 스네이크 케이스를 사용하지 않아요.
아래 stackoverflow에 따르면 객체 속성 내부의 값은 변할 수 있기 때문에 lowercase로 사용될 수 있다. 라고 하는데요, 저는 getLectureList, getMajorList 등의 api 엔드포인트 또한 재설계를 통해 언제든지 바뀔 수 있는 부분이라고 생각하기때문에 상수 선언문인 END_POINT는 대문자 스네이크 케이스로 두되, 내부의 속성명은 소문자로 해도 무방하다고 생각해요. 또 해당 요소가 상수인지는 END_POINT라는 대문자 스네이크 케이스를 통해 충분히 유추 가능하구요
There was a problem hiding this comment.
저는 코드 내에 작성 됐을 때 모두 대문자 스네이크 케이스인게 좀 더 직관적으로 선언문 내부의 속성이 상수라는 사실을 알 수 있다고 생각해서 모두 대문자 스네이크 케이스로 사용하고 있어요. 처음 코드를 보는 사람이 봤을 때도 바로 상수라는 사실을 알아차릴 수 있을 것이라고 생각해요. 이번에 의견을 나눠보고 한 방법으로 통일시키면 어떨까요?
There was a problem hiding this comment.
아래 stackoverflow에 따르면 객체 속성 내부의 값은 변할 수 있기 때문에 lowercase로 사용될 수 있다.
말씀하신대로 객체 내부 값은 변경될 수 있어요, 하지만 이와 같이 Endpoint를 나타내는 상수 값은 as const를 통해서 readonly가 되어야 한다고 생각해요 (객체 내부 값또한 상수 처리), 상수 내부 값은 당연히 상수여야하고 값이 변경되면 안돼요. 이 때문에 상수 객체의 key 또한 대문자 스네이크 케이스를 사용하고 있어요. (내부 값 또한 상수 이기 때문)
상수 선언문인 END_POINT는 대문자 스네이크 케이스로 두되, 내부의 속성명은 소문자로 해도 무방하다고 생각해요. 또 해당 요소가 상수인지는 END_POINT라는 대문자 스네이크 케이스를 통해 충분히 유추 가능하구요
TIMETABLE_ENDPOINT 객체의 요소(값)들은 과연 상수일까요? 내부 값도 as const or readonly가 명시되지 않을 경우 상수가 아니에요(타입스크립트 컴파일타임 한정), 값이 재할당이 될 수 있고 변경될 수 있지요 여기서 상수 처리는 TIMETABLE_ENDPOINT의 객체예요. 따라서 요소는 END_POINT라는 대문자 스네이크 케이스를 통해 충분히 유추 가능하지만, 실제로는 상수가 아니므로 as const(readonly)처리를 해야한다고 생각해요. 이로 인해 명확하게 해야할 필요가 있으며, 내부 속성 또한 대문자 스테이크 케이스가 명확하다고 생각해요. 타입스크립트에서 컴파일타임에서도 상수에 대한 에러를 캐치할 수 있기도 합니다.
const NEW_TIMETABLE_ENDPOINT = {
GET_LECTURELIST: 'api/v1/lecture/retrieve', // is readonly
GET_MAJORLIST: 'api/v1/lecture/retrieve/major' // is readonly
} as const
| interface UseDebounceParams { | ||
| value: unknown; | ||
| delay: number; | ||
| } | ||
|
|
||
| export default function useDebounce({ value, delay }: UseDebounceParams) { | ||
| const [debouncedValue, setDebouncedValue] = useState<unknown>(value); |
There was a problem hiding this comment.
| interface UseDebounceParams { | |
| value: unknown; | |
| delay: number; | |
| } | |
| export default function useDebounce({ value, delay }: UseDebounceParams) { | |
| const [debouncedValue, setDebouncedValue] = useState<unknown>(value); | |
| interface UseDebounceParams<TValue> { | |
| value: TValue; | |
| delay: number; | |
| } | |
| export default function useDebounce<TValue>({ value, delay }: UseDebounceParams<TValue>) { | |
| const [debouncedValue, setDebouncedValue] = useState<TValue>(value); |
기존 코드의 리턴 값과 value의 타입은 unkown이에요, 제너릭을 활용하여 적절한 타입이 추론되도록 유도해보는 건 어떨까요?? (타입스크립트의 타입성이 보장되지 않기 때문)
There was a problem hiding this comment.
좋은 생각이네요! 제네릭을 적용하여 재설계해볼게요
| inputClassName="border-gray-400 px-1 px-1.5 rounded-md" | ||
| placeholder="강의명을 입력해주세요" | ||
| onChange={(event) => | ||
| handleLectureSearchInput(event.currentTarget.value) |
There was a problem hiding this comment.
| handleLectureSearchInput(event.currentTarget.value) | |
| handleLectureSearchChange(event.currentTarget.value) |
| handleLectureSearchInput(event.currentTarget.value) | |
| onChange(event.currentTarget.value) |
Input을 활용한 컴포넌트일 경우 input의 props를 그대로 따라가도 명확하고 이해하기 용이하다고 생각해요
Summary
강의 필터링을 위한 타임테이블 모달 컴포넌트에 api 연결을 완료했습니다.
Tasks
DAY_STATUS가period.ts에 있는건 부자연스럽다고 판단)useSuspenseQuery와Suspense를 사용하여 데이터 로딩시 fallback 처리를 진행했습니다.useDebounce유틸 훅을 만들어 input 입력 시 일어나는 데이터 패칭을 최적화했습니다.queryKey factory를 적용하여 쿼리 키를 관리하도록 하였습니다.pills-input컴포넌트를 직접 구현하여 전공 선택시의 편의성을 높였습니다.ETC
Modal과 관련된 컴포넌트들의 일부가 중복된 로직들이 존재하는데, 이는 추후 재설계하여 공용 DS 파트로 분리할 예정입니다.Suspensive라이브러리를 사용하여ErrorBoundary와Suspsense컴포넌트를 적용할 예정입니다.Screenshot