-
Notifications
You must be signed in to change notification settings - Fork 44
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
[우지석] Week14 #448
The head ref may contain hidden characters: "part3-\uC6B0\uC9C0\uC11D-week14"
[우지석] Week14 #448
Conversation
…ithub-actions [Fix] delete merged branch github action
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.
지난주 분량까지 같이 작업해서 제출하시느라고 수고 많으셨습니다!!
약간 누락된 부분은 시간되면 진행해보세요 화이팅!
@@ -0,0 +1,76 @@ | |||
// components/EmailInput.tsx |
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.
파일명 보면 알 수 있는 부분이라 없어도 되는 주석인 것 같아요!!
export const emailCheck = (email: string) => { | ||
const emailForm = | ||
/^[0-9a-zA-Z]([-_.]?[0-9a-zA-Z])*@[0-9a-zA-Z]([-_.]?[0-9a-zA-Z])*\.[a-zA-Z]{2,3}$/i; | ||
return emailForm.test(email); | ||
}; |
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.
보통 요런 유틸성 함수는 별도 utils 폴더에 분리해서 관리하곤 합니다!
지금 emailCheck 함수를 export 하고 있는데, 다른 파일에서도 사용하고 있는 것 같아요
그렇다면 더더욱 분리하시는 것을 추천드립니다!
리액트 컴포넌트 파일에는 보통 리액트 컴포넌트 하나만 default export 하는 식으로 작성합니다
}; | ||
|
||
export function EmailInput({ value, onChange, error }: EmailInputProps) { | ||
const cx = classNames.bind(styles); |
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.
요거는 컴포넌트 렌더링 될 때마다 매번 다시 선언되지 않아도 되는 코드라서 컴포넌트 바깥쪽에 두셔도 돼요
저는 보통 import 문과 컴포넌트 사이에 둡니다
return ( | ||
<div className={cx("input__section")}> | ||
<label className={cx("text")}> | ||
이메일 <br /> |
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.
불필요해보이는 br 이네요!
const cx = classNames.bind(styles); | ||
const [email, setEmail] = useState(""); | ||
const [password, setPassword] = useState(""); | ||
const [emailError, setEmailError] = useState(""); | ||
const [passwordError, setPasswordError] = useState(""); | ||
const router = useRouter(); | ||
const handleSubmit = async (e: React.FormEvent) => { |
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.
다른 파일들처럼 적당히 빈 줄을 추가해주면 더 읽기 편할 것 같아요!
const emailChecked = emailCheck(email); | ||
const validPassword = isValidPassword(password); |
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.
emailCheck , isValidPassword 둘 다 동일하게 유효성 검증을 하는 함수인데
네이밍 방식도 다르고 사용한 단어 (valid, check) 도 달라서 좀 헷갈리는 것 같아요
멘토링 때 말씀드린대로 함수는 동사로 시작하게 작성해보시면 좋을 것 같고,
동일한 역할을 하는 함수라면 동일한 단어를 사용해서 일관성 있게 작성하면 더 좋을 것 같습니다!
함수의 리턴값이 boolean 형이니 변수도 boolean 형에 맞게 지어주시면 좋을 것 같고요!
const emailChecked = emailCheck(email); | |
const validPassword = isValidPassword(password); | |
const isEmailValid = validateEmail(email); | |
const isPasswordValid = validatePassword(password); |
const passwordMatch = password === confirmPassword; | ||
|
||
if (emailChecked && validPassword && passwordMatch) { | ||
const checkEmailUrl = "https://bootcamp-api.codeit.kr/api/check-email"; |
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.
요거는 submit 시점이 아니라 email 인풋 blur 시점에 확인해야 하는 것 같아요! (요구사항 확인)
checkEmailData | ||
); | ||
if (checkEmailResponse.status === 409) { | ||
setEmailError("이미 사용 중인 이메일입니다."); |
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.
스크린샷처럼 signup페이지에서 중복된 이메일을 적어서 폼제출을 했을때 409에러는 발생하는데
setEamilError가 실행이 안되는같아요.....
배포 url 이 있어서 확인이 가능했습니다!!
409 에러 났을 때 콘솔에 'hello2' 가 찍히는 걸 보니 47라인이 실행됐다는 건데요
그 말은 error 가 나서 catch 구문으로 넘어갔다는 거겠죠?
27~30 라인에서 axios 요청하면서 에러가 났기 때문에 31라인 이후로 진행되지 않고 46라인으로 이동한거에요
- catch 문에서 error의 status 가 409 인 경우 처리를 해주는 식으로 처리하시면 될 것 같습니다!
- 지금 submit 시점에 하나의 try~catch 문 안에서 두 개의 api (이메일 중복 확인, 회원가입) 을 처리하고 있는데
하나의 catch 문 내에서 두 api 의 에러 케이스를 같이 커버하면 코드가 섞일 수도 있고 하니
api 마다 각각 try~catch 로 감싸면 좋을 것 같아요
어차피 요구사항 상 두 개의 api 가 실행되어야 하는 시점이 별개라서 요구사항에 맞춰 수정하면 자연스럽게 정리될 것 같긴 합니다
@@ -0,0 +1,16 @@ | |||
$(document).ready(function () { |
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.
갑분제이쿼리 어디서 나온거죠 ㅋㅋ 급한건 아니니 시간되면 고치시고 그리 중요한 부분은 아니라 냅두셔도 됩니다
} | ||
.nav__bar { |
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.
띄어쓰기 일관성있게 해주세요!!
} | |
.nav__bar { | |
} | |
.nav__bar { |
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게
배포링크 : https://5-weekly-mission-rust.vercel.app/
setEamilError가 실행이 안되는같아요.....