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

Refactor/api provider #149

Merged
merged 6 commits into from
Dec 6, 2023
Merged

Refactor/api provider #149

merged 6 commits into from
Dec 6, 2023

Conversation

Lukaid-dev
Copy link
Collaborator

@Lukaid-dev Lukaid-dev commented Dec 5, 2023

Checklist

  • Code Review: 작성한 코드를 다시 한 번 꼼꼼이 확인했나요?
  • Testing: 앱이 잘 구동되는지 개발한 기능이 문제 없이 작동하는지 확인했나요?
  • Remove: print나 주석 등 필요없는 코드를 삭제했나요?
  • Rebase: (필요시) rebase를 완료했나요?
  • Conflict Resolution: 충돌을 해결하는 과정을 거쳤나요?

Description

원래는 브랜치 이름대로 ApiProvider를 도입하려 했지만, 개발과정에서 ApiProvider를 도입하는게 오히려 더 번거로워진다는 것을 깨닫고 철회

그래서 axios client를 공통적으로 사용하는 옵션을 추가해서 넣고, api를 통합해서 관리하도록 변경함

Changes Made

대부분의 변경이 axios사용에서 apiClient사용으로 변경 된 것입니다. 아래는 특별히 알아둬야 하는 것들만 기입했습니다.

client/src/apis/apiClient.ts

  • axiox 객체 만들어서 기본 세팅해줌

client/src/components/buttons/RoomCreateButton.tsx

  • alert를 띄우는 로직을 api쪽으로 이관 / 에러처리의 일환

client/src/contexts/AuthProvider.tsx

  • AuthContext -> AuthProvider
  • 기존의 모듈이름을 조금 더 역할에 맞게 수정
  • 타입 수정

@Lukaid-dev Lukaid-dev requested review from glowisn and kiuuon December 5, 2023 05:39
@Lukaid-dev Lukaid-dev self-assigned this Dec 5, 2023
Copy link
Collaborator

@glowisn glowisn left a comment

Choose a reason for hiding this comment

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

에러처리를 apis 파일들 안에서 한번 하기로 하지 않았나요?🙃
createRoom은 되있는데 나머지 것들도 저런 로직으로 되면 좋겠네요.

axios 예제를 보니 get. then. catch. 형식의 요청을 많이 쓰는거 같더라구요

@Lukaid-dev
Copy link
Collaborator Author

Lukaid-dev commented Dec 5, 2023

@glowisn

에러처리를 apis 파일들 안에서 한번 하기로 하지 않았나요?🙃 createRoom은 되있는데 나머지 것들도 저런 로직으로 되면 좋겠네요.

axios 예제를 보니 get. then. catch. 형식의 요청을 많이 쓰는거 같더라구요

네 지금 다른 PR develop에 머지되길 기다리고 있고, 지금까지 작성된 api 전부 모아서 한방에 고치려구요! 다른 api들 적용되는 형태 보고 고민 좀 해보려고 했습니다

일단은 돌아가는 것만 파악했고, 에러 핸들링은 이후 커밋에 추가할거에요!

@glowisn
Copy link
Collaborator

glowisn commented Dec 5, 2023

에러처리를 apis 파일들 안에서 한번 하기로 하지 않았나요?🙃 createRoom은 되있는데 나머지 것들도 저런 로직으로 되면 좋겠네요.
axios 예제를 보니 get. then. catch. 형식의 요청을 많이 쓰는거 같더라구요

네 지금 다른 PR develop에 머지되길 기다리고 있고, 지금까지 작성된 api 전부 모아서 한방에 고치려구요! 다른 api들 적용되는 형태 보고 고민 좀 해보려고 했습니다

아 좋습니다! 그거는 한번에 하는게 편하겠네요!

@Lukaid-dev Lukaid-dev marked this pull request as ready for review December 5, 2023 09:32
@Lukaid-dev Lukaid-dev requested a review from glowisn December 5, 2023 09:33
Copy link
Collaborator

@glowisn glowisn left a comment

Choose a reason for hiding this comment

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

LGTM!

@Lukaid-dev Lukaid-dev merged commit d88cdce into develop Dec 6, 2023
1 check passed
@Lukaid-dev Lukaid-dev deleted the refactor/ApiProvider branch December 6, 2023 01:41
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.

2 participants