-
Notifications
You must be signed in to change notification settings - Fork 46
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
[박연학] sprint11 #325
The head ref may contain hidden characters: "Next-\uBC15\uC5F0\uD559-sprint11"
[박연학] sprint11 #325
Conversation
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.
로그인, 회원가입, 리프레시토큰 로직을 모두 잘 구현해주셨네요! 👏👏
고생많으셨습니다. 다음 단계에서는 이 로그인 상태를 매번 모든 페이지에서 확인하지 않고, 전역 상태로 관리하는 방법에 대해 찾아보시면 좋을 것 같습니다 💪
@@ -7,6 +7,10 @@ export default function boards() { | |||
<> | |||
<BestPost /> | |||
<Suspense fallback={<div>로딩 중</div>}> | |||
{/* 이 부분에 suspense를 감싼 이유가 빌드 할 때 오류가 뜨더라고요...? |
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 isValid = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/.test( | ||
value | ||
); | ||
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.
중첩된 삼항연산자 사용은 지양해주세요!
가독성이 떨어질 수 있어요. if 문으로 대체해도 충분할 것 같습니다.
const [emailError, setEmailError] = useState(""); | ||
const [passwordError, setPasswordError] = useState(""); | ||
|
||
const validateEmail = (value: string) => { |
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.
파라미터로 받는 value 에 공백이 포함될 수도 있으니 String.trim() 메소드를 써도 좋을 것 같네요~ (아래 password 도 동일합니다.)
const [passwordError, setPasswordError] = useState(""); | ||
|
||
const validateEmail = (value: string) => { | ||
const isValid = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/.test( |
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.
요 정규표현식은 input 이 변화할때마다 매번 새롭게 선언될 필요는 없어보여요.
함수 외부에 선언해 두는게 더 좋을 것 같습니다.(password 도 동일)
validatePassword(value); | ||
}; | ||
|
||
const isFormValid = |
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 isFormValid = !emailError && !passwordError && email && password;
} = useSign(); | ||
const router = useRouter(); | ||
|
||
useEffect(() => { |
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.
멘토링 시간에 언급했던 것처럼 useEffect에서 localStorage를 바로 읽는 것은 서버 렌더링 과정에서 문제가 될 수 있습니다. 브라우저(클라이언트) 환경에서만 접근할 수 있게 조건을 추가해주시면 좋을 것 같네용
if (!response.ok) { | ||
if (response.status === 401) { | ||
const newToken = await refreshAccessToken(); | ||
token = newToken; |
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.
이 줄이 없어도 될 것 같아요.
아래 header 에 넣어주는 token 을 newToken 으로 대체하면 될 것 같네용
요구사항
기본
주요 변경사항
스크린샷
멘토에게