-
Notifications
You must be signed in to change notification settings - Fork 0
새로 고침 시 로그인이 풀리는 버그 수정과 그에 따른 hook과 컴포넌트 로직 수정 (after #69 merged) #74
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
base: dev
Are you sure you want to change the base?
Changes from all commits
e790e74
7a314e7
42e7d0e
8a84bd4
59b322d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,35 +1,39 @@ | ||
| import { useEffect, useState } from 'react'; | ||
|
|
||
| import { authApi } from '@/apis'; | ||
| import { useUserStateValue } from '@/atoms/userState'; | ||
| import { useUserState } from '@/atoms/userState'; | ||
|
|
||
| const useAutoLogin = (): { isLoading: boolean; isLoggedIn: boolean } => { | ||
| const userState = useUserStateValue(); | ||
| const [user, setUserState] = useUserState(); | ||
|
|
||
| const [isLoading, setIsLoading] = useState(false); | ||
| const [isLoggedIn, setIsLoggedIn] = useState(false); | ||
|
|
||
| const autoLogin = async () => { | ||
| try { | ||
| const userId = localStorage.getItem('userId'); | ||
| if (!userId) return; | ||
|
|
||
| setIsLoading(true); | ||
|
|
||
| await authApi.refresh(); | ||
|
|
||
| // @TODO refresh API 성공 시, 사용자 정보 가져오는 API를 호출하여 사용자 상태로 저장 | ||
|
|
||
| setIsLoggedIn(true); | ||
| setUserState({ userId }); | ||
| } catch (e) { | ||
| setIsLoggedIn(false); | ||
| /* | ||
| 따로 에러 핸들링 해야할 것으로 보입니다. | ||
| refresh를 실패했다는 것은 곧 refresh token이 만료되었을 경우가 대부분일테니 | ||
| 이에 따라서 다시 로그인 해달라는 창을 보여주는 것이 어떨지.. toastify 처럼.. | ||
| */ | ||
|
Comment on lines
+22
to
+26
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 개인적으로 저는 refresh token 이 만료되었을 때 모달같은 알림을 띄우는 것을 선호하지 않습니다 그린카라는 앱을 이용하다가 언급해주신 경험을 했는데요, 개인적인 생각이라 다른 방향으로 결정되어도 괜찮습니다!!
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그 훅이 뭐하는 건지.. 잘 몰라서요.. 어떤거죠 ?!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 영상 5초 부근 인데... 훔 하단 중간 부분에 보이게 된다는 단점이 있긴 하네요...
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 로그인 필요 여부는 반드시 사용자에게 보여줘야 하는 부분이지 않을까 싶어요. refresh token이 만료되게 되면 잘 사용하다가 갑자기 새로고침을 했더니 로그아웃이 되는 상황이 발생할 수 있고 이에 대한 피드백이 반드시 사용자에게 필요하다고 생각합니다. 요거에 대해서는 useAutoLogin 외에도 일반 API 요청을 할 때도 처리를 해줘야 하는 부분이라 다른 이슈에서 작업하는 게 좋지 않을까 싶습니다. |
||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| autoLogin(); | ||
| }, [userState]); | ||
| }, []); | ||
|
|
||
| return { isLoading, isLoggedIn }; | ||
| return { isLoading, isLoggedIn: user !== null }; | ||
| }; | ||
|
|
||
| export default useAutoLogin; | ||
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.
에러 throw를 할 필요가 있을까욥??