Conversation
hyejungan
added
매운맛🔥
There was a problem hiding this comment.
고생하셨습니다.
멘토링때 말씀해주신건 차근차근 고쳐보겠습니다. (에러핸들링, 토큰 proxy 전달)
👍
토큰을 axios 인터셉터로 저장해서 사용하고 싶은데 그렇게 하는것도 괜찮을까요? 토큰때문에 무조건 로그인부터 들어와야한다라는 조건이 붙어서 이 방향도 아닌것 같아 고민이 됩니다.
심화과제 내용이니 해보는것도 좋을것 같네요. 모든 인증이 필요한 요청에 대해서는 어쨌든 실제 backend api로 리퀘스트 하기 전에 토큰 유효성 등을 체크하는 로직이 공통으로 들어가게 되어 있는데요, 이를 server side가 아니라 client side에서 수행할때에는 interceptor가 그나마 활용하기 좋은 레이어이긴 합니다. 다만 말씀드린대로 인증이 필요한 요청에 대해서만 제약을 걸고 싶다면 axios global보다는 instance단위로 interceptor를 구현하시는 것이 좋을것같네요.
많이 알려주셨는데 구현을 못해봐서 아쉬워요ㅠ
다 구현할 수 있으면 3년차 이상일겁니다. 고생하셨어요
| <Script | ||
| src="https://t1.kakaocdn.net/kakao_js_sdk/2.5.0/kakao.min.js" | ||
| onLoad={initKakao} | ||
| /> |
There was a problem hiding this comment.
App에서 로드할 경우 불필요한 페이지에서도 해당 js를 로드해야 하는데요, 필요한 페이지에서만 로드하도록 하면 좋을것같아요
|
|
||
| export default function App({ Component, pageProps }: AppProps) { | ||
| function initKakao() { | ||
| if (!window.Kakao.isInitialized()) { | ||
| window.Kakao.init(process.env.NEXT_PUBLIC_KAKAO_KEY); |
There was a problem hiding this comment.
토큰을 환경변수로 관리하셨네요. 잘하셨습니다. 👍
| export const FooterContext = createContext({ | ||
| isFooterVisible: false, | ||
| setIsFooterVisible: (visible: boolean) => {}, | ||
| }); |
There was a problem hiding this comment.
Context 관련된 파일은 별도의 context 파일로 분리해주시는 것이 좋을것같습니다.
| if (localStorage.getItem('access_token')) { | ||
| router.push('/folder'); | ||
| useEffect (() => { | ||
| if(user) { |
There was a problem hiding this comment.
localStorage 관련 코드를 잘 추상화 하셨습니다.
| router.push('/folder'); | ||
| useEffect (() => { | ||
| if(user) { | ||
| router.push('/folder') |
There was a problem hiding this comment.
push 보다는 redirect를 하면 좋을것같아요
| }); | ||
| setCheckItem(checkItems); | ||
| } else if (value.length === 0) { | ||
| setCheckItem(links); |
There was a problem hiding this comment.
component prop으로 setter를 받기보다는 어떤 이벤트인지를 받는것이 좋습니다.
또한 외부에서 받는 value에 대해서 데이터를 정제한 뒤 setter로 올리는 코드라서 Search의 관심사가 아닐 가능성이 높지 않을까 해요. 해당 value를 상태관리하는 코드의 관심사가 맞지 않을까요?
| useEffect(() => { | ||
| Kakao.cleanup(); | ||
| Kakao.init(''); | ||
| }, []); |
There was a problem hiding this comment.
SocialShare 코드와 실제 kakaosdk를 로드하는 코드가 멀리 떨어져있고, 다른 social share vendor 코드와 섞여있으므로 카카오는 카카오끼리 묶을 수 있겠죠?
import kakaoIcon from '@/src/assets/Kakao.svg';
function KakaoShareButton() {
return (
// SDK js 로드 및 init, cleanup 담당
<KakaoSDKLoader>
{(sdk) => <SocialShareButton src={kakaoIcon} onClick={() => {
// sdk 호출
}}/>}
</KakaoSDKLoader>
)
}| const response = await axios.get(`/folders${folderId ? folderId : ''}`); | ||
| const nextFolders = response.data.data.folder; | ||
| setValues((prev) => ({ ...prev, folders: nextFolders })); | ||
| } |
There was a problem hiding this comment.
getFolders, getLinks는 auth의 관심사가 아닌데 같이 있을 필요는 없습니다.
auth에서는 말그대로 인증에 대한 처리 (signup, signin) 만 있으면 좋을것같고,
각각의 데이터 소스에 대한 내용은 각각의 데이터 소스에서 관리하면 됩니다.
| const { email, password } = data; | ||
| const response = await axios.post('/sign-in', { email, password }); | ||
| const { accessToken } = response.data.data; | ||
| axios.defaults.headers.common['Authorization'] = `Bearer ${accessToken}`; |
There was a problem hiding this comment.
지금처럼 모든 api요청에 대해서는 글로벌로 처리하신 것이 효과적이므로 잘 하셨습니다.
다만 codeit이아닌 외부 api 요청에 대해서도 해당 accessToken이 유출될 여지가 있습니다.
그러한 상황에서 저같으면 아래처럼 제공할것같네요. best는 아닙니다. 애초에 accessToken을 js에서 만지면 안되니까요
function useFolders() {
const {headers} = useAuth();
return useFetch('/folders', {
//인증 관련 header를 넘김
headers: headers.extend({
//...custom header
})
})
}| if (require && !context.user && !context.isPending) { | ||
| router.push('/signin'); | ||
| } | ||
| }, [require, context.user, context.isPending, router]); |
There was a problem hiding this comment.
만약 인증이 필요한 페이지에만 AuthProvider를 감쌀것이라면
AuthProvider 내부에서 위의 처리를 한 뒤에 children을 그리면 더 깔끔하지 않을까요?
hook은 무조건 한번은 화면이 렌더링 되어야 하는 문제가 있습니다.
function AuthProvider({children}) {
return (
<AuthContext.Provider>
// 이 컴포넌트 내부에서 인증 여부에 따라 redirect 수행
<RedirectOnNoAuth>{children}</RedirectOnNoAuth>
</AuthContext.Provider>
);
}
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게