apiClient 변경 및 기존 customFetch 함수 제거 #929

Jaymyong66 · 2024-11-21T08:05:15Z

⚡️ 관련 이슈

[FEAT] 에러 핸들링과 에러 바운더리의 reset #439

📍주요 변경 사항

기존 customFetch로 사용되던 api 요청을 모두 apiClient로 변경하였습니다.

그에 따라 api 함수에서는 에러 핸들링 로직에 대한 관심사 분리를 하였습니다.
또 반환 타입은 queries 훅에서 받아서 쓰도록 변경되었습니다. 해당 부분은 api 함수가 더 편할거 같다면 알려주세요.
ex) api 폴더의 요청 함수에는 반환값 타입이 없습니다. tanstack-query 훅 부분에서 타입 지정을 하여 사용합니다.

MSW handler에서의 API_URL을 config.ts에서 export하여 사용합니다. 따라서 URL이 변경된다면 각 api 폴더 하위 파일의 실제 요청 URL과 MSW URL 모두 변경할 필요 없이, 관리 포인트가 config.ts 한 곳에서 관리 가능합니다.
getErrorMessage 함수를 회원가입 시 아이디 중복인 경우에는 당장 필요해서 한가지 상황만 구현해놓았습니다. 순차적으로 errorCode와 instance를 고려해 사용자에게 보여줄 메시지와 디버깅을 위한 에러 메시지를 분리하고자 합니다.

🍗 PR 첫 리뷰 마감 기한

11/22 23:59

GetMemberNameResponse 타입 제거, Error 로직 책임 분리

Jaymyong66 · 2024-11-21T08:06:40Z

frontend/src/api/like.ts

-export const postLike = async ({ templateId }: LikePostRequest) => {
-  const response = await customFetch<HttpResponse>({
-    method: 'POST',
-    url: `${LIKE_API_URL}/${templateId}`,
-  });

-  return response;
-};
+import { apiClient } from './config';

