Skip to content
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

[김세환] sprint10 #304

Conversation

kimsayhi
Copy link
Collaborator

@kimsayhi kimsayhi commented Aug 17, 2024

체크리스트 [기본]

상품 등록 페이지

  • 상품 등록 페이지 주소는 “/addboard” 입니다.
  • 게시판 이미지는 최대 한개 업로드가 가능합니다.
  • 각 input의 placeholder 값을 정확히 입력해주세요.
  • 이미지를 제외하고 input 에 모든 값을 입력하면 ‘등록' 버튼이 활성화 됩니다.
  • 회원가입, 로그인 api를 사용하여 받은accessToken을 사용하여 게시물 등록을 합니다.
  • ‘등록’ 버튼을 누르면 게시물 상세 페이지로 이동합니다.

상품 상세 페이지

  • 상품 상세 페이지 주소는 “/board/{id}” 입니다.
  • 댓글 input 값을 입력하면 ‘등록' 버튼이 활성화 됩니다.
  • 활성화된 ‘등록' 버튼을 누르면 댓글이 등록됩니다

멘토님께

  • 댓글에서 cursor을 이용해 페이지네이션을 구현하려했으나 완성하지 못했습니다.
  • 댓글을 달았을 때 댓글이 실시간으로 등록될 수 있도록 새로고침을 하고싶은데 token을 context에 저장하다보니 새로고침하면 토큰이 사라지는 문제가 발생했습니다. 새로고침을 하지 않고 새로단 댓글은 로컬에서 state에 바로 추가할 수 있도록 하면 될 것 같은데 그렇게하면 괜찮을까요?

@kimsayhi kimsayhi added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Aug 17, 2024
@kimsayhi kimsayhi changed the base branch from main to Next-김세환 August 17, 2024 04:42
@Taero-Kim Taero-Kim self-assigned this Aug 18, 2024
@@ -16,3 +21,23 @@ export const getArticleList = async ({
const { list, totalCount }: ArticleResponse = res.data;
return { list, totalCount };
};

export const getArticlesById = async (articleId: number) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p4;
단일 아티클을 fetching하는 것이므로 getArticleById와 같이 단수형을 사용해도 좋을 것 같아요!

return data;
};

export const getArticleComment = async ({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p4;
요경우에는 commentList 혹은 comments가 되는게 맞는 것 같습니다!

};
const value = useMemo(() => {
return { accessToken, refreshToken, setAccessToken, requestToken };
}, [accessToken]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p3;
이렇게 token값 자체를 context (react 상태)로 관리하는 경우
상태가 메모리에 저장되어 컴포넌트가 렌더링 되는동안만 상태가 유지됩니다.

일반적으로 로그인은 브라우저가 새로고침 되어도 유지가 되는게 사용자 경험측면에서도 좋겠죠?!
따라서 상태를 새로고침 이후에도 유지하려면 브라우저 자체의 저장소에 저장하는게 필요할 것 같아요!

보통 토큰에 대한 요청을 할 때, 서버에서 쿠키로 설정해주는 것이 일반적이지만
해당 api의 경우 그런 처리를 하고 있는 것 같지는 않아요.

따라서, 클라이언트에서 로컬스토리지로 관리하는게 좋을 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

따라서 토큰을 이렇게 상태로 관리할 필요 없이,
'auth/signin' 라우트로 요청시에
localStorage.setItem('accessToken', tokenValue) 로 로컬스토리지에 저장하는게 어떨까요?

content: data,
};
const res = await axios.post(`/articles/${articleId}/comments`, body, {
headers: { Authorization: `Bearer ${token}` },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p3;
토큰을 로컬스토리지로 관리하게 되면 이렇게 매번 headers에 token value를 전달하여 사용하지 않고,
axios의 interceptor를 사용하여 요청을 서버로 보내기 전에 그 요청을 가로채어 header 설정을 할 수 있을 것 같아요.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ex)

const instance = axios.create({
  baseURL: 'https://panda-market-api.vercel.app',
});

instance.interceptors.request.use((config) => {
  const token = localStorage.getItem('accessToken'); 

  if (token) config.headers.Authorization = `Bearer ${token}`;
  return config;
});

export default instance;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그리고 jwt 토큰에는 일반적으로 만료기한이 설정되어 있습니다.
서버에서 전달 받은 jwt를 디코딩하면 만료 기한(Exp)을 확인하실 수 있습니다.

따라서, 로컬스토리지에 저장되어 있는 accessToken의 값을 디코딩하여,
만료 시간을 파악하고 토큰이 만료된 경우, 새로운 accessToken을 발급받는 등의 처리도 하실 수 있습니다

예를들어 Jwt의 유효 시간을 확인하는 방식은 아래와 같습니다.

// jwt 디코딩
const decodeToken = (token) => {
  const payload = token.split('.')[1];
  const decodedPayload = atob(payload);
  return JSON.parse(decodedPayload);
}

// 토큰의 유효 시간 확인 함수
const isTokenExpired = (token) => {
  const decoded = decodeToken(token);
  const currentTime = Math.floor(Date.now() / 1000);

  return decoded.exp < currentTime; // exp가 현재 시간보다 이전이면 토큰 만료
}

token: accessToken,
data: value,
});
setValue("");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p3;
postArticleComment를 한 뒤에 최신화된 댓글 목록을 불러오기 위해서는
commentList라는 상태를 post요청 이후에 업데이트할 필요가 있을 것 같아요.

