-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: game api 관련 react query hook 작성 #128
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.
game api들 함수로 정리하고, 리액트 쿼리 훅으로 만드시느라 고생하셨습니다~!👏
PR 명 커밋 컨벤션에 맞게 수정 부탁드려요~!
그리고 저희가 컨벤션 정한 바에 따르면 게임 조회 리액트 쿼리들도 모두 파일 각각 하나로 만들어 사용하는 것이 맞다고 생각해요!
바쁘시겠지만 수정 부탁드립니다~!
src/api/game.ts
Outdated
return data; | ||
}; | ||
|
||
export const getGameById = async (postId: number) => { |
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.
지난번 회의에서 type들을 모두 type파일에 정리해서 사용하기로 컨벤션을 정했듯이! 저희 타입을 number로 명시하기 보다, type 파일에 정의한 타입들을 활용하면 좋을 것 같습니다😀
src/api/game.ts
Outdated
}; | ||
|
||
export const deleteGame = async (postId: number) => { | ||
const { data } = await axiosInstance.delete<string>( |
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.
return type 또한 마찬가지로 string으로 명시하기 보다 type 파일에 정의해서 사용하면 좋을 것 같습니다!
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.
수정 사항 확인했습니다~! 고생하셨어요!!
src/constants/api.ts
Outdated
GAME: (postId: number) => `/games/${postId}`, | ||
EDIT_GAME: (postId: number) => `/games/${postId}`, | ||
DELETE_GAME: (postId: number) => `/games/${postId}`, |
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.
이 부분도 type 파일에서 가지고 와서 type 정의해주실 수 있을까요?!
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.
제가 엔드 포인트를 빼놓고 수정했네요 🫢
새로 커밋했습니다 !
src/types/type.ts
Outdated
@@ -0,0 +1,2 @@ | |||
export type PostId = number; |
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.
PostId를 Id로 변수명을 변경하는 건 어떨까요?🤔
제가 댓글 쪽 api를 현재 작업하면서 든 생각입니다! 아래 사진과 같이 저는 commetId랑 postId를 모두 받아야 하는 부분이 있더라구요
처음에는 Pick<Post, 'id'>
, Pick<Comment, 'id'>
이런 식으로 type을 명시할까 생각했었습니다! 아름님께서 공통 타입으로 빼신 걸 보고 id 는 항상 number 타입이므로 Id라는 변수명으로 타입을 정의하면 편리할 것 같다고 생각이 들었어요! 어떠실까요?? @areumH @kwaksj329
그리고 파일명은 type보다 api에서 사용되는 타입이므로 api 라고 이름을 짓는 거는 어떨까요??
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.
오 Id로 변경하는 것 좋은 것 같아요!! 파일명도 api가 더 괜찮은 것 같네요 😮
수정님도 확인하시면 결정된 의견 반영하여 수정하겠습니다 !!
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.
수정사항까지 확인 완료했습니다! 고생 많으셨어요 👍🏻
src/types/api.ts
Outdated
export type Pageable = { | ||
page: number; | ||
size: number; | ||
sort: string[]; | ||
}; |
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.
Pageable 타입을 api파일에 정의하셨군요! 혹시 통일성을 위해서 pagination.ts 파일에 정의된 PaginationType도 api 파일로 옮겨주시거나 또는 Pageable 타입을 pagination.ts 에 옮겨주실 수 있을까요??
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.
pagination.ts 파일로 옮겨두었습니다!!
💡 작업 내용
💡 자세한 설명
useGetGameQuery.ts
파일에 모아두었습니다.useEditGameMutation.ts
훅은 Game 뿐만 아니라 postId도 함께 넘겨주어야 하기 때문에 한 번에 모두 넘겨주는 것 보다 postId를 먼저 넘겨준 뒤 Game 값들을 넘겨주는 것이 더 깔끔하다고 생각되어useMutation
함수를 바로 return 해주었습니다!예시)
+) 추가
Pageable
타입이 다른 api에도 쓰이는 듯 하여types/api.ts
파일에 정의해두었습니다.axiosInstance.delete
의 response data가 빈 값이라 response로 받아오도록 작성해두었습니다.📗 참고 자료 (선택)
📢 리뷰 요구 사항 (선택)
🚩 후속 작업 (선택)
✅ 셀프 체크리스트
closes #127