refactor : useMutation으로 api 호출 방식 통일 #299

jonique98 · 2024-09-02T11:41:54Z

1️⃣ 어떤 작업을 했나요? (Summary)

resolved [REFACTOR] TanStack Query 라이브러리로 API 호출 상태를 선언적으로 관리하기 #262

기존 코드에 영향을 미치지 않는 변경사항

기존 코드에 영향을 미치는 변경사항

useMutation을 통해 api 호출 방식을 통일 했습니다.
juun이 얘기한 대로

  export const getAuthVerificationCheck = async (
    verificationCheckParams: VerificationCheckParams
  ): Promise<GetAuthVerificationCheckResponse> => {
    try {
      const res = await authClient.get('/auth/verification/check', {
        params: verificationCheckParams,
      });
      return { data: res.data };
    } catch (error: unknown) {
      return { error: error as AxiosError<AuthErrorData> };
    }
  };

    const res = await getAuthVerificationCheck({ session });

    if (res.data) {
      if (res.data.isVerified) onConfirm();
      else setError('이메일 인증을 완료해주세요.');
    } else if (res.error) {
      setError(res.error.response?.data.message || '이메일 인증 확인에 실패했습니다.');
    }
  };

try catch를 적용하면 promise가 reject되지 않기 때문에 errorBoundary를 랜더링 시킬 수 없습니다.

export const usePostAuthVerificationEmail = () => {
  return useMutation({
    mutationFn: postAuthVerificationEmail,
    onSuccess: (data) => {
      sessionStorage.setItem(STORAGE_KEYS.EMAIL_AUTH_SESSION_TOKEN, data.sessionToken);
      sessionStorage.setItem(
        STORAGE_KEYS.EMAIL_AUTH_SESSION_TOKEN_EXPIRED_IN,
        data.sessionTokenExpiredIn.toString()
      );
    },
    throwOnError: (error: AxiosError<AuthErrorData>) => {
      //이전과 같은 이메일일 경우 errorBoundary로 가지 않고 에러 처리
      if (error.response?.status === 400) return false;
      // 나머지는 errorBoundary로 처리
      return true;
    },
  });
};

수정된 코드입니다.
throwOnError를 통해 처리 가능한 에러 status 이외의 에러는 상위 컴포넌트인 errorBoundary를 랜더링 시키도록 했습니다.

버그 픽스

2️⃣ 알아두시면 좋아요!

emailForm과 emailAuth에는 중복 제출 방지를 위한 버튼 disable 로직이 존재합니다.
void인 mutate를 적용하면 await에서 실행 흐름을 기다리지 않기 때문에
isSubmitting같이 제출 상태를 체크하는 부분이 제대로 작동하지 않습니다.
때문에 mutateAsync를 통해 promise를 반환하도록 했습니다.

근데 throwOnError에서 status 잘 쓴건지 모르겠네요
혹시 throw하면 안되는 혹은 throw 해야하는(errorBoundary 띄워야 하는) status가 있는지 한번 봐주시면 감사하겠습니다

3️⃣ 추후 작업

리뷰 환영합니다.

4️⃣ 체크리스트 (Checklist)

main 브랜치의 최신 코드를 pull 받았나요?

nijuy

마참내......... 코멘트 쪼끔 남겨놨습니다 확인해주세용. 고생하셨습니다!! ^ㅅ^

nijuy · 2024-09-04T09:13:14Z

src/home/apis/authVerification.ts

tanstack query를 적용하면서 apis/authVerification 파일 내에 api 함수와 useMutation이 같이 위치하게 됐는데요.
useMutation은 home/hooks로 분리하는 게 폴더 구조상 적절해 보입니다!

아니면 같은 파일 내에 위치시키신 이유가 따로 있을까요?

저도 다른 API 호출 코드들과 통일성을 위해 apis 폴더 아래에는 axios를 이용한 API 호출 코드만 남기고 useMutation()을 반환하는 커스텀 훅은 home/hooks 아래로 위치시키는게 좋을 것 같습니다!

