✨[Code Review] aeong code review #37
Conversation
There was a problem hiding this comment.
안녕하세요, UMC 7th 웹 중앙 파트장 매튜/김용민입니다. 먼저, 부족한 웹 워크북이지만, 열심히 해주셔서 정말 감사합니다! 남은, 스터디 아래 내용들 위주로 조금 학습하시면 좋지 않을까 생각합니다!
- 협업을 위한, 좋은 폴더 구조 습관화.
- 비즈니스 로직 분리
- 전역 상태 관리 학습
- tanstack-query 쿼리키에 대한 이해
추후 10주차 미션은 타입스크립트로, 해당 프로젝트를 전환하는 것입니다! styled-components 또한, 정말 훌륭하나, tailwind-css 또한 정말 괜찮은 스타일링 라이브러리이기 떄문에 이를 학습해보는 것도 좋아보입니다!
또한 타입스크립트를, 적극적으로 활용해보시면서, 프로젝트를 진행해보시고, 데모데이도 진행해보시면 좋지 않을까 싶습니다!
정말 고생많으셨습니다!
| element: <RootLayout />, | ||
| errorElement: <NotFound />, | ||
| // Navbar 밑에 path에 해당하는 element를 보여주고 싶으면 아래와 같이 children을 활용 | ||
| children: [ |
There was a problem hiding this comment.
현재 프로젝트의 경우, 이렇게 진행해도, 페이지의 개수가 많지 않아, 괜찮을 것 같지만. 나중에, 페이지의 부모 자식 관계가 명확해진다고 했을 떄 아래 예시 코드처럼 구분하는 것도 좋아보입니다!
const router = createBrowserRouter([
{
path: '/',
element: <RootLayout/>,
errorElement: <NotFound/>,
children: [
{
index: true,
element: <HomePage/>,
},
{
path: 'movies',
children: [
{
index: true,
element: <Movies/>
},
{
path: 'now-playing',
element: <NowPlaying/>
},
{
path: 'up-coming',
element: <UpComingPage/>
},
{
path: 'top-rated',
element: <TopRatedPage/>
},
{
path: 'popular',
element: <PopularPage/>
},
{
path: ':movieId',
element: <MovieDetailPage/>
}
]
},
{
path: 'search',
element: <SearchPage/>
},
{
path: 'login',
element: <LoginPage/>
},
{
path: 'signup',
element: <SignUpPage/>
}
]
},
])| // 영화 컴포넌트를 클릭했을 때 상세 페이지로 이동하도록 | ||
| onClick={() => | ||
| navigate(`/movies/${movie.id}`, { | ||
| replace: false, |
There was a problem hiding this comment.
replace: false, 는 기본값이기에 굳이 사용하지 않아도 좋을 것 같습니다!
| export default Movie; | ||
|
|
||
| // CSS | ||
| const MovieContainer = styled.div` |
There was a problem hiding this comment.
저 같은 경우에는, 보통 파일 구조를 아래와 같이 가져가는 편입니다.
components
-
CardSkeleton
- index.tsx
- index.style.js
styled component와, 컴포넌트 or 페이지 파일들은, 따로 분리하는 것이 관리 측면에서 더 효율적인 것 같습니다!
| border-radius: 10%; | ||
| `; | ||
|
|
||
| const Title = styled.h3` |
There was a problem hiding this comment.
모든 컴포넌트에 대하여, 스타일드 컴포넌트를 만들 필요는 없다고 생각합니다.
const Container = styled.div`
padding: 5px;
width: 100%;
h1 {
overflow: hidden;
white-space: nowrap;
text-overflow: ellipsis;
word-break: break-all;
color: white;
font-size: 12px;
}
h3 {
color: lightgray;
font-size: 8px;
}
`Container 안에 쓰이는, h1 태그나, h3 태그 같은 경우, 위와같이, 하나의 스타일드 컴포넌트 안에, 잘 사용할 수 있을 것 같습니다!
| import { useNavigate } from "react-router-dom"; | ||
|
|
||
| const Navbar = () => { | ||
| const [user, setUser] = useState(null); |
There was a problem hiding this comment.
로그인 상태와 같은 경우는, 모든 컴포넌트가, 해당 상태를 공유해야한다고 생각합니다!!
그럴 경우에, 정말 Navbar 에서만, 로그인 된 상태를 관리하는 것이 맞는지 의문이듭니다!!!
제가 이전에, 강의로 제공해드린 Context API나 이런것을 활용해서, 전역 상태로, 해당 로그인 상태를 모든 컴포넌트에서, 공유해야하지 않을까 생각합니다!
useContext를 활용해서, 고치셔야 할 것 같고, 해당 강의영상 또한 제 유튜브를 참고하시면 좋을 것 같습니다!
| const MovieDetailsPage = () => { | ||
| const { movieId } = useParams(); | ||
|
|
||
| const { data: movieDetails } = useCustomFetch( |
There was a problem hiding this comment.
언어 같은 경우도, 충분히 사용자의 요구사항에 따라 변경될 수 있는 부분이기에, parameter로 받아서 관리하는 것도 좋아보입니다!
| isPending, | ||
| isError, | ||
| } = useQuery({ | ||
| queryKey: ["movies_detail"], |
There was a problem hiding this comment.
리액트 쿼리는, 쿼리키를 기준으로 캐싱을 합니다.
정말, 사용자의 정보를 불러오는 것이 movies_detail에 관리하는 것이 맞을까요??
위처럼 관리하게 된다면, 어떠한, 영화 페이지에 들어가도 캐싱되어있는 상태 동안은, 동일한 credits이 보일 것 이라고 생각합니다.
상세 페이지에 나오는 데이터 같은 경우에는
queryKey: ['credit', movie_id]
이런식으로, 영화 상세페이지를 조회할 수 있게 해주는 movie_id를 추가로, 쿼리키에 포함해서, 다른 영화 상세페이지에 들어가면, 다른 credit에 대한 정보가 나올 수 있도록 해주는 것이 좋아보입니다!
| <Details> | ||
| <MovieDetailsContainer> | ||
| <MovieDetailsText> | ||
| <h1>{movieDetails?.title}</h1> |
There was a problem hiding this comment.
매번 모든 데이터를 출력할 때 movieDetails?.키값 을 반복하게 되는데, 이를 구조 분해할당으로, 변경해보는 것도 좋아보입니다.
const { title, vote_average, releaseDate } = movieDetails| <CategoriesContainer> | ||
| <Category>카테고리</Category> | ||
| <Link to="/movies/now-playing"> | ||
| <CategoryImage |
There was a problem hiding this comment.
같은 코드를 반복하는것은 좋아보이지 않습니다. 지금 Link와 Category Image가, 계속해서 반복하고 있습니다!! 사실 영화 리스트에서, 영화 개별 컴포넌트를 불러오는 것과 같은 방법으로 이를 해결할 수 있습니다. 예시 코드는 아래와 같습니다.
const categories = [{ link: '/movies/popular', imageUrl: '이미지 주소' } , { link: '/movies/up_coming', imageUrl: '이미지 주소' }, ....]
categories.map((category) => <Link to={category.link}><image src={category.imageUrl} /></Link>| const [debouncedValue, setDebouncedValue] = useState(""); | ||
|
|
||
| // 디바운스: searchValue가 변경될 때마다 타이머로 debouncedValue 설정 | ||
| useEffect(() => { |
There was a problem hiding this comment.
debounce나, throttling과 같은 경우는, 정말 많은 곳에서 활용할 수 있는 hook의 예시라고 생각합니다.
hooks폴더를 만들어, 해당 로직을 useDebounce.js, useThrottling.js 이런식으로, 관리해서 사용하는 곳마다 해당 함수를 호출해서 사용하는 것이 좋아보입니다!
No description provided.