Skip to content
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

[Api] axios, react-query setting #26

Merged
merged 30 commits into from
Jun 4, 2023
Merged

Conversation

hyeongrok7874
Copy link
Member

@hyeongrok7874 hyeongrok7874 commented May 30, 2023

개요 💡

client api 연동을 위한 axios, react-query setting

작업내용 ⌨️

Copy link
Member

@sunwoo0706 sunwoo0706 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

axios create로 만든 instance로 http 메서드마다 함수로 만들어 사용중인데, 현재 ts를 제대로 활용하지 못 한 것 같아서 한번 리뷰 받고 싶습니다

일단 기존 코드 코멘트는 드렸는데, ts를 제대로 활용하지 못하였다고 느끼신 부분을 더 자세히 알고 싶군요

현재 각 api를 useQuery를 이용하여 hook으로 만들어 구현했는데 현재 방식에 대해 리뷰 부탁드립니다!

좋습니다~

현재 react-query key 컨벤션에 대해서 리뷰 부탁드립니다!

좋습니다~

Comment on lines 26 to 30
<QueryClientProvider client={queryClient}>
<ReactQueryDevtools />
<GlobalStyle />
{children}
</QueryClientProvider>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

layout 에서 provider 컴포넌트를 사용하는게 일반적인 컨벤션인가요?

Copy link
Member Author

@hyeongrok7874 hyeongrok7874 Jun 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

react query - nextjs13

위 공식 문서 참고하시면 root layout에서 provider를 사용하는 것을 확인 하실 수 있습니다

단지 위 공식 문서와 제 방식의 차이점은

  • 공식 문서에서는 QueryClientProvider를 커스텀하여 모듈을 분리해 사용하고 있지만, 저는 별 다른 분리 없이 그대로 사용하였습니다
  • queryClient를 const [queryClient] = React.useState(() => new QueryClient()) 이런 형태로 선언하여,
    setter 함수를 선언하지 않음으로써 참조 동일성을 유지하고, 방식으로 작성되어 있습니다.

제가 이해한 공식문서에서 QueryClientProvider를 커스텀한 이유는 아래와 같습니다.

  • 'use client' 선언을 커스텀 QueryClientProvider 모듈에만 추가하기 위함
    • useState는 client component에서 사용 가능하기 때문 (server component에서는 사용할 수 없습니다.)
  • useState로 queryClient를 선언하기 위함
    • 참조 동일성을 유지하고, 서로 다른 사용자와 요청 간에 데이터를 공유하지 않고 구성 요소 수명 주기당 한 번만 QueryClient를 생성(공식 문서 인용)
  • <Hydrate> 사용을 위해 useState로 queryClient 선언이 필요하다.

현재 제 구성은 <Hydrate> 사용을 고려하지 못한 구성인 듯 하여 공식 문서대로 수정을 할 예정입니다.

Copy link
Member Author

@hyeongrok7874 hyeongrok7874 Jun 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 제 구성은 <Hydrate> 사용을 고려하지 못한 구성인 듯 하여 공식 문서대로 수정을 할 예정입니다.

4c98a2d

추후 <Hydrate> 사용을 위한 방식으로 수정 완료했습니다!

import axios from "axios";

export const instance = axios.create({
baseURL: "/api/client",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dev와 Prod의 base url이 동일한가요?

그리고 instance라는 네이밍은 너무 포괄적이군요

Copy link
Member Author

@hyeongrok7874 hyeongrok7874 Jun 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dev와 Prod의 base url이 동일한가요?

baseUrl 관리는 next.config.js에서 하고 있고,
현재는 test 서버만 존재하기 때문에 prod, dev baseUrl 조건식을 작성해놓지 않았습니다.

해당 설정은 클라이언트(프론트)의 주소 (ex. localhost:3000/api/client)로 요청을 보내도록 작성한 코드입니다.

next.config.jsrewrites 속성으로 인해,
클라이언트(프론트)/api/client/*로 요청 시, 클라이언트 서비스 서버/api/*로 요청이 rewrites 되게끔 구성되어 있는 상태입니다.

next-config-js/rewrites

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance라는 네이밍은 너무 포괄적이군요

https://axios-http.com/kr/docs/instance

공식 문서나 여러 소스 코드들을 살펴보았을 때, instance라는 네이밍이 일반적인 듯 하여 이렇게 작성했습니다!

좀 더 의도가 명확한 네이밍 (ex. clientInstance) 으로 변경하는게 아무래도 직관적인 듯 하여 수정하겠습니다!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좀 더 의도가 명확한 네이밍 (ex. clientInstance) 으로 변경하는게 아무래도 직관적인 듯 하여 수정하겠습니다!

b135342

수정 완료했습니다!

@hyeongrok7874 hyeongrok7874 merged commit e218acd into develop Jun 4, 2023
@hyeongrok7874 hyeongrok7874 deleted the feature/axiosSetting branch June 4, 2023 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants