feat: uoslife-assignment-fe-2-송채희 #3
Conversation
Chaehee03
commented
Jul 10, 2023
Chaehee03
commented
- Auth 구현 요구 사항 1, 2, 3 & 추가 요구 사항 1 구현 완료.
Features
Chore
Bug Fixes
ContributorsCommit-Lint commandsYou can trigger Commit-Lint actions by commenting on this PR:
|
package.json
Outdated
| "cookie-parser": "^1.4.6", | ||
| "cors": "^2.8.5", | ||
| "dotenv": "^16.3.1", | ||
| "express": "^4.18.2", | ||
| "jsonwebtoken": "^9.0.1", | ||
| "ky": "^0.33.3", | ||
| "nodemon": "^2.0.22", |
There was a problem hiding this comment.
혹시 해당 패키지를 설치한 이유가 있을까요?! 실제로 사용되지 않는 것 같아서요~
src/pages/Login.tsx
Outdated
| const login = useCallback( | ||
| (e: React.FormEvent<HTMLFormElement>) => { | ||
| e.preventDefault(); | ||
| const login = async () => { |
There was a problem hiding this comment.
43번째줄과 같은 이름의 함수가 선언되어서 어떤 역할을 하는 함수인지 구분이 필요할 거 같아요~
| LoggedIn(); | ||
| }); | ||
|
|
||
| const login = useCallback( |
There was a problem hiding this comment.
혹시 useCallback으로 감싼 이유가 있을까요? 궁급합니다dependencies를 보면
// ...
, [email, password, navigate]인데 해당 login 함수가 여러번 호출되지도 않고 어짜피 email, password가 변경되면 다시 생성되서요!
| LoggedIn(); | ||
| }); | ||
|
|
||
| const login = useCallback( |
There was a problem hiding this comment.
함수명은 어떤 동작을 하는지 쉽게 알아볼 수 있게 동사로 작성해주면 좋을 것 같아요~
추천은 handleLogin, onSubmitLogin 등이 있습니당
src/pages/Main.tsx
Outdated
|
|
||
| const Login = () => { | ||
| const navigate = useNavigate(); | ||
| const logout = () => { | ||
| localStorage.clear(); |
There was a problem hiding this comment.
localStorage.clear()는 모든 스토리지 정보를 초기화해서, 만약 앱에서 스토리지에 다른 정보도 사용하고 있다면 안되겠죠??
token을 명시적으로 삭제하는게 좋아보여요~
| } catch (error) { | ||
| console.log(error); | ||
| } |
There was a problem hiding this comment.
error라면 refresh token도 만료된 상황이겠죠?
그렇다면 console.log만 찍는게 아니라, 어떤 동작이 필요할것같은데 수정해주세요!
src/pages/Main.tsx
Outdated
| const [isLoggedIn, setIsLoggedIn] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| const LoggedIn = async () => { |
There was a problem hiding this comment.
우선 함수를 작성할 때 첫글자를 소문자로 만들어주세요.
그리고 해당 함수는 로그인 유지기능을 담당하는 것 같은데, LoggedIn이라는 함수명은 그 정보를 담지 않아 이해하기 어려운 것 같아요!
There was a problem hiding this comment.
README에 구현 요구사항이 추가되었습니다! 회원정보가 모두 입력되지 않았다면 API 요청을 보내지 않도록 구현해주세요.
|
|
||
| const Login = () => { | ||
| const navigate = useNavigate(); | ||
| const logout = () => { |
There was a problem hiding this comment.
마찬가지로, handle- onSubmit-과 같은 접두사를 붙인 형태로 함수명을 작성해주세요!
