[임예지] sprint11 #141
[임예지] sprint11 #141
Conversation
kimjong95
left a comment
kimjong95
left a comment
There was a problem hiding this comment.
전체적으로 typescript를 사용하도록 마이그레이션 해주신것 같은데, 타입을 사용은 하고있지만 의미없이 사용되는 부분이 꽤많습니다.
타입을 정의하는 이유중 하나는 해당 타입을 사용해서 런타임에 발생할 수 있는 문제를 컴파일타임에 확인하고 해결하는데 있습니다. 하지만 현재는 타입을 각파일별로 선언하여 사용하고있고 제대로 활용하고있는 모습을 보기 어렵습니다.
추가적으로는 컴포넌트와 로직의 분리도 필요해보입니다. 모듈화라는 관점을 가져볼 때 우리가 구성한 컴포넌트는 하나의 함수로 이루어져있고, 단일책임원칙(SRP)에따르면 해당함수는 하나의 책임을 가져가는형태로 구성해주는게 좋습니다. 하지만 현재 구성된 컴포넌트들은 너무많은 일을 하고있고, 그에따른 사이드이펙트도 관리하기 어려울것 같습니다.
코드를 관심사에 맞게 분리하고 타입을통해 안정성을 가져가며 조합할수 있다면 더 안정적이고 가독성이좋은 코드가 될 것 같습니다
| import Footer from "@/components/footer"; | ||
| import FormatDate from "@/utils/Format"; | ||
| import FormatCurrency from "@/utils/FormatCurrency"; | ||
| import { useRouter } from "next/router"; |
There was a problem hiding this comment.
! appRouter를 사용하시는걸로 확인되는데 해당라우터는 pageRouter일것 같아요.
next/navigator를 사용해야 하지 않을까 싶습니다!
| } = useQuery<Comment[]>({ | ||
| queryKey: ["productComments", id], | ||
| queryFn: async () => { | ||
| const res = await axios.get(`/products/${id}/comments`); |
There was a problem hiding this comment.
먼저 typescript형태만 본다면 axios.get 형태에서 generic타입을 부여해야할 것 같아요.
현재 res의 타입은 여전히 any일것 같고, 타입에대한 검증이 반만 이루어질것 같습니다.
또한 axios요청하는 부분은 함수로 따로두어 관리해주는게 좋을것 같아요.
분리된 axios호출부분이 공통적인 Response에 대한 타입을 두고 처리될 수 있고, 원하는 viewModel 형태로 구성할 수 있다면 더 좋겠죠.
interface ResponseType<T> {
code: HttpStatusCode,
data: T,
error?: Error,
message: string,
}
...
const res = await axios.get<ResponseType<Comment[]>>(`/products/${id}/comments`);
...
| if (isEditMode) { | ||
| // 수정 API 호출 | ||
| const response = await axios.patch( | ||
| const response = await axios.patch<PostData>( |
There was a problem hiding this comment.
오! 여기는 타입을 제대로 작성해주셨네요 다른 axios부분도 이와같이 수정되면 좋을것 같아요!
| } else { | ||
| // 등록 API 호출 | ||
| const response = await axios.post( | ||
| const response = await axios.post<{ |
There was a problem hiding this comment.
이런부분도 마찬가지로 axios요청하는부분이 컴포넌트와 별개의 파일로 구분이되면 좋을것 같아요.
현재 해당컴포넌트에서는 너무많은 코드(axios요청, jsx컴포넌트그리기, 각종 비즈니스로직, 에러처리 등등..)들이 동작하고있습니다
| alert(`오류 발생: ${response.data.error || "알 수 없는 오류"}`); | ||
| } | ||
| } catch (error) { | ||
| } catch (error: any) { |
There was a problem hiding this comment.
이런곳엔 Error타입을 사용해주시면 좋을것 같아요
| id: number; | ||
| title: string; | ||
| createdAt: string; | ||
| [key: string]: any; // 추가적인 속성을 허용 |
There was a problem hiding this comment.
특별한 이유가없다면 모델을 정의하는 타입에 추가속성을 동적으로 가져가게 하는 방법은 지양해야될 포인트 같습니다
| import styles from "./ItemSection.module.css"; | ||
|
|
||
| const getPageSize = () => { | ||
| type Item = { |
There was a problem hiding this comment.
이렇게 공통적으로 사용되는 타입도 한번만 정의하여 export해서 사용하면 좋을것 같아요.
현재상태라면 Item이라는 타입이 변경되었을때 수많은 파일에서 수정해주어야 하고 추적도 어려울것 같습니다.
또한 type을 두는 파일의 위치도 고민해보셔야 할 것 같아요
2 participants
요구사항
기본 요구사항
공통
Github에 위클리 미션 PR을 만들어 주세요.
React 및 Express를 사용해 진행합니다.
TypeScript를 활용해 프로젝트의 필요한 곳에 타입을 명시해 주세요.
any 타입의 사용은 최소화해 주세요.
복잡한 객체 구조나 배열 구조를 가진 변수에 인터페이스 또는 타입 별칭을 사용하세요.
Union, Intersection, Generics 등 고급 타입을 적극적으로 사용해 주세요.
타입 별칭 또는 유틸리티 타입을 사용해 타입 복잡성을 줄여주세요.
타입스크립트 컴파일러가 에러 없이 정상적으로 작동해야 합니다.
프론트엔드
멘토에게