추가적으로 postAuthVerificationEmail(), getAuthVerificationCheck() 함수도 별도의 파일로 분리하는게 좋을 것 같네요.

c532f3a
분리 했습니다!

nijuy · 2024-09-04T09:14:32Z

src/home/apis/authVerification.ts

-  GetAuthVerificationCheckResponse,
-  PostAuthVerificationEmailResponse,


확인해보니까 이 두 타입은 이제 사용하는 곳이 없던데, 한번 확인해보시고 이번 PR에서 멸종 ㄱㄱ 어때요

c532f3a
멸종 시켰습니다
쏜애플 - 멸종 이라는 노래도 들어보세요

감미롭네요

owl1753

#300 과 작업 부분이 많이 겹치는 거 같은데 conflict 해결 가능하실까요??

2wndrhs

수고하셨습니다👏👏

추가적으로 #262 에서 언급한 postAuthSignUp() 부분도 tanstack query를 사용해서 API 호출 상태를 관리하면 좋을 것 같네요!

2wndrhs · 2024-09-16T07:39:13Z

src/home/apis/authVerification.ts

+      //이전과 같은 이메일일 경우 errorBoundary로 가지 않고 에러 처리
+      if (error.response?.status === 400) return false;
+      // 나머지는 errorBoundary로 처리
+      return true;


Suggested change

//이전과 같은 이메일일 경우 errorBoundary로 가지 않고 에러 처리

if (error.response?.status === 400) return false;

// 나머지는 errorBoundary로 처리

return true;

return error.response?.status !== 400

으로 줄일 수도 있을 것 같네요!

2wndrhs · 2024-09-16T07:57:59Z

src/home/components/SignupContents/EmailAuth/useEmailAuth.ts

+          setAuthed(false);
+          setError('인증 메일 재전송에 실패했습니다.');
+        },
+      }


Suggested change

}

onSettled: () => {

setEmailSending(false);

resetTimer();

},

다른 콜백들과의 일관성을 위해 mutation이 성공하거나 실패했을 때 호출되는 onSettled() 콜백에서 setEmailSending(), resetTime()를 호출하면 어떨까요?

추가적으로 onSettled() 콜백을 사용하면 동기적으로 mutation 결과를 기다릴 필요가 없으니 mutateAsync()도 mutate()로 바꿀 수 있을 것 같네요

onSettled 라는게 있군요
공식문서좀 읽어보는 습관을 들여야 하는데.. 감사합니다

mutateAsync에 관한 내용은 밑의 코멘트에 달아놓겠습니다!

2wndrhs · 2024-09-16T07:59:13Z

src/home/components/SignupContents/EmailForm/useEmailForm.ts

