-
Notifications
You must be signed in to change notification settings - Fork 37
[최성락] Sprint8 #277
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
[최성락] Sprint8 #277
The head ref may contain hidden characters: "React-\uCD5C\uC131\uB77D-sprint8"
Conversation
…rint-Mission into React-최성락
- TypeScript 설치 및 관련 패키지 추가 - 기존 JS 파일 확장자를 TS/TSX로 변경 - tsconfig.json 파일 추가 및 설정 - @types/react, @types/react-dom 등 타입 정의 패키지 설치
… 에러 메시지 표시”로 간단히 처리.
Lanace
left a comment
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.
개발하시느라 고생 많으셨어요ㅎㅎ!
| const getProducts = async (limit: number, sort: SortOption): Promise<void> => { | ||
| try { | ||
| const { list } = await fetchProducts(sort, limit); | ||
| setItems(list); | ||
| } catch (error) { | ||
| console.error(error); | ||
| const message = error instanceof Error ? error.message : "알 수 없는 오류가 발생했습니다"; | ||
| setError(message); | ||
| } | ||
| }; |
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.
이정도로 예외처리 해도 충분하긴 해요!
아마 느끼셨겠지만 이런 부분들이 매우 많이 사용될꺼에요.
그래서 이걸 추상화하고 공통으로 묶어서 사용하려고 사람들이 많이 시도해왔는데, react-query라는 라이브러리를 한번 사용해보시면 좋을것같아요ㅎㅎ!
이거 같이 멘토링에서 해보시져...!
| useEffect(() => { | ||
| const debounceResize = debounce(handleResize, 300); | ||
| window.addEventListener("resize", debounceResize); | ||
| return () => window.removeEventListener("resize", debounceResize); | ||
| }, [handleResize]); |
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.
resize 될때 처리한 부분도 나쁘지 않게 잘 된것같아요ㅎㅎ!
이부분떄문에 서버에 성능부하를 주진 않을꺼에요~
왜냐하면 값이 바뀌었을때만 요청이 날아갈꺼거든요!
| <section className="all-products"> | ||
| <div className="all-products-list"> | ||
| {items.map((item) => ( | ||
| <ItemCard key={item.id} item={item} /> | ||
| ))} | ||
| </div> | ||
| </section> |
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.
깔끔하게 잘 구현하셨네요ㅋㅋ
| const getProducts = async (limit: number): Promise<void> => { | ||
| if (limit !== null) { | ||
| try { | ||
| const { list } = await fetchProducts("favorite", limit); | ||
| setItems(list); | ||
| } catch (error) { | ||
| console.error(error); | ||
| const message = error instanceof Error ? error.message : "알 수 없는 오류가 발생했습니다"; | ||
| setError(message); | ||
| } | ||
| } | ||
| }; |
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.
전반적으로 비슷한 코드 스타일로 통일되어있네요
매우 좋은 습관이에요
코드의 스타일이 일관되어야 읽기도 좋고 나중에 문제가 생겨도 찾기가 좋거든요ㅎㅎ!
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.
다만 거의 비슷한 코드가 계속해서 나오는건 공통화 시킬 수 있다는거니 개선해볼 수 있을것같아요ㅎㅎ!
| }, [handleResize]); | ||
|
|
||
| useEffect(() => { | ||
| if (typeof pageSize === "number") { |
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.
pageSize 값이 number가 아닌 다른값이 나올떄도 있나요??
| interface CommentListProps { | ||
| comments: Comment[]; | ||
| } | ||
|
|
||
| function CommentList({ comments }: CommentListProps) { | ||
| const getRelativeTime = (createdAt: string, updatedAt: string) => { |
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.
Props 타입 잘 정의하셨는데요?ㅋㅋ
깔끔하게 필요한 props만 정의해서 사용했네여
| const [inputValue, setInputValue] = useState<string>(""); | ||
|
|
||
| const handleInputChange = (e) => { | ||
| const handleInputChange = (e: React.ChangeEvent<HTMLInputElement | HTMLTextAreaElement>): void => { |
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.
input 이랑 textarea랑 같이 사용하려고 하신 이유가 있을까요?
| interface ImageUploadProps { | ||
| title: string; | ||
| } | ||
|
|
||
| function ImageUpload({ title }: ImageUploadProps) { |
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.
이 컴포넌트도 잘 작성해주신것같아요
이미지 업로드를 하려고 하면 URL.revokeObjectURL 로 미리보기를 보여주려고 하신거가 좋네여
근데 스웨거에 보면 https://panda-market-api.vercel.app/docs/#/Image/ImageUpload 가 있어서
이미지를 업로드하면 url이 응답값으로 오는걸 볼 수 있을거에요
그래서 차라리 image 를 업로드하고 결과값으로 오는 url을 화면에 보여주는게 좀더 좋을것같아요ㅎㅎ!
| export async function apiRequest<T>(url: string, options?: RequestInit): Promise<T> { | ||
| try { | ||
| const response = await fetch(url, { | ||
| headers: { "Content-Type": "application/json" }, | ||
| ...options, | ||
| }); | ||
| if (!response.ok) { | ||
| handleResponseError(response); | ||
| } | ||
| return await response.json(); | ||
| } catch (error) { | ||
| if (error instanceof HttpException) { | ||
| throw error; | ||
| } | ||
| console.error("네트워크 오류", error); | ||
| throw new Error("네트워크 오류가 발생했습니다. 잠시 후 다시 시도해주세요."); | ||
| } | ||
| } |
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.
이거는 예전부터 사용해오시던건가여??
잘 작성하셨길래...ㅎㅎ;;
좀더 개선해보자면 Promise reject으로 error 를 throw해주면 좀더 좋을것같아요ㅎㅎ!
| export function debounce<T extends (...args: any[]) => void>(callback: T, delay = 300) { | ||
| let timeoutId: ReturnType<typeof setTimeout> | null = null; | ||
|
|
||
| return (...args: Parameters<T>) => { | ||
| if (timeoutId) { | ||
| clearTimeout(timeoutId); | ||
| } | ||
| timeoutId = setTimeout(() => { | ||
| callback(...args); | ||
| }, delay); | ||
| }; | ||
| } |
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.
이건 직접 작성하신건가여??
lodash를 사용해보시는것도 좋을것같아요ㅎㅎ!
|
타입스크립트 관련해서는 너무 처음부터 잘 쓰려고 생각하시는것보단 그냥 다른 라이브러리나 다른사람이 작성해둔 코드를 사용해보다보면 어떤게 편하고 어떤게 불편하다는게 느낌이 올꺼에요ㅎㅎ! 특히 서버랑 통신하는 부분을 보면 어떻게 response가 올지 모르는 any로 반환하는 함수와 타입이 명시되어있는 api는 개발자한테 개발경험이 매우 다르거든요ㅎㅎ;; 그래서 한번 멘토링에서 같이 보면 좋을것같아요! |
이거 관련해서는 딱히 debounce를 사용하지 않았어도 괜찮았을것같아요 |
요구사항
기본
심화
주요 변경사항
멘토에게