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 #720

Conversation

oris8
Copy link
Collaborator

@oris8 oris8 commented Jul 19, 2024

요구사항

기본

  • [x]
  • []
  • []

심화

  • [x]
  • []

주요 변경사항

스크린샷

image

멘토에게

  • recoil, 리액트쿼리 적용하면서 기존 프로젝트를 전체적으로 수정중인데 시간이 없어서 완성하지 못했습니다
    • 요구사항 부분은 오류없이 동작하도록 만들었으니 (일단은) item(상품)관련 페이지만 확인하시면 될거같습니다!!
  • 올리긴올리는데 갈아엎는중이라 수정못한 부분이 엄청나게 많아요🥹 (타입오류나 불필요한 파일, 중복파일, 안쓰는 import문 등등..)
  • 리액트쿼리 관련해서 사용한 방법이나 위치가 적절한지 리뷰부탁드립니다🥹🥹

@oris8 oris8 requested a review from lisarnjs July 19, 2024 05:35
@oris8 oris8 self-assigned this Jul 19, 2024
@oris8 oris8 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jul 19, 2024
Copy link
Collaborator

@lisarnjs lisarnjs left a comment

Choose a reason for hiding this comment

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

정민님 재능이 있으신대요 👍
코드 퀄리티가 좋아서 놀랐습니다!
마지막 프로젝트 때도 react-query 도입해서 많이 써보세요!
이번주도 화이팅입니다 :)

@@ -6,10 +6,12 @@ import Button from "@/components/Button/Button";
import FormGroup from "@/components/FormGroup/FormGroup";
import { AuthLogoHeader, SocialLogin } from "@/components/PageComponents/auth";
import useAuthForm, { LogInRequest } from "@/hooks/useAuthForm";
import { useAuth } from "@/contexts/AuthProvider";
import { login } from "@/lib/api/auth";
import { useSetRecoilState } from "recoil";
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 recoil을 도입해보셨네요 👍

};

useEffect(() => {
if (name && price && description && images) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

useEffect로 유효성 검사도 잘 해주셨네요!
조건문에서 name, price ... 등이 있냐 없냐도 좋고 조금 더 보완하자면
name?.length > 0 처럼 각각의 조건들이 더 진실성 있게끔 바꿔보는 것도 좋을 것 같네요 :)

}
};

const uploadProductMutation = useMutation({
Copy link
Collaborator

Choose a reason for hiding this comment

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

mutationFn과 응답값을 타입스크립트의 도움을 받아서 좀 더 정확한 타입으로 디벨롭하면 더 좋을 것 같아요
예를 들면

const uploadProductMutation = useMutation<ItemCreationData, Error>({
  mutationFn: (newProduct: ItemCreationData) => createProduct(newProduct),
  onSuccess: () => {
    queryClient.invalidateQueries({ queryKey: ["products"] });
  },
});

const handleSubmit = async (e: React.FormEvent) => {
  e.preventDefault();

  try {
    const data = await uploadProductMutation.mutateAsync(formData as ItemCreationData);
    router.replace(`/items/${data.id}`);
  } catch (error) {
    alert(`Error adding post: ${error}`);
  }
};

import { APP_BASE_URL } from "@/constants/common";
import { ITEM_COMMENT_LIMIT } from "@/constants/pageLimit";
import { getProductDetail } from "@/lib/api/product";
import { useQuery, keepPreviousData } from "@tanstack/react-query";

const Page = async ({ params }: { params: { id: string } }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

useQuery가 비동기 데이터 가져오기를 내부적으로 처리하므로 구성 요소 자체는 동기식이어야 합니다.
즉, Page 자체를 async로 구성하는 것은 좋지 않을 것 같아요!

});

if (isLoading) return <div>Loading...</div>;
if (error) return <div>Error loading data</div>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

return <div>Error loading data : {error.message}</div>;

위 처럼 에러 메시지나 에러 시 보여줄 수 있는 컨텐츠를 포함해서 적어주는 것이 가장 좋아요!

};
}) => {
const { isVisible, toggleDropdown, dropdownRef, handleItemClick } =
useDropdown(options || {});
Copy link
Collaborator

Choose a reason for hiding this comment

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

|| 대신 ?? 로 options가 null이거나 undefined일때 {}를 지정해라 를 의미할 수 있도록 바꿔주시면 더 직관적일 것 같아요 👍


import Card from "@/components/Card/Card";
import useFavoriteButton from "@/hooks/useFavoriteButton";

Copy link
Collaborator

Choose a reason for hiding this comment

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

interface Item {
id: number;
name: string;
price: number;
description: string;
tags: string[];
images: string[];
}
과 같이 Item타입을 명시한 곳을 못 찾았네요..!


return (
<div className={`${className}`}>
<div className="text-20 font-bold text-cool-gray-800">베스트 상품</div>
<div>
{data && data.length !== 0 ? (
{bestItems && bestItems.length !== 0 ? (
Copy link
Collaborator

Choose a reason for hiding this comment

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

옵셔널 체이닝을 이용한다면 더 간결해질 수 있게다는 생각입니다 :)

import { useRecoilValue } from "recoil";

const ItemComments = () => {
const params = useParams<{ id: string }>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

const { id } = useParams<{ id: string }>(); 로 구조 분해를 여기서도 사용해보세요!

favoriteButtonLikeCount,
queryClient.setQueryData(
["favoriteStatus", path, id],
(prev: { favoriteCount: number; isFavorite: boolean } | undefined) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

{ favoriteCount: number; isFavorite: boolean } 도 타입으로 빼면 더 좋겠네요 👍

@lisarnjs lisarnjs merged commit c75b1c7 into codeit-bootcamp-frontend:Next.js-이정민 Jul 22, 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