-      setEmailError(res.error?.response?.data.message || '이메일을 다시 확인해주세요.');
-    }
+
+    await postAuthVerificationEmailMutation.mutateAsync(


onSuccess(), onError() 함수는 mutation 결과에 따라 tanstack query가 동기적으로 호출해주니 mutateAsync()를 사용할 필요가 없을 것 같아요!

아주 좋은 방식인 것 같습니다! 다만 제가 mutateAsync를 사용한 이유는 React Hook Form의 isSubmitting 상태를 활용하여 버튼을 비활성화 하기 때문인데요.

const onEmailSubmit = async () => { const fullEmail = parseFullEmail(email); postAuthVerificationEmailMutation.mutate( { email: fullEmail, verificationType: 'SIGN_UP' }, { onSuccess: () => { onConfirm(fullEmail); }, onError: (error: AxiosError<AuthErrorData>) => { setEmailError(error.response?.data.message || '이메일을 다시 확인해주세요.'); }, } ); };

mutate는 비동기적으로 API를 호출하는 작업이 있지만, 반환값은 void입니다.
mutate 내부 로직까진 정확히 알지 못하지만, 일반적으로 비동기 작업 이후의 콜백 함수(onSuccess, onError 등)는 태스크 큐로 들어가고, onEmailSubmit 함수는 콜 스택에서 빠지며 실행 흐름이 종료됩니다.

export const EmailForm = ({ onConfirm }: EmailFormProps) => { const { email, emailError, onEmailSubmit, onChange } = useEmailForm({ onConfirm }); const { handleSubmit, formState: { isSubmitting }, } = useForm(); return ( <StyledSignupContentContainer onSubmit={handleSubmit(onEmailSubmit)}> <StyledSignupContentTitle>회원가입</StyledSignupContentTitle> {/* ... */} </StyledSignupContentContainer> ); };

RHF에서는 handleSubmit에 전달된 함수의 실행 상태에 따라 isSubmitting이 업데이트됩니다. 따라서 onEmailSubmit 함수가 실행을 완료하면 isSubmitting도 false(제출 완료)로 변경됩니다.

onEmailSubmit이 콜 스택에서 빠져나가면서 비동기 작업의 콜백 함수들이 태스크 큐에서 실행되고, 이로 인해 실행 흐름이 다음과 같이 진행될 가능성이 높습니다:

버튼을 누르자마자 제출이 완료되고 버튼은 활성화 상태

비동기 작업(API 호출) 진행

콜백 함수 동작에 따라 다음 컴포넌트로 이동하거나 에러 표시

mutateAsync를 사용하여 onEmailSubmit의 실행을 await하지 않으면, 중복 제출을 방지하기 위한 버튼 비활성화 기능이 의도대로 작동하기 힘들었습니다.

RHF의 formState를 사용하지 않고 별도의 상태를 관리하면 이 문제를 쉽게 해결할 수 있는데, 준이 코멘트 달아준 emailSending 부분이 그러한 방식인 것 같습니다. 혹시 이외에 더 좋은 방법이 있다면 알려주세요. 바로 적용해보겠습니다.

두 컴포넌트 모두 제가 작성한 것이 아니기 때문에 어떤 방식이 더 나은지는 판단하기 어려운데요. 아무튼 두 가지 방식 중 하나로 통일하는 것이 좋다고 생각해 코멘트를 남겼습니다.

오.. 자세한 설명 감사합니다👏👏

버튼을 disabled 상태로 만드는 것이 API 중복 호출을 방지하기 위함이라면 formState.isSubmitting 대신 mutation.isPending으로 버튼의 disabled 상태를 관리하면 어떨까요? (지금은 EmailForm 컴포넌트에서 postAuthVerificationEmailMutation이 선언되어 있지 않아서 EmailForm 컴포넌트에서 postAuthVerificationEmailMutation의 상태를 가져오려면 useMutationState()을 사용해야 될 것 같지만)

tkdodo씨는 mutateAsync()는 사용자에게 프로미스의 제어권을 넘기기 때문에 수동으로 프로미스 에러 처리를 해줘야 해서 mutate() 사용을 권장하더라구요

Since mutateAsync gives you control over the Promise, you also have to catch errors manually, or you might get an unhandled promise rejection.

https://tkdodo.eu/blog/mastering-mutations-in-react-query

코멘트에서도 말했듯 기능은 유사하지만 각자 사용하는 상태가 달라서 고민하고 있었는데, 아주 좋은 방법인 것 같아서 바로 반영했습니다!

추후에 리액트 훅 폼으로 폼 입력 및 에러 관리, mutation.isPending으로 제출 상태 관리 방식으로 통일하면 좋을 것 같습니다.

2wndrhs · 2024-09-16T08:01:05Z

src/home/components/SignupContents/EmailForm/EmailForm.tsx

+  const {
+    handleSubmit,
+    formState: { isSubmitting },
+  } = useForm();


react-hook-form으로 form 제출 상태를 관리하는게 좋네요👍

email error도 react-hook-form으로 관리할 수 있을 것 같은데 나중에 해보는 걸루..

좋습니다~

jonique98 · 2024-09-21T19:15:33Z

수정하면서 추후에 진행하면 좋을 것 같은 이슈가 있어서 의견 써봅니다.

useEmailVerification 과 useEmailAuth
useEmailInputForm 과 useEmailForm

위에 기재해놓은 hook들은 인증 메일 전송, 검증, 재전송에 사용되는 hook 들입니다.
각각 회원가입과 비밀번호 재설정에서 쓰고 있는데요.
회원가입과 비밀번호 재설정에서 사용하는 이메일 인증은 일단 서로 같은 api를 인자만 다르게 호출하는 형태기도 하고, 인증폼과 관련하여 공유할 수 있는 state도 많습니다.
때문에 하나로 통일하여 재사용하면 좋지 않을까 합니다.

또 세부적으로 봤을때 비밀번호 재설정쪽 hook만 react-hook-form을 활용했는데요.
아마 react hook form을 통일성있게 활용하기로 한 이후에 만들어져서 그런 것 같습니다.
그래서 통일한다면 juun이 코멘트 에서 말한 내용도 적용할 수 있을 것 같습니다.

일단은 이슈 범위를 조금 벗어나는 것 같아서 코멘트로 남깁니다!

nijuy

제롬이 말한 컨플릭트 해결이 아직인 거 같아서요!
아마 리뷰 과정에서 수정이 또 생길 수 있으니 머지 직전에 하시려던 건가? 싶기도 한데.. 이 부분만 잘 체크해주시면 될 거 같아요
고생하셨습니다~~~🤸‍♀️🤸‍♀️

@2wndrhs @owl1753 제가 오늘 배포할 거라 가능하면 오늘 안에 확인 부탁드립니다!
코드 리팩토링이니까 무조건 포함되어야 하는 건 아닌데 겸사겸사...... 같이 하면 좋으니께..........

jonique98 · 2024-09-22T05:50:50Z

@nijuy
아 네 머지 직전에 하려고 했는데, 아마 큰 수정사항 없을 것 같아서 충돌 해결해놨습니다!

2wndrhs · 2024-09-22T08:34:35Z

수정하면서 추후에 진행하면 좋을 것 같은 이슈가 있어서 의견 써봅니다.

useEmailVerification 과 useEmailAuth useEmailInputForm 과 useEmailForm

위에 기재해놓은 hook들은 인증 메일 전송, 검증, 재전송에 사용되는 hook 들입니다. 각각 회원가입과 비밀번호 재설정에서 쓰고 있는데요. 회원가입과 비밀번호 재설정에서 사용하는 이메일 인증은 일단 서로 같은 api를 인자만 다르게 호출하는 형태기도 하고, 인증폼과 관련하여 공유할 수 있는 state도 많습니다. 때문에 하나로 통일하여 재사용하면 좋지 않을까 합니다.

확실히 지금 훅들은 특정 컴포넌트에 강하게 결합되어 있고 재사용성도 떨어져 커스텀 훅으로 분리하는 효과가 많이 떨어지는 것 같네요(오히려 depth만 깊어져서 코드 파악하기가 어려운 느낌..)

특히 tanstack query와 react hook form을 사용해서 직접 관리해야 하는 상태들도 줄이면 훨씬 좋아질 것 같아요(e.g., error -> formState.error, emailSending -> mutation.isPending)

jonique98 linked an issue Sep 2, 2024 that may be closed by this pull request

[REFACTOR] TanStack Query 라이브러리로 API 호출 상태를 선언적으로 관리하기 #262

Closed

github-actions bot assigned jonique98 Sep 2, 2024

nijuy requested review from nijuy, owl1753 and ssolfa September 2, 2024 11:42

jonique98 changed the title ~~Refactor/#262 useMutation으로 api 호출 방식 통일~~ refactor : useMutation으로 api 호출 방식 통일 Sep 2, 2024

jonique98 added 3 commits September 3, 2024 10:39

feat: mutation을 사용하여 훅 분리

2620e77

feat: onSubmit에 mutateAsync적용

88464d8

feat: formState로 중복 제출 방지

b3e7f7d

jonique98 force-pushed the refactor/#262-auth-verification branch from 1932b02 to b3e7f7d Compare September 3, 2024 01:39

2wndrhs added the refactor label Sep 3, 2024

nijuy reviewed Sep 4, 2024

View reviewed changes

owl1753 reviewed Sep 4, 2024

View reviewed changes

2wndrhs reviewed Sep 16, 2024

View reviewed changes

jonique98 added 2 commits September 22, 2024 01:51

fix: hook 분리

c532f3a

fix: 비밀번호 재설정 훅의 api 호출 패턴 통일

3ebc6de

fix: postSignUpMutate 적용

2e2815e

jonique98 force-pushed the refactor/#262-auth-verification branch from 35dec61 to 2e2815e Compare September 21, 2024 19:26

nijuy requested review from owl1753, 2wndrhs and nijuy September 22, 2024 04:58

nijuy reviewed Sep 22, 2024

View reviewed changes

fix: resolve conflict

963953c

fix: isPending을 통해 버튼 disabled

fecf515

2wndrhs approved these changes Sep 22, 2024

View reviewed changes

nijuy approved these changes Sep 22, 2024

View reviewed changes

ssolfa approved these changes Sep 22, 2024

View reviewed changes

nijuy merged commit d525291 into develop Sep 22, 2024

nijuy deleted the refactor/#262-auth-verification branch September 22, 2024 11:01

2wndrhs mentioned this pull request Nov 23, 2024

fix: 메일 인증 시간 10초 -> 8분으로 변경 #308

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor : useMutation으로 api 호출 방식 통일 #299

refactor : useMutation으로 api 호출 방식 통일 #299

jonique98 commented Sep 2, 2024 •

edited

Loading

nijuy left a comment

nijuy Sep 4, 2024

2wndrhs Sep 16, 2024

jonique98 Sep 21, 2024

nijuy Sep 4, 2024

jonique98 Sep 21, 2024

nijuy Sep 22, 2024

owl1753 left a comment

2wndrhs left a comment •

edited

Loading

2wndrhs Sep 16, 2024

2wndrhs Sep 16, 2024

jonique98 Sep 21, 2024

2wndrhs Sep 16, 2024

jonique98 Sep 21, 2024 •

edited

Loading

2wndrhs Sep 22, 2024 •

edited

Loading

jonique98 Sep 22, 2024 •

edited

Loading

2wndrhs Sep 16, 2024

jonique98 Sep 21, 2024

jonique98 commented Sep 21, 2024 •

edited

Loading

nijuy left a comment

jonique98 commented Sep 22, 2024

2wndrhs commented Sep 22, 2024

		GetAuthVerificationCheckResponse,
		PostAuthVerificationEmailResponse,

-      }
+        onSettled: () => {
+          setEmailSending(false);
+          resetTimer();
+        },

