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

[정인재] Sprint12 #331

Conversation

Injaeeee
Copy link
Collaborator

@Injaeeee Injaeeee commented Oct 4, 2024

요구사항

기본

  • 문의하기 input창에 값을 입력 후 ‘등록’ 버튼을 누르면 댓글이 등록됩니다.
  • 내가 등록한 댓글은 수정, 삭제가 가능합니다.

주요 변경사항

  • 기존 작성했던 get , post 패치 함수를 useQuery로 마이그레이션만 해보았습니다 .. ㅠㅠ

스크린샷

image

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@Injaeeee Injaeeee added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Oct 4, 2024
@Injaeeee Injaeeee requested a review from lhc0506 October 4, 2024 11:12
Copy link
Collaborator

@lhc0506 lhc0506 left a comment

Choose a reason for hiding this comment

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

안녕하세요 인재님!
기존 fetch를 react query로 바꾸시느라 고생하셨습니다.

전체적으로 잘 query와 mutation 을 작성해주신것같아요.

한가지 아쉬운점은 멘토링때 말씀드렸던, 쿼리 키관리와 hook으로 빼는 것을 한번 시도해보시면 더욱 좋을것같습니다!

그리고 불필요한 주석이나, storybook에서도 불필요한 파일들은 지워주는것이 개발 협업 상에서 훨씬 좋답니다!

한주간 고생하셨고 이번주도 화이팅입니다!!

@@ -33,3 +33,5 @@ yarn-error.log*
# typescript
*.tsbuildinfo
next-env.d.ts

*storybook.log
Copy link
Collaborator

Choose a reason for hiding this comment

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

현재 filechanged를 보시면 아래 사진처럼 하나의 오류가 나오게 됩니다.
이는 "End Of Line(EOL)" 이 없다는 경고입니다. 이를 없애려면 파일 내 코드 마지막 줄 뒤에 개행을 해주셔야 해요.
image
블로그를 보시면서 왜 마지막 줄에 개행이 없을 때 깃헙에서 오류를 보여주는지 확인해보시고, 사용하시는 코드에디터에서 설정을 해보시는 것을 추천드립니다! (물론 해결을 안해도 코드상에선 문제가 없지만, 코드리뷰할 때 불필요하게 이것에 한번 더 눈이 가는것을 줄일 수 있습니다!)
(다른곳에선 없는거보아 실수하신것 같지만 첫 리뷰라 혹시몰라 남깁니다!)


export default function Board() {
const router = useRouter();
const { id } = router.query;
const [board, setBoard] = useState<BoardItemType | null>(null);
const [comment, setComment] = useState<CommentType[]>([]);
// const [comment, setComment] = useState<CommentType[]>([]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// const [comment, setComment] = useState<CommentType[]>([]);

불필요한 주석은 삭제부탁드립니다!

@@ -16,16 +16,41 @@ import CommentItemList from "@/components/commentitemlist";
import user from "@/images/user.png";
import { FormatDate } from "../util/formatDate";
import Link from "next/link";
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";

export default function Board() {
const router = useRouter();
const { id } = router.query;
const [board, setBoard] = useState<BoardItemType | null>(null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

board도 아래에서 fetch해오는것같아요! 이부분도 query로 바꿔줄 수 있을것같습니다!

const { data: comment = [] } = useQuery<CommentType[]>({
queryKey: ["comments", id],
queryFn: () => getArticleComment(id as string),
enabled: !!id, // id가 있을 때만 쿼리를 실행
Copy link
Collaborator

Choose a reason for hiding this comment

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

[id]페이지니 항상 id는 있을것이라 예상되는데 아마 query를 하면 타입이 nullish하기 때문에 붙여주신것같아요.

할 수 있는 방법으로는 router.query대신 query util 함수를 만들어주어서 id type을 nullish하지 않게 하거나,
id 가 undefined라면 가져올수없는 id값 (예를들어 -1) 로 넣어주기도 합니다!

onSuccess: () => {
// 성공 시 댓글 목록 업데이트

queryClient.invalidateQueries({ queryKey: ["comments", id] }); // 댓글 관련 캐시 무효화
Copy link
Collaborator

Choose a reason for hiding this comment

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

comments라는 key값이 상수가 아니면 사용할 때 신경을 써야하고 또한 바꾸게 될 때도 쉽지않습니다.
지난번에 멘토링 때 말씀드린 쿼리 키관리가 블로그에서 이에 대해 자세히 나와있으니 한번 보시고 적용시켜보시는 것을 추천드립니다!

enabled: !!id, // id가 있을 때만 쿼리를 실행
});

const uploadArticleMutation = useMutation({
Copy link
Collaborator

Choose a reason for hiding this comment

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

useMutation을 함수로 잘 만들어주셨어요!
이것을 아예 다른 파일로 빼서 customHook처럼 만들면 좀더 이 파일에서 관심사 분리가 될것같아요!

}, []);
const { data: nextBestBords = [] } = useQuery<BoardItemType[]>({
queryKey: ["bestBoards"],
queryFn: () => getArticles(1, 3, "like"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

'like' 가 그냥 상수값이어서 언제 어떻게 바뀌거나 뭐가올지 모를것같아요
getArticles함수에서 orderBy 타입을 그냥 string이 아닌 enum값으로 해주면 더 확실히 해줄 수 있을 것같아요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

nextBoards라 되어있어서 다음 것들이 오나했는데 지금은 30개만 볼수가 있네요
무한스크롤이나 페이지네이션이 있어야할것같습니다!

commentContent: string;
}) => postArticleComment(id, commentContent),
onSuccess: () => {
// 성공 시 댓글 목록 업데이트
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 주석과 아래주석은 겹치는것같아요!

이건 개인적인 의견이지만 로직 자체를 설명하는것은 아주 헷갈리거나 어려운 부분들에만 추가하는 것을 지향하는 편입니다~!

@lhc0506 lhc0506 merged commit 799e13b into codeit-bootcamp-frontend:Next-정인재 Oct 10, 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