-export const deleteLike = async ({ templateId }: LikeDeleteRequest) => {
-  const response = await customFetch<HttpResponse>({
-    method: 'DELETE',
-    url: `${LIKE_API_URL}/${templateId}`,
-  });
+export const postLike = async (templateId: number) => await apiClient.post(`${END_POINTS.LIKES}/${templateId}`, {});


해당 부분에서 templateId가 객체로 전달되고 있었는데, 굳이 객체일 필요가 없어서 변경했습니다.

Jaymyong66 · 2024-11-21T08:08:33Z

frontend/src/api/templates.ts

-export const editTemplate = async ({ id, template }: { id: number; template: TemplateEditRequest }): Promise<void> => {
-  await customFetch({
-    method: 'POST',
-    url: `${TEMPLATE_API_URL}/${id}`,
-    body: JSON.stringify(template),
-  });
-};
+export const editTemplate = async (template: TemplateEditRequest) =>
+  await apiClient.post(`${END_POINTS.TEMPLATES_EXPLORE}/${template.id}`, template);


id와 template을 TemplateEditRequest라는 타입으로 묶어도 문제 없을 것이라 생각해 TemplateEditRequest 타입에 id를 추가해 타입을 합쳤습니다.

생각해보니 id도 해당 템플릿의 id이기 때문에 묶어도 괜찮을 것 같네요👍👍

healim01

안녕하세요 마위~!
코드 리팩토링 잘 봤습니다...~~!

~~절대 경로를 수정 전에 알았다면 더 좋았겠지만..~~ 그래도 변경된 사안이 더 좋네요

healim01 · 2024-11-21T09:05:48Z

frontend/src/mocks/handlers.ts

혹시 핸들러 파일을 분리하는 것에 대해서 마위님은 어떻게 생각하시는지 궁금합니다!

해당 파일의 길이가 꽤 긴 점 + 도메인이 다른 여러 분야의 핸들러가 섞여있는거 같아서 분리 후 핸들러에서는 handlers 만 관리하는 것에 대해서 어떻게 생각하시는지 궁금합니다~!

저도 하고 싶습니닷! 다만 해당 PR에서 분리해도 리뷰하시기 괜찮을지 여쭤보고 싶네요~! 괜찮을까요?

넹 저도 분리하는게 좋다고 생각합니당~!

healim01 · 2024-11-21T09:06:49Z

frontend/src/api/ApiClient.ts

 import { ApiError } from './Error/ApiError';
+import { getErrorMessage } from './Error/getErrorMessage';
 import { HTTP_STATUS } from './Error/statusCode';


절대 경로 다 고쳤는데,,,, 또르륵 ㅜ

healim01 · 2024-11-21T09:07:21Z

frontend/src/api/authentication.ts


-const API_URL = process.env.REACT_APP_API_URL || 'https://default-url.com';
+export const postSignup = async (signupInfo: SignupRequest) => await apiClient.post(END_POINTS.SIGNUP, signupInfo);


코드가 확연히 줄었네요 매우 좋습니다~! 👍👍👍

healim01 · 2024-11-22T04:27:43Z

frontend/src/api/config.ts

config 파일은 index.ts 에 넣지 않으신 이유가 따로 있을까요?

깜빡했네요 ㅜㅜ 넣겠습니닷! 감사합니다

healim01 · 2024-11-22T04:31:19Z

frontend/src/api/authentication.ts

 export const getLoginState = async () => apiClient.get(END_POINTS.LOGIN_CHECK);



이 파일에 get 요청에는 바로 apliClient 를 리턴하고 다른 파일에는 json 으로 리턴하네요? 차이가 있는 걸까요?

해당 요청에 대한 응답이 없고, 사용하지 않기 때문입니다. 혹시 일관성을 위해 구조를 통일해야할까요?

흠,,, 이건 개발자 취향일거 같긴하지만,,, 전 일관성과 통일감을 위해 통일하는 편이긴합니다...

저도 평소 통일성을 위해 일관된 반환값을 선호하는 편이지만, 지금 이 경우에는 애초에 서버에서 오는 반환값이 없기 때문에 json()이 아예 불가능한 상황이라고 생각하였습니다. 그래서 지금 이대로도 괜찮을 것 같아요..!

Hain-tain

이렇게 모두 수정하니 기존 customFetch를 썼던 코드보다 훨씬 간결하고 명확해진 것 같네요!! 너무 고생하셨습니다!! 👍👍👍

전반적으로 소소한 코멘트들이라 Approve 남겼습니다. 리뷰 확인 후 코멘트 달아주시면 바로 머지하러 올게요!!

이번 PR 리뷰 중 추후 보강되어야 한다고 생각한 부분

핸들러 파일 분리 (apiClient 변경 및 기존 customFetch 함수 제거 #929 (comment) 참조)
END_POINTS와 ROUTE_POINT 분리 및 적용 (apiClient 변경 및 기존 customFetch 함수 제거 #929 (comment) 참조)
에러 코드 및 각 코드에 대한 에러 메세지 상수 분리 (apiClient 변경 및 기존 customFetch 함수 제거 #929 (comment) 참조)

Hain-tain · 2024-11-22T12:42:42Z

frontend/src/api/Error/getErrorMessage.ts

에러 반환 함수 만들어주셨군요👍
다음에 에러 코드 상수로 분리하면서, 에러 메세지에 대해서도 같이 논의 + 상수 분리 하면 좋을 것 같네요!!👍

Hain-tain · 2024-11-22T13:02:42Z

frontend/src/api/templates.ts

-export const editTemplate = async ({ id, template }: { id: number; template: TemplateEditRequest }): Promise<void> => {
-  await customFetch({
-    method: 'POST',
-    url: `${TEMPLATE_API_URL}/${id}`,
-    body: JSON.stringify(template),
-  });
-};
+export const editTemplate = async (template: TemplateEditRequest) =>
+  await apiClient.post(`${END_POINTS.TEMPLATES_EXPLORE}/${template.id}`, template);


생각해보니 id도 해당 템플릿의 id이기 때문에 묶어도 괜찮을 것 같네요👍👍

Hain-tain · 2024-11-22T13:09:43Z

frontend/src/api/authentication.ts

 export const getLoginState = async () => apiClient.get(END_POINTS.LOGIN_CHECK);



저도 평소 통일성을 위해 일관된 반환값을 선호하는 편이지만, 지금 이 경우에는 애초에 서버에서 오는 반환값이 없기 때문에 json()이 아예 불가능한 상황이라고 생각하였습니다. 그래서 지금 이대로도 괜찮을 것 같아요..!

Hain-tain · 2024-11-22T13:12:52Z

frontend/src/queries/authentication/useLoginMutation.ts

        successAlert('로그인 성공!');
+        navigate(END_POINTS.memberTemplates(memberId));


일단 지금은 이렇게 두지만 추후 ROUTE_END_POINT로 바꿔주시면 좋을 것 같아요!!

서버에 요청을 보내는 경우 사용하는 경로 => END_POINTS
클라이언트에서 네이게이션을 위해 사용하는 경로 => ROUTE_END_POINT (또는 네이밍을 ROUTE_POINT 등으로 바꿔도 좋겠네요)

Hain-tain · 2024-11-22T13:13:46Z

frontend/src/queries/authentication/useLoginMutation.ts

+    onError: () => {
+      handleLoginState(false);
+      handleMemberInfo({ memberId: undefined, name: undefined });
      failAlert('로그인에 실패하였습니다.');
    },


혹시 이 경우에 로컬스토리지에 저장되는 유저 정보를 다시 비워주지 않아도 되나요?

우선 기존 로직을 변경을 하지 않았는데, 로그인 실패라는 것은 기존에 저장된 값이 없다는 것 같아요.
우려되는 상황이 아이디 두개로 로그인할 때, 데이터의 불일치라면 이 부분보다 loginCheck하는 부분에서 응답으로 쿠키에 해당하는 유저 정보를 받고 로컬에 저장된 유저 정보를 받아 일치하는지를 확인해야할 것 같네요.

다만, 이렇게 memberInfo를 초기화시켜 주는 과정은 로컬 스토리지와 일치 시켜줘야해서 따로 하나의 함수로 분리하거나 리팩토링이 필요해보이는게 맞네요

현재 백엔드에서 진행중인 인증 정보를 헤더 토큰으로 바꾸는 작업을 하면서 같이 개선해볼게요

Hain-tain · 2024-11-22T13:19:40Z

frontend/src/mocks/handlers.ts

핸들러 파일 분리는 저도 너무 찬성이에요!
하지만 현재 이번 PR의 주요 목적은 apiClient 적용이기도 하고, 현재 코드 리뷰 결과 크게 변경되어야 할 부분이 없으면 오늘 내로 머지가 가능할 것 같아서...

만약 빠르게 작업이 가능하다면 이번 PR에서 함께 분리하는 걸로 하고, 만약 어려울 것 같아면 핸들러 분리 부분은 별도의 PR에서 진행해보는건 어떨까요?? 핸들러 분리 이외에도 추후 보강되어야 하는 부분은 많으니까 그것들 묶어서 작업 단위로 가져가도 좋을 것 같습니다.

Jaymyong66 added 17 commits November 21, 2024 12:09

refactor(ApiClient): apiClient 인스턴스를 config 파일에서 선언

e5af0d5

refactor(categories): 카테고리 get, post, put - apiClient로 변경

61b5bd3

refactor(api): 카테고리 delete - apiClient로 변경

8192cc6

refactor(mocks): handlers - 카테고리 url을 config의 API_URL과 END_POINTS를 활용

bd20e68

feat(mocks): handlers - member name GET에 대한 핸들러 추가

182341a

refactor(api): members - apiClient로 변경

433ae91

GetMemberNameResponse 타입 제거, Error 로직 책임 분리

refactor(mocks): handlers - MEMBER_API_URL 의존성 제거

a89d255

refactor(api): tags - apiClient로 변경, MSW handler - tags의 API_URL 의존성 변경

2cb566d

refactor(api): 좋아요 POST, DELETE - apiClient로 변경, handler의 API_URL 의존성 변경

8b75c4d

refactor(api): templates - get, post, edit, delete 요청을 apiClient로 변경

ceb82d1

refactor(mocks): �TEMPLATE_API_URL 요청 경로 의존성 제거

c718c2a

refactor(api): authentication - 회원가입, 로그인 요청을 apiClient로 변경

2e18300

refactor(api): 로그아웃 요청을 apiClient로 변경

1518d28

feat(Error): errorCode, instance를 통해 getErrorMessage 함수 적용

5efa301

refactor(api): authentication - checkName 함수를 apiClient로 변경

e20477b

refactor(mocks): handlers - authentication 관련 엔드 포인트의 API_URL 의존성 변경

7caab2a

refactor(api): customFetch 제거, 사용하지 않는 URL 변수 제거

cf71479

Jaymyong66 added FE 프론트엔드 refactor 요구사항이 바뀌지 않은 변경사항 labels Nov 21, 2024

Jaymyong66 added this to the 7차 스프린트 💭 milestone Nov 21, 2024

Jaymyong66 requested review from healim01 and Hain-tain November 21, 2024 08:05

Jaymyong66 self-assigned this Nov 21, 2024

Jaymyong66 commented Nov 21, 2024

View reviewed changes

healim01 reviewed Nov 22, 2024

View reviewed changes

refactor(api): API_URL을 index에서 export

f748d07

healim01 approved these changes Nov 22, 2024

View reviewed changes

Hain-tain approved these changes Nov 22, 2024

View reviewed changes

Hain-tain merged commit f11f763 into dev/fe Nov 22, 2024
5 checks passed

Hain-tain deleted the refactor/439-api-client branch November 22, 2024 15:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

apiClient 변경 및 기존 customFetch 함수 제거 #929

apiClient 변경 및 기존 customFetch 함수 제거 #929

Jaymyong66 commented Nov 21, 2024 •

edited

Loading

Jaymyong66 Nov 21, 2024

Jaymyong66 Nov 21, 2024

Hain-tain Nov 22, 2024

healim01 left a comment

healim01 Nov 21, 2024

Jaymyong66 Nov 22, 2024

healim01 Nov 22, 2024

healim01 Nov 21, 2024

healim01 Nov 21, 2024

healim01 Nov 22, 2024

Jaymyong66 Nov 22, 2024

healim01 Nov 22, 2024

Jaymyong66 Nov 22, 2024

healim01 Nov 22, 2024

Hain-tain Nov 22, 2024

Hain-tain left a comment •

edited

Loading

Hain-tain Nov 22, 2024

Hain-tain Nov 22, 2024

Hain-tain Nov 22, 2024

Hain-tain Nov 22, 2024

Hain-tain Nov 22, 2024

Jaymyong66 Nov 23, 2024

Hain-tain Nov 22, 2024 •

edited

Loading


		const API_URL = process.env.REACT_APP_API_URL \|\| 'https://default-url.com';
		export const postSignup = async (signupInfo: SignupRequest) => await apiClient.post(END_POINTS.SIGNUP, signupInfo);

		export const getLoginState = async () => apiClient.get(END_POINTS.LOGIN_CHECK);

		successAlert('로그인 성공!');
		navigate(END_POINTS.memberTemplates(memberId));

apiClient 변경 및 기존 customFetch 함수 제거 #929

apiClient 변경 및 기존 customFetch 함수 제거 #929

Conversation

Jaymyong66 commented Nov 21, 2024 • edited Loading

⚡️ 관련 이슈

📍주요 변경 사항

🍗 PR 첫 리뷰 마감 기한

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

healim01 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hain-tain left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hain-tain Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Jaymyong66 commented Nov 21, 2024 •

edited

Loading

Hain-tain left a comment •

edited

Loading

Hain-tain Nov 22, 2024 •

edited

Loading