refactor : useMutation으로 api 호출 방식 통일 #299

refactor : useMutation으로 api 호출 방식 통일 #299

Conversation

jonique98 commented Sep 2, 2024 • edited Loading

1️⃣ 어떤 작업을 했나요? (Summary)

기존 코드에 영향을 미치지 않는 변경사항

기존 코드에 영향을 미치는 변경사항

버그 픽스

2️⃣ 알아두시면 좋아요!

3️⃣ 추후 작업

4️⃣ 체크리스트 (Checklist)

nijuy 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

owl1753 left a comment

Choose a reason for hiding this comment

2wndrhs 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

jonique98 Sep 21, 2024 • edited Loading

Choose a reason for hiding this comment

2wndrhs Sep 22, 2024 • edited Loading

Choose a reason for hiding this comment

jonique98 Sep 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonique98 commented Sep 21, 2024 • edited Loading

nijuy left a comment

Choose a reason for hiding this comment

jonique98 commented Sep 22, 2024

2wndrhs commented Sep 22, 2024

jonique98 commented Sep 2, 2024 •

edited

Loading

2wndrhs left a comment •

edited

Loading

jonique98 Sep 21, 2024 •

edited

Loading

2wndrhs Sep 22, 2024 •

edited

Loading

jonique98 Sep 22, 2024 •

edited

Loading

jonique98 commented Sep 21, 2024 •

edited

Loading