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

경북대 FE_정서현 4주차 Step2 #79

Open
wants to merge 25 commits into
base: hyunaeri
Choose a base branch
from

Conversation

hyunaeri
Copy link

@hyunaeri hyunaeri commented Jul 19, 2024

안녕하세요 멘토님, Step2 제출합니다.

질문이 한가지 있습니다!

아래는 제가 상품의 Detail 과 Option 정보를 불러오기 위해 작성한 커스텀 훅입니다.

export const useGetProductsInfo = (product: string) =>
  useQuery({
    queryKey: [getProductsDetailPath(product), getProductsOptionPath(product)],
    queryFn: async () => {
      const [detail, options] = await Promise.all([
        getProductsDetail(product),
        getProductsOptions(product),
      ])
      return { detail, options }
    }
  })

과제 구현 중, /v1/products/${product}/detail/v1/products/${product}/options 경로가 두개로 나뉘어지는데,
하나의 useQuery 문에서, 두 개 이상의 API 호출을 하는 정석적인 방식이 있을까요?

물론 병렬로 두개의 useQuery 훅을 사용하는 방법도 있겠지만 하나로 작성하는 방법이 알고 싶습니다!

hyunaeri added 22 commits July 17, 2024 15:06
- 카드 메시지 미 입력 시 메시지 입력 안내
- 현금 영수증을 신청했을 때, 번호 미 입력 시 현금 영수증 번호 입력 안내
- 현금 영수증 입력은 숫자만 입력하도록 안내
@hyunaeri hyunaeri changed the base branch from main to hyunaeri July 19, 2024 13:07
Copy link

@sjoleee sjoleee left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~
여러 api콜을 날리는 것은 useQueries를 사용하시면 됩니다! 리뷰에 남겨두었습니다

현재 step1 내용과 충돌이 발생했는데요, 한 번 해결해보시면 좋겠습니다~!

Comment on lines +24 to +27
const [detail, options] = await Promise.all([
getProductsDetail(product),
getProductsOptions(product),
])
Copy link

Choose a reason for hiding this comment

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

여러 api를 호출해야 한다면 useQueries를 사용해보면 좋을 것 같아요!
https://tanstack.com/query/latest/docs/framework/react/reference/useQueries

Comment on lines 17 to 50
const handleFinish = () => {
const number = numberRef.current?.value;
const message = messageRef.current?.value;

// 카드 메시지가 100자를 초과할 경우 경고 표시
if (message && message.length > 100) {
alert('카드 메시지는 100자 이내로 입력해 주세요!');
return;
}

// 현금 영수증 신청을 한 경우 (체크 박스 활성화 상태)
if (check) {
if (number && message) {
if (Number.isNaN(Number(number))) {
alert('현금 영수증 전화번호는 숫자만 입력 가능합니다!');
} else {
alert('주문이 완료되었습니다.');
}
} else if (!message) {
alert('선물과 함께 보낼 카드 메시지를 작성해 주세요!');
} else if (!number) {
alert('현금 영수증에 필요한 전화번호를 입력해주세요!');
}
}

// 현금 영수증 신청을 하지 않은 경우 (체크 박스 비활성화 상태)
else {
if (message) {
alert('주문이 완료되었습니다.');
} else if (!message) {
alert('선물과 함께 보낼 카드 메시지를 작성해 주세요!');
}
}
};
Copy link

Choose a reason for hiding this comment

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

이전 pr에서 언급한 것처럼, if문 사용이 가독성을 해치고 있어요. 개선해보시면 좋겠습니다!

Copy link
Author

Choose a reason for hiding this comment

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

if (check) {
    if (!number) {
        alert('현금 영수증 번호를 입력해주세요');
        return;
    }

    if (!message) {
        alert('메시지를 입력해주세요');
        return;
    }

    if (Number.isNaN(Number(number))) {
        alert('현금 영수증 번호는 숫자만 입력해주세요');
        return;
    }

    alert('주문이 완료되었습니다.');
} else {
    if (!message) {
        alert('메시지를 입력해주세요');
        return;
    }

    alert('주문이 완료되었습니다.');
}

const navigate = useNavigate()

// 선물 최대 제한 수량
const giftOrderLimit = data?.options?.giftOrderLimit ?? 0
Copy link

Choose a reason for hiding this comment

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

0으로 방어를 하셨군요...? 그래도 괜찮나요??

Copy link
Author

Choose a reason for hiding this comment

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

어 .. 임의로 넣은 값이긴 합니다

0으로 undefined 를 방어하면 선물하기가 안되긴 하겠네요.. ㅠㅠ

Comment on lines +33 to +39
if (value > giftOrderLimit) {
alert(`최대 선물 가능 수량은 ${giftOrderLimit}개 입니다.`)
setItemCount(giftOrderLimit)
}
else {
setItemCount(value)
}
Copy link

Choose a reason for hiding this comment

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

else로 연결하는 방식을 많이 사용하시는 것 같아요. 대신 얼리 리턴에 익숙해지는 것이 좋을 것 같습니다

Comment on lines 67 to 72
useEffect(() => {
if (data) {
setTotalPrice(data.detail.price.basicPrice * itemCount)
console.log(`상품명: ${data.detail.name} / 선물 최대 제한 수량: ${giftOrderLimit}`)
}
}, [data, itemCount, giftOrderLimit])
Copy link

Choose a reason for hiding this comment

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

지난 pr과 동일하게, useEffect를 사용하지 않는 방식으로 수정해보시면 좋을 것 같아요!

@hyunaeri
Copy link
Author

고생하셨습니다~ 여러 api콜을 날리는 것은 useQueries를 사용하시면 됩니다! 리뷰에 남겨두었습니다

현재 step1 내용과 충돌이 발생했는데요, 한 번 해결해보시면 좋겠습니다~!

충돌이 발생한 5개의 파일에 대하여 충돌 해결했습니다!

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