-
Notifications
You must be signed in to change notification settings - Fork 116
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
[윤병현] week15 #995
The head ref may contain hidden characters: "part3-\uC724\uBCD1\uD604-week15"
[윤병현] week15 #995
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
병현님 react hook form 사용해 폼 로직 잘 구현해주셨네요~ 몇가지 부분들 피드백 드렸어요
<ErrorMesage>필수 입력사항입니다.</ErrorMesage> | ||
)} | ||
{errors && <ErrorMesage text={error?.message?.toString()} />} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r:
errors
가 아니라error
가 되어야 할 것 같아요.- 앞에서 error가 truthy로 검증되었기 때문에 error에서 message 호출할 때는 체이닝하지 않아도 될 것 같습니다.
<ErrorMesage>필수 입력사항입니다.</ErrorMesage> | |
)} | |
{errors && <ErrorMesage text={error?.message?.toString()} />} | |
{errors && <ErrorMesage text={error.message?.toString()} />} |
/> | ||
{type === "password" && ( | ||
<ImgPosition onClick={toggleEyesButton}> | ||
{pwState ? ( | ||
<Image src={setPwOff} alt="Pw-off" /> | ||
) : ( | ||
<Image src={setPwOn} alt="Pw-on" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{text}{" "} | ||
<Link style={LinkStyle} href={`/`}> | ||
회원 가입하기 | ||
{linkText} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c: 회원 가입하기 링크의 href가 메인 페이지가 맞을까요? 👀
로고 정도만 컴포넌트로 분리하고 회원이 아니신가요와 회원가입하기 링크정도는 회원가입과 로그인 페이지 렌더링 코드부분
에 뒀으면 어땠을까 생각이 드네요. 어떤 컴포넌트를 분리하고 분리하지 않아야 유지보수에 좋은 코드가 될 수 있을지에 대해 한번 생각해보셨으면 좋겠습니다!
placeholder="비밀번호를 입력해 주세요" | ||
></Input> | ||
<LogoBox text="회원이 아니신가요?" linkText="회원 가입하기" /> | ||
<Form onSubmit={handleSubmit(onSubmit)}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r: 폼 요소의 에러 처리는 handleSubmit
의 두번째 인자인 onError
로 처리하면 함수의 책임이 분리되어 더 유연한 코드가 될 것 같아요.
<Form onSubmit={handleSubmit(onSubmit)}> | |
<Form onSubmit={handleSubmit(onSubmit, onError)}> | |
{/* 잘못된 값으로 폼 제출 시 `register` 함수에서 정의한 에러 메시지를 `onError`에서 전달받아 사용하기 /*} |
const res = await loginRequest(data); | ||
|
||
if (res) { | ||
const { data } = res; | ||
window.localStorage.setItem("accessToken", data.accessToken); | ||
router.push("/folder"); | ||
} else { | ||
setError("userEmail", { message: "이메일을 확인해주세요" }); | ||
setError("userPw", { message: "비밀번호를 확인해주세요" }); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c: await loginRequest(data)
가 성공하고 res
가 없는 경우가 있나요? 로그인 요청 실패 시 첫번째 라인에서 예외 발생해 else 절로 진입이 안될 것 같아요.
try {
const res = await loginRequest(data);
// loginRequest 실패 시 진입 불가능한 코드들
const { data } = res;
window.localStorage.setItem("accessToken", data.accessToken);
router.push("/folder");
} catch(e) {
// @todo 에러 처리
}
const res = await signupRequest(data); | ||
|
||
if (res) { | ||
const { data } = res; | ||
window.localStorage.setItem("accessToken", data.accessToken); | ||
router.push("/signin"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c: 사용자 인증/인가 로직은 커스텀 훅 등으로 분리해주세요. 다른 컴포넌트에서 쉽게 재사용할 수 있고 여러 인증 방식 추가에 유연하게 대응할 수 있습니다.
const res = await signupRequest(data); | |
if (res) { | |
const { data } = res; | |
window.localStorage.setItem("accessToken", data.accessToken); | |
router.push("/signin"); | |
} | |
try { | |
// API 요청, 토큰 저장 등을 분리 | |
await signup(data); | |
router.push("/signin"); | |
} catch { | |
// 예외 처리 | |
} |
} else if (response.status === 500) { | ||
console.log("서버에러"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c: 500 에러는 axios interceptor로 공통 에러 처리하면 좋겠네요!
if (response.status === 200) { | ||
const res = response.data; | ||
return res; | ||
} else if (response.status === 400) { | ||
console.log("없는 유저 정보"); | ||
} else if (response.status === 500) { | ||
console.log("서버에러"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r: 여기도 마찬가지로 진입 불가능한 코드가 있네요. axios에서 상태 코드 4~500번대이면 에러 발생해 catch절이 실행됩니다.
if (response.status === 200) { | |
const res = response.data; | |
return res; | |
} else if (response.status === 400) { | |
console.log("없는 유저 정보"); | |
} else if (response.status === 500) { | |
console.log("서버에러"); | |
} | |
const res = response.data; | |
return res; | |
// } else if (response.status === 400) { | |
// console.log("없는 유저 정보"); | |
// } else if (response.status === 500) { | |
// console.log("서버에러"); | |
// } |
2d784f7
into
codeit-bootcamp-frontend:part3-윤병현
미션 진행을 위해 머지합니다 |
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게