[refactor] 페이지 이동 시 Access Token 확인 후 렌더링하는 로직으로 변경 #17
Conversation
| const ready = useCheckAccessToken(); | ||
|
|
||
| if (!ready) { | ||
| return null; | ||
| } | ||
|
|
There was a problem hiding this comment.
p2: 동료 개발자가 useCheckAccessToken이라는 훅을 사용할 때 기존에 이 훅을 만드신 분은 변수 네이밍을 뭘로 했지? 하면서 컨벤션을 맞추려는 비용이 들 것 같은데요. hook에서 객체를 리턴해줘서 아래와 같이 사용하면 훨씬 편리할 것 같은데 어떠신가요? 추가적으로 useCheckAccessToken에 변화가 생겨 return해야 하는 값이 추가되어도 수정할 필요가 없을 것 같아요.
| const ready = useCheckAccessToken(); | |
| if (!ready) { | |
| return null; | |
| } | |
| const { ready } = useCheckAccessToken(); | |
| if (!ready) { | |
| return null; | |
| } | |
| export default function useCheckAccessToken(): boolean { | ||
| const [ready, setReady] = useState(false); |
There was a problem hiding this comment.
p3: 이 훅이 하는 역할은 accessToken이 있으면 return true, 없으면 / 경로로 redirect 시켜주는 것 같은데요. 내부적으로 navigate가 감춰져 있다는 게 기대와 다르게 사용이 될 수 있을 것 같다는 생각이 들어요. 개발자 입장에서 AccessToken이 존재하는지 체크하려고 했는데 만약 존재하지 않으면 redirect가 되어버릴 수 있는거죠.
함수가 2가지의 일을 하는 것과 side-effect가 있다는 것으로 생각해볼 것 같은데 redirect 로직은 추출해서 사용하는 쪽에서 ready가 false면 redirect 해주는 게 어떨까 싶어요.
|
|
||
| import useAccessToken from './useAccessToken'; | ||
|
|
||
| export default function useCheckAccessToken(): boolean { |
There was a problem hiding this comment.
p3: 훅이 하는 일은 accessToken을 가지고 유효한지, 아닌지를 가르고 있는 것 같은데요. 우리가 집중해야 하는 것은 accessToken을 확인해서 인증된 유저인지 보는 것 보다는 현재 유저가 인증된 유저인지만 확인하고 싶은 것 같아요. 외부에서는 어떤 방식으로 인증된 유저인지 확인하는 것에는 관심이 없으니까요. 따라서 네이밍이 조금 더 범용적으로, 무엇을 하고 싶은지에 맞춰서 작성이 되도 좋을 것 같아요.
| export default function useCheckAccessToken(): boolean { | |
| export default function useAuth(): boolean { | |
| if (accessToken) { | ||
| setReady(true); | ||
| } | ||
| }, [accessToken, setAccessToken]); |
There was a problem hiding this comment.
p3: setAccessToken이 deps에 필요할까요?
eslint-plugin-react-hooks도 사용해보시면 좋을 것 같아요.
| } | ||
| }, [accessToken, navigate]); | ||
|
|
||
| return ready; |
There was a problem hiding this comment.
p2: 인터페이스가 아래처럼 되면 좋을 것 같은데 어떠신가요?
| return ready; | |
| return { ready }; |
| useEffect(() => { | ||
| if (!accessToken) { | ||
| navigate('/'); | ||
| } | ||
| }, [accessToken, navigate]); |
| export default function useCheckAccessToken(): boolean { | ||
| const [ready, setReady] = useState(false); | ||
|
|
||
| const { accessToken, setAccessToken } = useAccessToken(); |
There was a problem hiding this comment.
p2: setAccessToken은 사용되고 있지 않은 것 같아요.
No description provided.