✨[Code Review] ralph code review #38
Conversation
There was a problem hiding this comment.
안녕하세요, UMC 7th 웹 중앙파트장 매튜/김용민입니다. 먼저, 부족한 워크북이지만, 열심히 해주신 랄프에게 감사의 말씀 드립니다.
전체적인 피드백은 아래와 같습니다.
- 폴더구조 명확하게 하기.
- 비즈니스 로직 분리.
- 컴포넌트/페이지 파일과 스타일 파일 분리하기.
- 전역상태에 대한 학습
추가적으로, 10주차의 경우 TS를 활용해서, 해당 영화 프로젝트를 전체적으로 전환하는 미션을 드렸으니, 시험끝나고, 여유가 생겼을 떄 팀원분들과 함께 타입스크립트를, 학습하시면서 해당 영화페이지를 전환해봄을 추천드립니다!
스타일드 컴포넌트 또한 훌륭한 CSS-in-JS 라이브러리이지만, 요즘 많은 사람들이 사용하는 tailwind-css 또한 공부해봄을 추천드립니다!
정말 고생많으셨습니다!
| const router = createBrowserRouter([ | ||
| // router 객체 생성 | ||
| { | ||
| //경로(path)와 해당 경로에 매칭되는 컴포넌트(element)를 설정 |
There was a problem hiding this comment.
해당 부분, 애옹님 리뷰 참고 부탁드립니다!
| @@ -0,0 +1,9 @@ | |||
| import axios from "axios"; | |||
| const axiosInstance2 = axios.create({ | |||
There was a problem hiding this comment.
axiosInstance2라는 이름은, 다른 사람들로 하여금, 이해하기 어려운 네이밍이라고 생각합니다. tmdb를 호출하는, axiosInstance와 겹친다면은, 해당 이름은 movieAxiosInstance, authAxiosInstance 이런식으로, 조금 더 다른 사람이 이해하기 쉬운 네이밍을 갖고가는 것도 좋아보입니다!
| @@ -0,0 +1,62 @@ | |||
| import styled, { keyframes } from "styled-components"; | |||
There was a problem hiding this comment.
조금 더 스타일 파일임을 명시하기 위해서
movieSkeletongstyle.js라는, 이름 보다는
movieSkeleton.style.js 이라는 이름을 가져가는 것이 좋아보입니다!
| } | ||
| `; | ||
|
|
||
| const Edward = styled.div` |
There was a problem hiding this comment.
Edward라는 스타일드 컴포넌트가, 무엇을 의미하는지 이해하기 어려워 보입니다. 다른 사람이 이해할 수 있는, 컴포넌트 명을 만드는게 좋아보입니다!
| function Movielistskeleton({ number }) { | ||
| return new Array(number) | ||
| .fill(0) | ||
| .map((_, idx) => <Movieskeleton></Movieskeleton>); |
There was a problem hiding this comment.
사소한 부분이지만, 컴포넌트 내부에, children 이 들어가지 않습니다. 그럴 경우에, 뒤에 부분을 생략할 수 있으니
이런식으로 작성해주는 것이 좋아보입니다!
|
|
||
| const useGetInfiniteMovies = (category) => { | ||
| return useInfiniteQuery({ | ||
| queryKey: [category], |
There was a problem hiding this comment.
물론, 현재의 경우에서는, category만을 쿼리키로 전달해서, 관리해도 큰 문제가 없지만, 조금 더 영화들을 포괄하는 쿼리키를 맨 앞에 주는 것이 좋아보입니다.
queryKey: ["movie", category]
이렇게 관리했을 떄, 추후에 배울 useMutation을 통해 특정 쿼리키를 invalidateQueries를 했을 떄, 조금 더 관리하기 쉬워집니다!
| import React from "react"; | ||
| import { useNavigate } from "react-router-dom"; | ||
| // 영화의 인물 정보를 나타내는 페이지 | ||
| const IMG_BASE_URL = "https://image.tmdb.org/t/p/w500/"; |
There was a problem hiding this comment.
여러 곳에서, IMG_BASE_URL을 사용함을 확인했습니다. 이렇게 많이 사용하는, 상수인 경우에는 따로 constants 폴더를 만들어, 상수들을 관리하는 파일 내부에 아래와 같이 선언해서, 사용하는 곳에서 import 하는것이 좋아보입니다.
export const IMG_BASE_URL = "https://image.tmdb.org/t/p/w500/";| import QueryUserInfo from "./QueryUserInfo"; | ||
|
|
||
| const Navbar = () => { | ||
| const [token, setToken] = useState(localStorage.getItem("accessToken")); |
There was a problem hiding this comment.
해당 부분도, 애옹님 리뷰 참고하시면 될 것 같습니다!
| margin-left: 0px; | ||
| `; | ||
|
|
||
| const LogoLink = styled(Link)` |
There was a problem hiding this comment.
react-router-dom 의 NavLink를 사용해봄을 추천드리며, 해당 부분 리뷰도, 애옹님 리뷰 참고하시면 될 것 같습니다!
| console.log(data); | ||
|
|
||
| // post 메서드로 인증 요청 | ||
| try { |
There was a problem hiding this comment.
auth 관련, api 호출 로직들을, apis 폴더의 auth.ts 이런식으로 만들어, 로그인, 로그아웃, 회원가입 등의 API들을 묶어서 관리하는 것이 좋아보입니다!
사용하는 사람은, 안에 구현 로직과 상관없이, 호출하는 함수 이름만 보고도, 어떤 기능인지 파악할 수 있게 말이죠!!
No description provided.