따라서, useArticleComment 훅에서 관리하는 commentList라는 상태가 여기에도 공유되고,
post 요청 이후 업데이트가 되어야, 댓글 작성후 새로운 댓글 리스트를 불러올 수 있을 것 같아요.

const INIT_CURSOR: Cursor = {
cursorArr: [0],
cursorIndex: 0,
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p4;
커서의 값에서, cursorArr를 따로 관리하시는 이유가 있으신가요?
서버에서 주는 nextCursor값을 저장하는 것으로 충분해보입니다!

만약, 더 불러올 데이터가 있는지 (nextCursor의 유무) 여부를 저장하고 싶으신거라면
별도의 상태를 하나 만들어서 관리하는게 어떨까요?

ex)

const [cursor, setCursor] = useState(null);
const [hasMore, setHasMore] = useState(true); 

const loadComment = async() => {
   ...
        const data = await getArticleComment({
        articleId: parseInt(id),
        limit: 5,
        cursor,
      });

     const { nextCursor } = data;
     setCursor(nextCursor);

    if(!nextCursor) setHasMore(false);
    ...
}

@Taero-Kim
Copy link
Collaborator

댓글을 달았을 때 댓글이 실시간으로 등록될 수 있도록
새로고침을 하고싶은데 token을 context에 저장하다보니
새로고침하면 토큰이 사라지는 문제가 발생했습니다.
새로고침을 하지 않고 새로단 댓글은 로컬에서
state에 바로 추가할 수 있도록 하면 될 것 같은데 그렇게하면 괜찮을까요?

로컬 state로 추가하는건 적절하지 않은 것 같아요! 왜냐면 댓글을 작성한 경우,
댓글을 작성한 당사자는 로컬 state 업데이트를 통해 자기가 작성한 댓글을 볼 수는 있겠지만,
그 사이에 다른 사람이 댓글을 동시간에 작성했다면, 그건 업데이트가 되지 않기 때문입니다.

따라서, 코드 리뷰에도 언급 드렸지만 댓글을 POST 요청으로 업데이트 한 경우에
api 호출을 통해 최신화된 commentList를 불러오는게 맞는 것 같습니다.

그렇게 하기 위해서는 commentList의 상태관리를 어디서 하고, 어떻게 훅들에 공유되어야 하는지 고민이 필요할 것 같아요.
지금은 useCommentForm의 댓글 POST 요청과 useArticleComment의 댓글 GET 요청이 독립적으로 동작하고 있어서
댓글에 대한 업데이트가 제대로 되지 않을 수밖에 없는 것 같아요.

물론, 페이지 전체 새로고침을 통해 api요청을 할 수도 있습니다만, 댓글을 위해 전체 페이지를 새로고침하여
context 등의 react 상태를 모두 초기화 시키는 방법은 적절치 않은 것 같습니다.

또한, 새로고침을 한다 하더라도 발급받은 토큰의 상태(즉, 로그인 상태)가 초기화 되는 방식도
사용자 경험상 좋지 않은 것 같습니다.

코드리뷰에서도 말씀드렸지만, 토큰 값은 적절하게 로컬스토리지 등의 브라우저 저장소에 저장해놓은 후
사용하시는게 좋을 것 같습니다.

@Taero-Kim
Copy link
Collaborator

안녕하세요, 세환님!
여러 api 요청을 훅을 통해 모듈화 하시고, 컴포넌트도 책임 소재를 나누어 모듈화 하시려는게 코드에서 많이 느껴졌습니다.

로그인 상태를 위한 토큰 값 관리 및 커서 기반의 fetching 등 조금 생소하시고, 어려우셨을텐데요
그럼에도 이런 저런 고민을 하며 코드로 작성하신 점 칭찬드립니다👍

코드리뷰를 하고 보니 텍스트로 전달드리기 한계가 있는 부분들도 있었습니다.
그래서 코드리뷰를 보고도 전달드리려는 내용이 어떤 것인지 파악이 힘드실 수도 있을 것 같다는 생각을 했습니다.
따라서, 혹시 코드리뷰에 대한 내용이 잘 이해가 가지 않으시면 말씀주세요.
음성 채팅을 통해 더 부연할 수 있도록 하겠습니다.

@Taero-Kim Taero-Kim merged commit 7e6a22e into codeit-bootcamp-frontend:Next-김세환 Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants