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

Featuring : 로그인 API 구현 #141

Merged
merged 8 commits into from
Jul 28, 2023
Merged

Featuring : 로그인 API 구현 #141

merged 8 commits into from
Jul 28, 2023

Conversation

guesung
Copy link
Member

@guesung guesung commented Jul 27, 2023

💡 왜 PR을 올렸나요?

  • 신규 피처

💁 무엇이 어떻게 바뀌나요?

  1. api 유저 플로우에 따라 인증번호 인증 후 > login api를 쏴 유저라면(response.existUser가 true라면) 토큰 설정 & home으로 이동하고, 유저가 아니라면 다음 페이지(회원가입 2페이지)로 이동합니다.
  • 토큰 세팅도 하려했으나, 아직 서버에서 토큰을 날려주지 않아 구현못하였습니다.

💬 리뷰어분들께

@guesung guesung requested a review from kangju2000 as a code owner July 27, 2023 07:17
@github-actions github-actions bot added this to the 7월 2주차 Sprint milestone Jul 27, 2023
@github-actions
Copy link

Labeler가 제목과 설명에 있는 특별한 텍스트와 일치하는 레이블을 적용했습니다.
Label을 검토하고 필요한 변경 사항을 적용해 주세요.

@github-actions github-actions bot added the Feature 기존 페이지/컴포넌트에서 기능을 추가한 경우 label Jul 27, 2023
Copy link
Contributor

@kangju2000 kangju2000 left a comment

Choose a reason for hiding this comment

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

LGTM👍

Comment on lines +31 to +42
mutateLogin(
{ phoneNumber: data.phoneNumber },
{
onSuccess: (response: LoginResponse) => {
if (response.existUser) {
// TODO : 토큰 설정 & home으로 이동
} else {
nextStep();
}
},
}
);
Copy link
Contributor

Choose a reason for hiding this comment

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

p2)useMutation의 option 값을 props로 받아서 처리하면 어떨까요?

지금은 mutate를 호출할 때 성공 로직을 적고 있는데 이렇게 되면 여러곳에서 mutate를 호출할 때마다 각각 로직을 작성해야 하는 번거로움이 있을 것 같아요.

커스텀 mutation 훅을 호출할 때 onSuccess 같은 핸들러 로직을 작성하게 하는건 어떨까요?

const { mutate: mutateLogin } = useLoginMutation({
  onSuccess: handleSuccess
});

다른 프로젝트 예 : 커스텀 mutation, 사용

다른 사용법

Copy link
Member Author

Choose a reason for hiding this comment

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

이런 식으로도 로직 분리가 가능하군요. 위 링크에서는 공통적인 처리 로직이 있는 것으로 보입니다.

위 로그인의 경우 아래와 같이 로직을 분리해보았습니다.

  const handleVerifySuccess = () => {
    mutateLogin(
      { phoneNumber: getValues().phoneNumber },
      {
        onSuccess: (response: LoginResponse) => {
          if (response.existUser) {
            // TODO : 토큰 설정 & home으로 이동
          } else {
            nextStep();
          }
        },
      }
    );
  };

  const { mutate: mutateSMSVerify } = useSMSVerifyMutation({
    onSuccess: handleVerifySuccess,
  });
  const { mutate: mutateLogin } = useLoginMutation();

공통적으로 적용되는 로직이 아닌지라 다소 복잡해보이는데 어떠신가요 ??

Copy link
Contributor

Choose a reason for hiding this comment

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

mutateLogin도 똑같이 훅의 인자로 onSuccess 처리해야하지 않을까요?

Copy link
Contributor

@kangju2000 kangju2000 Jul 27, 2023

Choose a reason for hiding this comment

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

SMSVerify 성공 -> Login 요청이 항상 동일하게 적용된다면 커스텀 훅 만든 곳에서 onSuccess 처리해도 될 것 같은데 어떻게 생각하시나요?

사용하는 쪽의 내부 상태를 바꿔야하는 경우가 아닐 때는 커스텀 훅 쪽에서 처리해도 좋을것 같아요! invalidQueries 같은 것들도 훅 쪽에서 처리하도록하면 사용할 때는 캐시 무효화 세팅 안해도 되는 장점도 있겠네용

@guesung guesung changed the base branch from feature/use-funnel-join-page to develop July 28, 2023 02:00
@guesung guesung merged commit 7d1a3d5 into develop Jul 28, 2023
1 check passed
@guesung guesung deleted the feature/add-login-api branch July 28, 2023 05:32
guesung added a commit that referenced this pull request Jul 12, 2024
guesung added a commit that referenced this pull request Jul 17, 2024
guesung added a commit that referenced this pull request Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature 기존 페이지/컴포넌트에서 기능을 추가한 경우
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants