Skip to content

test: 주석 추가2 - #6

Open
lywoo00 wants to merge 1 commit into
mainfrom
fearture/search
Open

test: 주석 추가2#6
lywoo00 wants to merge 1 commit into
mainfrom
fearture/search

Conversation

@lywoo00

@lywoo00 lywoo00 commented Nov 12, 2025

Copy link
Copy Markdown
Contributor

📌 PR 개요

  • 이 PR이 어떤 목적을 가지고 있는지 간단히 설명해주세요.

🔍 관련 이슈

  • Closes #이슈번호
    (ex. Closes #12)

🔧 변경 유형

해당하는 항목에 체크해주세요.

  • ✨ feat (새 기능 추가)
  • 🐛 fix (버그 수정)
  • 📝 docs (문서 수정)
  • 🎨 style (코드 스타일 변경)
  • ♻️ refactor (리팩토링)
  • ✅ test (테스트 코드)
  • 🛠 chore (빌드/환경설정)

✨ 변경 사항

  • 주요 변경 내용을 리스트로 정리해주세요.

ex

  • 로그인 API 연동 (/api/login) 추가
  • 로그인 폼에서 이메일/비밀번호 유효성 검사 로직 추가
  • 로그인 성공 시 JWT 토큰을 localStorage에 저장하도록 수정
  • UI: 로그인 버튼 클릭 시 로딩 스피너 추가

📝 PR 제목 규칙

PR 제목은 커밋 컨벤션을 따라야 합니다.
ex) feat: 롤링페이퍼 작성 기능 추가 (#15)

✅ 체크리스트

  • 코드가 정상 동작함
  • 빌드 및 실행 확인 완료
  • 리뷰어가 이해하기 쉽게 변경 이유를 설명했음

📸 스크린샷 (선택)

  • UI 변경이 있다면 캡처 이미지 첨부

🤝 기타 참고 사항

  • 리뷰어가 참고하면 좋을 추가 맥락(설계 의도, 제약사항 등)

@lywoo00
lywoo00 requested a review from yujinimda November 12, 2025 12:44
// 들어가세요
const Search = ({ placeholder, onKeywordChange }: SearchProps) => {
const [value, setValue] = useState('');
const [isFocused, setIsFocused] = useState<boolean>(false);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

초기값이 false여서 추론 가능합니다!
boolean 제너릭빼도 될꺼같아요! 근데 원시값은 추론 가능하다는데 어디까지인지 잘 모르겠어서 이거 물어보고오쎄요!

const fetchProducts = async () => {
setLoading(true);
try {
const response = await productListApi.getProducts({

@yujinimda yujinimda Nov 12, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

저 이 api 요청하는 부분을 sevice로 빼서 import해서 사용하고있어요!
service 없이 훅에서 직접 호출하면 비즈니스 로직 과 통신 로직 을 둘 다 담당해서 훅이 점점 커진대요!
그런데 api 구조가 단순하고 변경가능성이 없다면 사실 sevice로 빼는게 의미가 없다고 하긴해요
API 호출이 여러 곳에서 반복되고 기본값, 응답 가공, 에러 변환가 필요할 때 분리가 필요하다구 했습니다!

});

// 여기는 디바운스에요
// 여기는 디바운스에요~~~

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

이건 따로 커스텀훅을 빼도 될꺼같아요!

setPage(1);
};

if (error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

얼리 리턴문 좋대요!

setPage(1);
};

const handleChangeOrder = (newOrder: string) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

이거는 처음에 string이라서 다른 문자열이 들어올수도 있다고 해요!
그래서 as로 이건 'recent' | 'favorite'이야! 하고 강제로 캐스팅하는건데 이거는 런타임 안전성이 없다구합니다!

애초에

const handleChangeOrder = (newOrder: 'recent' | 'favorite') => {
setOrder(newOrder);
};

이렇게 받으면 되는거 같은데

const [order, setOrder] = useState<'recent' | 'favorite'>('recent'); 이미 이게있어서

const handleChangeOrder = (newOrder: typeof order) => {
setOrder(newOrder);
};

이걸로 쓰면 될꺼같아요!

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