-
Notifications
You must be signed in to change notification settings - Fork 10
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
[6주차] azito 조유담 & 이나현 미션 제출합니다. #12
base: master
Are you sure you want to change the base?
Conversation
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.
안녕하세요 프론트 19기 멘토 김현민 이라고합니다~!!
전체적으로 매우 깔끔한 코드 너무 잘 봤습니다 ㅎㅎ 코드리뷰 하면서 제 의견들 살짝 남겨보았고, 제가 배워가는 부분도 있었던 것 같습니다!!
제 의견은 정답이 아니라 참고용으로 한번 읽어보시면 좋을 것 같아요~
이번 과제 정말 수고 많으셨습니다~!!!
@@ -0,0 +1,34 @@ | |||
name: Deploy |
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.
Vercel 이 팀 레포를 서빙할 때는 요금이 부과되어서 개인 레포로 옮기는 과정을 Github Actions로 자동화 하신 것 같은데 멋지네요~!!!
export const instance = axios.create({ | ||
baseURL: process.env.NEXT_PUBLIC_API_URL, | ||
headers: { "Content-Type": "application/json", authorization: `Bearer ${process.env.ACCESS_TOKEN}` }, | ||
params: { | ||
api_key: process.env.NEXT_PUBLIC_API_KEY, | ||
}, | ||
}); |
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.
axios 인스턴스를 생성하여 공통정보를 설정해 놓으신 점 너무 좋네요 ㅎㅎ
@layer utilities { | ||
.flex-center { | ||
@apply flex items-center justify-center; | ||
} | ||
|
||
.flex-column { | ||
@apply flex flex-col; | ||
} | ||
} |
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.
자주 사용되는 스타일 적절한 이름으로 세팅하신 점 인상깊네요!!
export const metadata: Metadata = { | ||
title: "Netflix", | ||
description: "나현이와 유담이의 넷플릭스 클론코딩", | ||
}; |
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.
metadata 설정까지 너무 좋네요~!!
<button className="flex-center h-45pxr w-303pxr gap-15pxr rounded-md bg-gray-30 text-18pxr font-semibold text-black"> | ||
<PlayIcon alt="play 아이콘" /> | ||
Play | ||
</button> |
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.
Play 버튼을 컴포넌트로 분리하고 해당 컴포넌트를 파악하기 위해 중요한 정보인 "Play" 텍스트를 외부에서 넘겨주면 가독성에 더 좋을 것 같아요!!
또 비슷한 디자인의 Play 버튼이 홈에도 있어서 PlayButton 컴포넌트를 생성한 뒤, 세부 디자인은 외부에서 전달해서 변경하면 더 좋을 것 같네요!!(twMerge 라이브러리 사용)
PlayButton 컴포넌트 ->
return <button className={twMerge("flex-center rounded-md bg-gray-30 text-18pxr font-semibold text-black", className)}>
<PlayIcon alt="play 아이콘" />
{text}
</button
PlayButton 컴포넌트 사용(홈에서는 className 세부 디자인만 다르게 넘기기) ->
<button className="flex-center h-45pxr w-303pxr gap-15pxr rounded-md bg-gray-30 text-18pxr font-semibold text-black"> | |
<PlayIcon alt="play 아이콘" /> | |
Play | |
</button> | |
<PlayButton text="Play" className="h-45pxr w-303pxr gap-15pxr" /> |
const [randomIndex, setRandomIndex] = useState<number>(0); | ||
|
||
useEffect(() => { | ||
const index = Math.floor(Math.random() * movieData.results.length); | ||
setRandomIndex(index); | ||
}, [movieData]); |
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.
해당 컴포넌트에서 랜덤 index를 세팅하는 상세 로직 자체는 그렇게 중요한 부분이 아니기 떄문에 커스텀 훅에서 해당 로직을 실행하고 index만 넘겨줘도 좋은 코드가 될 것 같아요 ㅎㅎ
useRandomIndex 훅 생성
export default function UseRandomIndex(movieData) {
const [randomIndex, setRandomIndex] = useState<number>(0);
useEffect(() => {
const index = Math.floor(Math.random() * movieData.results.length);
setRandomIndex(index);
}, [movieData]);
return randomIndex;
}
const [randomIndex, setRandomIndex] = useState<number>(0); | |
useEffect(() => { | |
const index = Math.floor(Math.random() * movieData.results.length); | |
setRandomIndex(index); | |
}, [movieData]); | |
const randomIndex = useRandomIndex(movieData); |
const handleScroll = (direction: "left" | "right") => { | ||
if (containerRef.current) { | ||
const { scrollLeft, clientWidth } = containerRef.current; | ||
const scrollTo = direction === "left" ? scrollLeft - clientWidth : scrollLeft + clientWidth; | ||
containerRef.current.scrollTo({ left: scrollTo, behavior: "smooth" }); | ||
} | ||
}; |
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.
방향에 따라 스크롤 동작을 분기처리하신 점 너무 좋네요~!!!!
await queryClient.prefetchQuery({ queryKey: ["popularMovies"], queryFn: () => getMovies("popular") }); | ||
|
||
return ( | ||
<HydrationBoundary state={dehydrate(queryClient)}> |
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.
클라이언트 컴포넌트에서 데이터 fetching시 react-query 버전 5를 사용할 때 새로 사용되는 거라고 들었어요!! 저는 아직 버전 4까지만 활용해봐서 아직 사용을 못해봤는데 신기하네요 ㅎㅎ저도 한번 버전 5 사용해보겠습니다~~~
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 { isVisible, targetRef } = useInfiniteScroll(); | ||
|
||
const { isFetching, isFetchingNextPage, data, fetchNextPage, hasNextPage } = useInfiniteQuery({ |
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를 활용한 무한 스크롤 구현 너무 멋지네요 !!!
import { movieSectionTitle } from "@/constants/movies"; | ||
|
||
export interface Movie { | ||
adult: boolean; | ||
backdrop_path: string | null; | ||
genre_ids: number[]; | ||
id: number; | ||
original_language: string; | ||
original_title: string; | ||
overview: string; | ||
popularity: number; | ||
poster_path: string | null; | ||
release_date: string; | ||
title: string; | ||
video: boolean; | ||
vote_average: number; | ||
vote_count: number; | ||
} | ||
|
||
export type MovieCategory = keyof typeof movieSectionTitle; |
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.
Tanstack query를 사용하시는 모습이 정말 인상 깊었어요!
클라이언트 컴폰넌트와 서버 컴포넌트의 차이를 잘 이해하시고 다르게 적용하시는 모습에서도 많이 배웠구요.
이번 과제도 정말 고생 많으셨습니다 :)
#!/bin/sh | ||
. "$(dirname "$0")/_/husky.sh" | ||
|
||
npx lint-staged | ||
echo "Azito FrontEnd 화이팅! (*ૂ❛ᴗ❛*ૂ)" |
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.
husky 라이브러리를 활용해서 commit convention을 통일 하시는 것 매우 바람직한 것 같아요 👍
<html> | ||
<body> | ||
<h2>Something went wrong!</h2> | ||
<button onClick={() => reset()}>Try again</button> | ||
</body> | ||
</html> |
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.
페이지 컴포넌트 수준에서의 에러는 error.tsx 컴포넌트가, 이들이 잡아주지 못하는 수준의 "최후의 방어선" 개념에서는 global-error.tsx 컴포넌트가 에러를 보여준다는 것을 잘 이해하고 계신 것 같아요!
다만, 렌더링 되는 UI가 상당히 많이 겹치는 것 같은데, 이를 잘 분리해서 활용해도 좋을 것 같아요
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.
global-error
를 따로 설정할 수도 있군요!
queries: { | ||
// SSR(서버 사이드 렌더링)에서는 클라이언트에서 즉시 다시 데이터를 가져오는 것을 | ||
// 방지하기 위해 기본 staleTime을 0보다 높게 설정하는 경우가 많습니다. | ||
staleTime: 60 * 1000, // 1분 |
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.
Tanstack Query
와 SWR
라이브러리의 핵심 개념인 staleTime을 이용하셨네요!
staleTime
과 cacheTime
의 차이를 이해해보시고 후자 역시 적극 활용하셔도 좋을 것 같아요
} | ||
@layer utilities { | ||
.flex-center { | ||
@apply flex items-center justify-center; |
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.
tailwindCSS는 클래스가 너무 많아질 경우, 코드의 가독성이 떨어진다는 단점이 있죠!
이를 일종의 alias로 만들어주시고 작업하신 점 나이스 합니다
const categories: MovieCategory[] = ["upcoming", "now_playing", "popular", "top_rated"]; | ||
const fetchPromises = categories.map((category) => getMovies(category)); | ||
|
||
const moviesResults = await Promise.all(fetchPromises); |
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.all()
메서드를 활용하면 여러 비동기 api 호출의 종료 시점을 맞춰준다는 장점이 있죠!
이를 모두 따로따로 진행할 경우, 도착한 것들 순서대로 드문 드문 data 가 렌더링되는 현상이 발생하는데 이걸 고려하셔서 잘 작성하신 것 같네요
"use client"; | ||
import { usePathname } from "next/navigation"; | ||
import BottomNavbarItem from "./BottomNavbarItem"; | ||
|
||
const BottomNavbar = () => { | ||
const pathname = usePathname(); | ||
if (pathname === "/") return null; | ||
return ( | ||
<nav className="fixed inset-x-0pxr bottom-0pxr max-w-375pxr mx-auto z-50 bg-black h-48pxr flex-center"> | ||
<ul className="w-full px-26pxr flex items-center justify-between"> | ||
<BottomNavbarItem item="home">Home</BottomNavbarItem> | ||
<BottomNavbarItem item="search">Search</BottomNavbarItem> | ||
<BottomNavbarItem item="comingsoon">Coming soon</BottomNavbarItem> | ||
<BottomNavbarItem item="downloads">Downloads</BottomNavbarItem> | ||
<BottomNavbarItem item="more">More</BottomNavbarItem> | ||
</ul> | ||
</nav> | ||
); | ||
}; | ||
|
||
export default BottomNavbar; |
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.
현재 해당 컴포넌트를 루트 레이아웃에 포함시키고, 클라이언트 컴포넌트로 만들어서 usePathname()
훅으로 조건부 검사하여 렌더링하고 계신 것처럼 보입니다.
하지만 함수 내부적으로 너무 분기적인 로직을 쓰는 것보다 UI 렌더링에만 초점을 맞추는 것이 추후 확장성에 있어서 더 좋을 수 있다고 생각해요. 지난 주차에 저희 조가 발표한 내용처럼 일종의 CommonLayout 디렉터리를 ()
를 이용해서 만드신 후, 랜딩 페이지를 제외한 페이지들에서 공통적인 BottomNavbar를 적용해보시면 어떨까 싶어요
params: Params; | ||
} | ||
|
||
const DetailPage = async ({ params }: DetailPageProps) => { |
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.
dynamic routing을 진행하는 경우에, 클라이언트 컴포넌트에서는 usePathname()
같은 훅을 이용하면 되지만, 서버 컴포넌트의 경우에는 params 속성을 객체 구조 분해 방식으로 받아 쓰면 된다는 것을 잘 이해하고 계신 것 같아요!
searchParams
속성으로 나중에 query string까지 낚아채는 것도 살펴보시면 좋을 것 같습니다 :)
export const instance = axios.create({ | ||
baseURL: process.env.NEXT_PUBLIC_API_URL, | ||
headers: { "Content-Type": "application/json", authorization: `Bearer ${process.env.ACCESS_TOKEN}` }, | ||
params: { | ||
api_key: process.env.NEXT_PUBLIC_API_KEY, | ||
}, | ||
}); | ||
|
||
// 항상 패칭 요청을 보내면 response.data가 값이 리턴되게 | ||
instance.interceptors.response.use( | ||
function (response) { | ||
return response.data; | ||
}, | ||
// 에러 일괄 처리 | ||
async (error) => { | ||
console.log(error.message); | ||
return Promise.reject(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.
아예 api 디렉터리를 app 디렉터리 하위로 옮겨버려, 일종의 api routing 기능으로서 동작하게 만드는 것은 어떨까하는 생각도 드네요!
await queryClient.prefetchQuery({ queryKey: ["popularMovies"], queryFn: () => getMovies("popular") }); | ||
|
||
return ( | ||
<HydrationBoundary state={dehydrate(queryClient)}> |
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.
기존의 reactJS 방식에서처럼 클라이언트 컴포넌트 단에서는 data를 fetching 하는 로직을 useQuery()
나 useMutation()
훅을 통해서 진행했던 것에 반해, nextJS는 풀스택 프레임워크인 만큼 서버 단에서 미리 data fetching을 진행하고 hydration을 진행해야 하는 것을 매우 잘 알고 계신 것 같아요!
nextJS의 근본 원리에 대해 너무 잘 알고 계신 것 같아, 매번 많이 배웁니다.
<section className="flex-column mb-43pxr items-center gap-11pxr"> | ||
<div className="relative h-375pxr w-full"> | ||
{randomIndex ? ( | ||
<Link href={`/media/${data.id}`} className="cursor-pointer"> |
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.
Link
컴포넌트는 최종적으로 a 태그로 변환되어서 자동으로 cursor-pointer 효과가 적용되지 않나요...?
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.
6주차 과제 고생하셨어요!
Tanstack Query
를 통해 무한스크롤을 구현해주신거 인상깊게 봤어요! 현재 검색페이지에서 호출되는 API를 개발자도구 네트워크에서 확인해보면 API키가 노출되는데, 이를 해결할 방법도 찾아보시면 좋을 거 같아요!
display: none; | ||
} | ||
} | ||
@layer utilities { |
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.
자주 쓰이는 함수들에 대해 @layer
를 통해 함수화 시켜준거 좋아요!
<html> | ||
<body> | ||
<h2>Something went wrong!</h2> | ||
<button onClick={() => reset()}>Try again</button> | ||
</body> | ||
</html> |
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.
global-error
를 따로 설정할 수도 있군요!
@@ -0,0 +1,17 @@ | |||
import { dehydrate, QueryClient, HydrationBoundary } from "@tanstack/react-query"; |
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.
TanStack Query
의 dehydrate, QueryClient, HydrationBoundary
사용해주신 점 멋져요!
debounce((value: string) => { | ||
setDebouncedInputValue(value); | ||
}, 700), |
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.
debounce
를 통해 리렌더링 횟수를 줄일 수 있군요!
<SearchedMovies moviesData={searchMovies} isLoading={isFetching && searchMovies.length === 0} /> | ||
{!isFetching && searchMovies.length === 0 && ( | ||
<p className="flex-center mt-120pxr">{debouncedInputValue}에 대한 검색 결과가 없어요.</p> | ||
)} |
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.
검색어가 바뀔때마다 API 호출을 통해 영화를 갱신하면서 debounce
를 통해 최적화를 시도해서 훨씬 좋은거 같아요! 사실 이렇게 하면 너무 자주 API를 호출하지 않을 까 했는데 0.7초라 넉넉하기도하고 현재 넷플릭스도 이런식으로 작동하네요!
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.
이렇게 최적화 할 수도 있군요..!
) : ( | ||
<> | ||
<h1 className="px-12pxr py-24pxr text-27pxr font-bold">Top Searches</h1> | ||
<SearchedMovies moviesData={popularMoviesData?.results || []} isLoading={isLoadingPopularMovies} /> |
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.
여기도 무한 스크롤이 필요하지 않을까 싶어요! 사실 사용자경험적으로 검색창에서 무한스크롤 기능을 이용하는 것은 검색 이후가 아니라 검색이전 일거란 생각이 들어서요.popularMovies
에서 useQuery
대신 useInfiniteQuery
훅을 한번 더 사용하거나 기존에 있던 useInfiniteQuery
훅에서 enabled
속성을 변경하거나 하는 방식으로요!
<div className="relative"> | ||
<SearchIcon className="absolute left-16pxr top-15pxr" alt="돋보기 모양 아이콘" /> | ||
<input | ||
value={inputValue} | ||
onChange={handleInputChange} | ||
className="h-52pxr w-full bg-gray-20 px-50pxr py-20pxr text-gray-30" | ||
placeholder="Search for a show, movie, genre, e.t.c." | ||
/> | ||
<button onClick={() => setInputValue("")}> | ||
<CloseIcon className="absolute right-16pxr top-18pxr" alt="검색 결과 되돌리기 아이콘" /> | ||
</button> | ||
</div> |
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 SearchedMovies = ({ moviesData, isLoading }: SearchedMoviesProps) => { | ||
if (isLoading) { | ||
return <SkeletonMovies />; | ||
} |
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.
이번주 과제도 수고하셨습니다..! nextJS에 대한 이해도가 높으신것 같아 많이 배워갑니다!👍
<button | ||
onClick={ | ||
// Attempt to recover by trying to re-render the segment | ||
() => reset() | ||
} |
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.
오 에러까지..! 많이 신경쓰신게 느껴집니다!
body { | ||
@apply text-white; |
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.
이런식으로 텍스트 색상 설정을 해주는 방법도 있네요! 배워갑니다
<SearchedMovies moviesData={searchMovies} isLoading={isFetching && searchMovies.length === 0} /> | ||
{!isFetching && searchMovies.length === 0 && ( | ||
<p className="flex-center mt-120pxr">{debouncedInputValue}에 대한 검색 결과가 없어요.</p> | ||
)} |
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.
이렇게 최적화 할 수도 있군요..!
6주차 미션: Next-Netflix
🐻 배포 링크
https://next-netflix-19th.vercel.app/
👩💻 구현 기능
유담
검색 페이지에만 리액트 쿼리 적용해놓은 상태입니다.
나현
❓ Key Questions
1. 정적 라우팅(Static Routing) / 동적 라우팅(Dynamic Routing)이란?
정적 라우팅(Static Routing)은 고정된 페이지로 라우팅 하는 것이다. 직접 경로를 일일이 설정을 해주는 것이다. Next.js에서는 pages 폴더 안에 만들어진 파일의 이름으로 경로를 설정해주면 된다.
동적 라우팅(Dynamic Routing)은 미리 정의되고 고정된 페이지가 아니라 사용자가 접근한 경로 또는 특정 값에 따라 동적으로 변화하는 주소를 의미한다. Next.js에서는 컴포넌트 파일명에 괄호를 씌워주면 된다.
2. 성능 최적화를 위해 사용한 방법
3. 전반적인 협업 과정
노션과 깃허브 이슈, PR을 주로 사용했습니다. 폴더 구조, 커밋 컨벤션, 변수나 함수명 컨벤션 등을 통일하여 노션에 작성해두었고 역할을 분담하고 각자의 맡은 기능에 대한 브랜치, 이슈를 생성하여 작업하였습니다. 정리하자면, 이슈 생성 -> 브랜치 생성 -> PR 순서대로 작업했습니다.