-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Re-op]KDT5_윤금엽_YoonGeumYeop_영화 검색 사이트 #70
base: main
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.
컨디션 난조에도 불구하고 구현해주신 금엽님 고생하셨습니다.👍🏻 단순히 강의의 내용을 따라가는 것 뿐 만 아니라 state의 내부 로직이 어떻게 진행되고 있고, 클래스로 구현된 컴포넌트들의 constructor render 메서드가 어느시점에 호출되는지 세부적으로 들여다볼 수 있었으면 합니다.
리뷰를 진행하며 궁금한 부분과 생각해볼만한 부분 코멘트 드렸습니다. 확인해보시고 리팩토링 해주시면 되겠습니다. 끝까지 화이팅~💪🏻
const { APIKEY } = process.env; | ||
|
||
export default async function handler(request, response) { | ||
const { title, page, id } = JSON.parse(request.body); |
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.
handler
를 통해서 요청 - 응답 객체를 변경하는 작업을 진행하신 것은 매우 좋은 시도라고 생각합니다. 주로 handler
를 통한 APP의 요청 - 응답 객체에 대한 변경은 로그인 체크나, 쿠키를 심는 것과 같은 모든 요청(APP 전역)에 대해서 필요한 작업을 위해 사용합니다. 아마 APIKEY
를 심는 공통의 작업을 위해서 handler
를 사용하신 것으로 보입니다. 하지만 여기서 조금 고려해볼 만한 것이 있습니다.
-
현재 요청을 진행하는 URL이 두개이기에 분기처리를 통해서 가능하지만, URL이 20-30개의 프로젝트일 경우에는 분기처리로는 구분이 어려울 것 같습니다. 이를 위해서는
APIKEY
를 심는 작업만handler
내에 구현하고, URL에 따라서 메서드를 각각 구현해야할 것으로 보입니다. -
모든 요청의 상태코드가 200(
response.status(200)
)으로 고정되어 있습니다. 이는 모든 요청에 대한 응답이 심지어 서버에서 전달하는 값이 오류가 발생하였다고 하더라도 API의 응답은 200, 정상이라는 의미를 전달하고 있습니다. HTTP에 따르면 응답의 상태에 따라서 200이 아닌, 400- 500번의 상태값을 반환하고 있는데요. API 서버에서 전달하는 상태값에 맞게 대응할 수 있도록 구분하여 구현해야합니다. 이를 위해서 HTTP 상태코드에 대해서 한 번 알아보도록 합시다!
HTTP response status codes
|
||
<!-- Google fonts - Oswald, Roboto --> | ||
<link | ||
rel="preconnect" |
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.
preconnect를 사용하여 font 파일을 들고오고 있네요:) 해당 속성이 어떠한 의미로 사용되고 있는지 또 어떤 효과가 있어서 추가한 것인지 설명해주실 수 있을까요?
<div class="the-loader hide"></div> | ||
` | ||
|
||
const moviesEl = this.el.querySelector('.movies') |
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.
영화 목록 태그를 클래스명으로 조회하고 있는데, CSS 특이성에 따르면 구현하신 페이지 내에서 MovieList
컴포넌트는 하나만 사용되므로 클래스명이 아닌 ID를 통해서 요소를 조회하는 것이 더 효율적일 것으로 보입니다. 이와 마찬가지로 아래의 loadEl
또한 ID를 통해서 조회하는 방식을 검토할 수 있을 것 같아요! 클래스명을 사용하여 조회해야한다면 그 이유도 설명해주실 수 있을까요?🤔
@@ -0,0 +1,13 @@ | |||
import fetch from 'node-fetch'; | |||
|
|||
const { APIKEY } = process.env; |
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.
process.env
의 경우, 빌드 파라미터로 들어가는 것으로 알고 있는데 어떠한 방식으로 해당값을 넣는지 궁금합니다.
} | ||
try { | ||
const res = await fetch('/api/movie', { | ||
method: 'POST', |
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.
handler
를 통해서 API 로직을 분리한 것은 매우 훌륭합니다. axios 라이브러리 및 get()
메서드 도입을 고려해보면 좋을 것 같아요. 현재 axios()
를 통해서 사용하고 있으나, axios에서는 HTTP 요청의 방식인 method
(GET
)에 대응되는 get()
를 제공하고 있기 때문입니다. (아래의 POST
방식과 마찬가지로 post()
메서드도 제공하고 있습니다.)
이 메서드를 사용해야하는 이유는 '?' 뒤에 들어가는 여러가지 값들을 객체의 형태로 전달할 수 있기 때문입니다. 현재도 body
의 값을 객체형태로 사용하고 있지만, handler
로 전달하기 위해서 JSON.stringfy
를 사용하여 직렬화하고 있지만,해당 메서드를 사용하게 되면 객체 형태를 그대로 사용하여 불필요한 작업을 제거할 수 있기 때문입니다. axios docs 링크를 남겨두었으니 한번 읽어보시는 것을 추천드립니다.
menus: [ | ||
{ | ||
name: 'SEARCH', | ||
href: '#/' |
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이 #
을 포함하고 있습니다. movie
라는 단어가 해당 페이지에 대한 의미를 내포하듯, URL에서 모든 문자는 페이지가 제공하는 정보를 의미하여야 하는데요. #
는 어떠한 의미를 가지고 있는지 설명해주실 수 있을까요?
🎬 영화 검색
주어진 API를 활용해 '완성 예시' 처럼 자유롭게 영화 검색 기능을 구현해보세요!
과제 수행 및 리뷰 기간은 별도 공지를 참고하세요!
배포페이지
영화 검색 사이트
요구사항
필수 요구사항은 꼭 달성해야 하는 목표로, 수정/삭제는 불가하고 추가는 가능합니다.
선택 요구사항은 단순 예시로, 자유롭게 추가/수정/삭제해서 구현해보세요.
각 요구사항은 달성 후 마크다운에서
- [x]
로 표시하세요.❗ 필수
❔ 선택
고찰
=> Scroll event와 IntersectionObserver API의 차이점 정리 + 다음에 사용해보기
1차 리팩토링(23.05.10)