-
Notifications
You must be signed in to change notification settings - Fork 79
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 #725
The head ref may contain hidden characters: "Next.js-\uC774\uC601\uD6C8-sprint12"
[이영훈] sprint12 #725
Conversation
values, | ||
}: { | ||
productId: number; | ||
values: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any는 TypeScript의 타입 시스템의 장점을 무력화시키기 때문에, 가능한 한 피하는 것이 좋습니다. any 타입은 모든 타입을 허용하기 때문에, 타입 안전성을 보장하지 않습니다. 이는 런타임 오류의 가능성을 높이고, 코드의 신뢰성을 떨어뜨립니다.
name: "", | ||
}); | ||
|
||
const validation = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 코드에서는 newValues 객체의 price, description, name 속성 값이 모두 존재할 때만 .trim() 메서드를 호출하고 있습니다. 만약 이 속성 중 하나라도 존재하지 않으면 코드가 예기치 않은 결과를 낳을 수 있습니다. 따라서 각각의 필드를 개별적으로 검증하는 것이 좋습니다. 이렇게 하면 각 필드의 유효성을 별도로 체크할 수 있으며, 문제가 되는 필드를 더 정확히 식별할 수 있습니다. 또한, .trim() 메서드는 문자열에서 공백을 제거하지만, 해당 속성 값이 문자열이 아닐 경우 오류를 발생시킬 수 있습니다.
const validateField = (value) => typeof value === 'string' && value.trim() !== "";
const validation = {
price: validateField(newValues.price),
description: validateField(newValues.description),
name: validateField(newValues.name),
};
// Check if all fields are valid
const isValid = Object.values(validation).every(Boolean);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
항상 묶어서 유효성 검사를 했었는데 개별적으로 해보겠습니다!
}, [product]); | ||
|
||
const uploadProductMutation = useMutation({ | ||
mutationFn: ({ productId, values }: { productId: number; values: any }) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any 타입 지향해주세요!
src="/images/ic_kebab.svg" | ||
width={24} | ||
height={24} | ||
alt="케밥아이콘" | ||
/> | ||
{isOpenDropdown ? ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&& 연산자를 사용하면 코드가 간결하고 읽기 쉬워질 수 있습니다. 조건이 참일 때만 컴포넌트가 렌더링되므로, 조건부 렌더링이 직관적으로 이해됩니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 수정하겠습니다!
setPreview(previewUrl); | ||
|
||
const formData = new FormData(); | ||
formData.append("image", selectedFile); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try-catch 블록을 사용하여 비동기 요청에서 발생할 수 있는 에러를 처리하고, 에러 메시지를 출력하는 로직을 추가하는 것이 좋습니다.
try {
// 파일 업로드 요청
const response = await postFile(formData);
const fileUrlData = await response.data;
if (fileUrlData.url) {
setValues(prev => ({
...prev,
images: fileUrlData.url,
}));
}
} catch (error) {
console.error('파일 업로드 실패:', error);
// 에러 처리 로직 추가 (예: 사용자에게 에러 메시지 표시)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
에러처리가 아직 너무 미숙하네요ㅠ 수정해보겠습니다!!
<button className="flex h-42 w-88 cursor-pointer items-center justify-center rounded-8 bg-[#9ca3af] px-23 py-12 text-16 font-semibold leading-[26px] text-[#ffffff]"> | ||
<button | ||
disabled={validateButton} | ||
className={`${validateButton ? "bg-gray-400" : "bg-blue"} flex h-42 w-88 cursor-pointer items-center justify-center rounded-8 px-23 py-12 text-16 font-semibold leading-[26px] text-white`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validateButton 대신 isButtonDisabled
등은 어떨까요? validateButton이라는 이름은 버튼의 상태를 명확히 설명하지 않습니다. '검증'이라는 용어는 버튼이 검증 기능을 수행하는지, 아니면 버튼이 활성화되는 조건을 검증하는지를 명확히 하지 않습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네이밍이 정말 중요하군요! 수정하겠습니다!
}); | ||
const { name, description, price, tags } = values; | ||
|
||
const validateButton = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const isFormValid = (values: ProductForm): boolean => {
const { name, description, price, tags } = values;
return (
name.trim() !== "" &&
description.trim() !== "" &&
price.trim() !== "" &&
tags.length > 0
);
};
|
||
const UploadProducts = () => { | ||
const router = useRouter(); | ||
const queryClient = useQueryClient(); | ||
const [values, setValues] = useState({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인터페이스를 정의하면 좋을 것 같습니다.
interface ProductForm {
images: string;
tags: string[];
price: string;
description: string;
name: string;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이미 types/product.ts 에 타입이 정의되어있네요 : )
@@ -35,6 +92,9 @@ const UploadProducts = () => { | |||
<label className="flex flex-col gap-12 text-18 font-bold text-[#1f2937]"> | |||
태그 | |||
<input | |||
name="tags" | |||
value={tags} | |||
onChange={handleInputChange} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
태그의경우 일반적인 폼 입력과 처리방식이 다를 수 있습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 감사합니다 멘토님!!!
리액트 쿼리는 캐싱을 통해 데이터 fetching의 성능을 향상시킵니다. 캐싱된 데이터는 서버에서 다시 가져오는 것이 아니라 클라이언트의 메모리에서 가져오므로, 데이터가 fresh 상태일 때는 네트워크 요청을 줄일 수 있습니다. 반면, stale 상태일 때는 서버에서 다시 데이터를 가져옵니다. 테스트하는 데이터의 양이 적으면, 네트워크 속도와 서버 응답 시간이 상대적으로 적기 때문에 큰 차이가 나지 않을 수 있습니다. 데이터를 더 많이 요청하여 캐싱과 네트워크 요청의 차이를 명확하게 볼 수 있습니다. |
요구사항
기본
상품
댓글
주요 변경사항
스크린샷
상품 상세 페이지
상품 수정
댓글 등록
댓글 수정
멘토에게