-
Notifications
You must be signed in to change notification settings - Fork 20
[선태영] Sprint 4 #54
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
base: basic-선태영
Are you sure you want to change the base?
[선태영] Sprint 4 #54
The head ref may contain hidden characters: "mission4-\uC120\uD0DC\uC601-api-request"
Conversation
…RUD with json format Updated README to reflect sprint mission details and project structure.
iamjaeholee
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.
스프린트 미션4 수행하느라 수고하셨어요 🥇
전반적으로 코드에서 여러 케이스에 대해 고려하여 설계를 해주신 것 같습니다.
main.js에서 테스트를 통해 API에 대한 검증도 매우 잘 해주신 것 같아요.
node_modules폴더는.gitignore파일에 추가하여 PR에 올리지 않으셔도 됩니다. 해당 파일은 용량이 너무 크기 때문에 보통 빌드시점에 필요한 부분만 같이 빌드되어 올라가는 형식으로 많이 작성이 됩니다. 현재는 static file로 구성되어 있기 때문에 제외해도 될 것 같습니다.axios사용을 위해서node_modules를 활용하는 환경을 구성해주신 것 같습니다.npm환경이 아니라도<script src="https://cdn.jsdelivr.net/npm/axios/dist/axios.min.js"></script>와 같은 cdn을 활용하시면axios모듈을 사용하실 수 있으니 참고 부탁드립니다 : -)
|
|
||
| //getArticleList() function with parameters: page, pageSize, keyword | ||
| export async function getArticleList(params = {}) { | ||
| const url = new URL(`https://panda-market-api-crud.vercel.app/articles`); |
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에 대해서 변수로 할당하여 재사용하는 것도 좋은 방법일 것 같습니다.
| export async function getArticleList(params = {}) { | ||
| const url = new URL(`https://panda-market-api-crud.vercel.app/articles`); | ||
| Object.keys(params).forEach((key) => | ||
| url.searchParams.append(key, params[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.
url searchParams를 활용해 동적으로 파라미터를 구성하는 방법 매우 좋습니다 : -)
| }); | ||
|
|
||
| //getArticleList() function with parameters: page, pageSize, keyword | ||
| export async function getArticleList(params = {}) { |
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.
한 파일에 getArticleList라는 이름의 함수가 여러개가 존재하여, 혼동되어 있는 것 같습니다. 스프린트 요구사항에 맞도록 Article API에서는 fetch와 then, catch를 활용한 함수만 작업하시면 될 것 같다는 의견입니다 !
| const res = await fetch('https://panda-market-api-crud.vercel.app/articles'); | ||
| if (!res.ok) { | ||
| throw new Error(`HTTP error! status: ${response.status}`); | ||
| } | ||
| const data = await res.json(); | ||
|
|
||
| Promise.resolve(data) | ||
| .then(processedData => { | ||
| console.log(`Data received and processed:`, processedData); | ||
| }).catch(err => { | ||
| console.error("Error during data processing:", err); | ||
| }); |
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.
현재는 async await을 활용해 비동기 처리를 한 번 진행한 후 결과값에 대해서 다시 Promise로 생성하여 then, catch메서드를 활용해주고 계세요.
저는 async await을 사용하지 않고 바로 Promise상태에서 then, catch메서드를 활용해주시면 좋을 것 같다는 의견입니다.
| export async function getProductList(params = {}) { | ||
| const res = await instance.get( | ||
| '/products', { | ||
| params | ||
| }); | ||
| //return res.data; | ||
| } | ||
|
|
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.
Product API에서도 마찬가지로 getProductList라는 함수명이 여러개가 선언되어 있는 것 같아서 export시에 아마 한 함수만 export가 될 것으로 보입니다.
그런 부분에 대한 정리가 필요해 보입니다. !
|
conflict 해결해주시고, |
요구사항
기본
스프린트 4 미션에 적힌 디렉션에 따라 api 명세서를 활용하여
fetch와 axios 를 활용한 동기, 비동기 request를
구현하였습니다.
다만, 현재 axios 메소드를 활용한 코드들은 모두 nodejs 환경에서만 작동하는 것으로
nodejs가 설치되지 않은 호스팅 서버환경에서는 작동하지 않음을 고지해야 한다는 점을
이해해야 합니다.
심화
주요 변경사항
스크린샷
멘토에게