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

[#340] Feat: 게시글 상세페이지 텐스택쿼리 마이그래이션 #341

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

HoberMin
Copy link
Collaborator

@HoberMin HoberMin commented Jun 3, 2024

💬 Issue Number

closes #340

🤷‍♂️ Description

작업 내용에 대한 설명

📷 Screenshots

작업 결과물

image image

👻 Good Function

팀원에게 공유하고 싶은 함수나 코드 일부

📋 Check List

PR 전 체크해주세요.

  • Merge 하는 브랜치가 올바른가?
  • 코딩컨벤션을 준수하는가?
  • PR과 관련없는 변경사항이 없는가?

📒 Remarks

팀원이 코드리뷰 시 주의할 점 또는 말하고 싶은 점 특이사항

  • 정책을 변경했습니다. - 기존의 URL로 이동하던 로직을 바꿨어요.
    • 이미 투표한 사용자라면 투표불가능
    • 메인페이지에서 상세보기 버튼 하나로 동작
    • 투표 결과가 렌더링 된다면 그냥 결과값만 보이도록
  • 아직 유저정보에 대한 react query를 적용시키지 못해서 로그인되지 않은 사용자를 대상으로 어떻게 동작하는지에 대해 생각을 해보지 못했어요. 고려해봐야 할 것 같습니다.
  • SuspenseQuery를 기반으로 구현해놓아서 나중에 Suspense로 fallback을 설정해준다면 로딩화면을 바꿔서 보여줄 수 있을 것 같아요. 스켈레톤UI를 사용해볼 예정인데 Shadcn을 써볼까 고민중입니다.
  • 기존의 core.ts 파일을 분리했어요. Mutation 코드에서 에러를 처리해야하는데 그것보다 axios에서 처리하는 내용을 래핑해서 사용하는게 더 편할 것 같아서 분리해놓았습니다. 지금은 에러응답에 대한 처리가 없지만, 시간을 내서 모달이나 토스트창을 활용해볼예정입니다.

@HoberMin HoberMin self-assigned this Jun 3, 2024
alert(e);
}
if (!token) return;
await createComment(JSON.stringify({ type: 'comment', content: comment }));
Copy link
Collaborator

Choose a reason for hiding this comment

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

인자를 json.stringfy 해서 주기보다는 인자로 주는것이 어떨까요?!
인자로 type, content 를 주게될시에 타입추론이되어서 사용하기에 편리할것 같습니다.

그래서 createComment 함수 내부에서 type 인자를 받아 'comment' or 'vote' 를 판단하여 분기시키는 역할을 주는것이 더 좋을것 같습니다.! 혹시 어떻게 생각하실까요?

사용하는 입장에서는 create, comment 각각 둘로 잘 분리되었으면 좋겠다는생각입니다!

createComment(content);
createVote(content);

이렇게 분리 시켜 주는것도 좋을것 같구요! 지금 당장 분리시키는것이 좋을진 모르겠지만, 이렇게하면 독립적으로 함수들을(여기선 comment, vote) 확장시킬때 comment, vote 역할을 모두 책임지는 createComment 에서 확장시키는것보단 좋을것 같습니다.

@@ -0,0 +1,7 @@
import { useDeletePostAPI } from '@/apis/posts';

export const useDeletePost = (postId: string) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

위에 useCommentAPI 로 명시하신 것처럼 useDeletePostAPI 로 명시해주시면 좋을것 같습니다!
API 를 붙여주시면 통일성도 있고, 사용할때 api 호출관련 훅이구나라고 판단하기 쉬울것 같습니다!

import { useMutation, useQueryClient } from '@tanstack/react-query';
import { api } from './core';

const postVote = (postId: string, comment: string) =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 postComment 가 더 나을것 같다, 라고생각이드는데 혹시어떻게 생각하실까요?!

api 명세가 comment 라고 명시되어있고 postVote 라고 한다면 해당 함수를 사용할때, api 에 vote 관련 명세가 있었나? 라고생각할수도있을것같습니다.
그래서 제생각에는 api 와 가장 가까운 함수에서는 postComment 가 나을것 같구, 해당 함수를 사용하거나 래핑하는 함수에서 comment 와 vote 로 분기하는 식으로 설계하면 어떨까?싶습니다!

userId: postDetail.author._id,
};
if (!voteContent || !token) return;
await createComment(JSON.stringify({ type: 'vote', content: voteContent }));
Copy link
Collaborator

Choose a reason for hiding this comment

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

요기도 위에랑 같은 의견입니다!

Comment on lines +1 to +12
const timeOffset = (standardTime: string | Date) => {
if (standardTime instanceof Date) {
standardTime = standardTime.toString();
}

const MINUTE = 60;
const HOUR = MINUTE * 60;
const DAY = HOUR * 24;
const MONTH = DAY * 30;
const YEAR = MONTH * 12;

const createTime = new Date(standardTime);
Copy link
Collaborator

Choose a reason for hiding this comment

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

new Date() constructor 에 string 대신에 Date 타입이 들어가도 작동하는것 같습니다!
그래서 toString 으로 바꾸지 않아도될것같습니다!

그리고 string 이 date 인지 검사하는게 필요할것같습니다!

Copy link
Collaborator

@ienrum ienrum left a comment

Choose a reason for hiding this comment

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

어려운 작업 하시느라 수고하셨습니다 호민님!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants