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

[김민찬] Sprint8 #247

Conversation

itscold96
Copy link
Collaborator

@itscold96 itscold96 commented Aug 2, 2024

배포

주소: https://panda-market-itscold96.netlify.app/

요구사항

기본

  • 스프린트 미션 1 ~ 7에 대해 typescript를 적용해주세요

주요 변경사항

  • 프로젝트 내 오타 수정
    • 오타 검토 확장 프로그램을 사용하여 수정하였습니다.
  • 미디어쿼리 사이즈 변화 문자열 상수화
    • 기존에는 단순히 화면 크기만 상수화한 것을 미디어 쿼리 사이즈 문자열 전체로 상수화하였습니다.
      image
  • 로그인 폼, 회원가입 폼 컴포넌트에서 사용하던 useValidForm 커스텀 훅을 리팩토링하였습니다.
    1. useValidForm 로직을 useEmail, usePassword, useNickName, useVerifyPassword로 분리
    2. useEmail, usePassword을 합쳐서 useLogin 훅 제작
    3. useEmail, usePassword, useNickName, useVerifyPassword을 합쳐서 useSignup 훅 제작
    • useValidForm에 집중되어 있던 form 관련 로직을 각 input 용도에 따라 분리(관심사의 분리)하였습니다.
    • 이로써 특정 input에 문제가 생기면 해당 input 관련 hook만 확인하면 되어,
      유지 보수성을 개선하였습니다.
    • 이전에는 모든 유효성 검사 로직이 useValidForm 훅 안에 모두 모여 있어,
      만약 이메일만 유효성 검사를 해야하는 요구사항이 발생하는 경우 재사용성에 문제가 있었으나,
      이제는 각각을 훅으로 따로 분리하여 원하는 대로 조합할 수 있게 되었습니다.
    • useLogin, useSignup 훅에서 반환 객체 내부를 논리적으로 그룹화 하였고,
      로그인 폼, 회원가입 폼에서는 모든 폼의 입력 및 유효성 검사 관련 로직은 커스텀 훅 내부에 있어,
      사용부에서의 코드 가독성을 개선하였습니다.
      image
      image
      image

멘토에게

  • 저는 Part2 시작 때, 스프린트 1~4를 리액트로 마이그레이션하면서 타입스크립트도 함께 적용시켜 놓았습니다.
    따라서 해당 PR에는 타입스크립트로 마이그레이션하는 과정은 따로 없고, 오타와 같은 작은 수정들과 훅 리팩토링이 포함되어 있습니다.
  • 로그인, 회원가입 입력 및 유효성 검사 폼 관련 로직을 이전에 useValidForm 커스텀 훅으로 만들어두었는데,
    모든 입력 관련 로직을 하나의 커스텀 훅에서 관리하는 것이 오히려 관심사의 분리를 하지 못하고, 유지 보수성을 떨어 뜨리는 것 같아,
    각각의 서로 다른 유효성 검사를 가진 input들을 따로 커스텀 훅으로 분리한 뒤, 로그인, 회원가입 목적에 맞게 필요한 것만 뽑아서
    useLogin, useSignup 훅을 만들어 보았습니다. 해당 부분이 좋은 선택이었는 지 궁금합니다.
  • 주로 서로 다른 유효성 검사를 필요로 하거나 복잡하게 연관된 입력들이 많은 입력 폼은 어떤 식으로 구현하게 되는 건지 알려주시거나,
    참고할 만한 사이트를 알려주신다면 감사하겠습니다.
  • 현재 로그인, 회원가입 폼에서 최초 렌더링 시에는 유효성 검사를 시행하지 않고, 한 번이라도 입력이 들어오는 경우에만 유효성 검사를 실시할 수 있도록 최초 렌더링 시에는 사이드 이펙트를 제거하는 useDidMountEffect 커스텀 훅과 validType('default', 'valid', 'invalid')으로 제어하고 있습니다.
    간단히 설명을 덧붙이자면, 최초 렌더링시에 사이드 이펙트를 제거하여 validType을 초깃값인 default로 유지시키고,
    입력이 들어오기 전까지는 validType을 default로 두어 유효성 에러를 내지는 않지만, 유효성 검사를 통과 할 수도 없게 만드는 로직입니다.
    일반적으로 페이지 첫 진입 시에 입력폼 유효성 검사를 막을 때, '최초 렌더링 시에는 유효성 검사를 시행하지 않고, 한 번이라도 입력이 들어오는 경우에만 유효성 검사를 실시할 수 있도록' 이라는 목적을 달성하기 위해 제가 사용한 방법이 올바른 방법인지, 아니면 더 간단한 방법이 있는데 굳이 어렵게 돌아가고 있는 건지 궁금합니다.

unvalid -> invalid
useValidForm을 useEmail, useNickname, usePassword, useVerifyPassword로 분리
is- 접두사는 boolean 값을 반환할 거라 기대하기 마련인데
실제로 boolean값이 아닌 값이 들어오고 있었기 때문에,
$validType으로 변경
이벤트의 종류가 뒤로 가도록 수정
useEmail, usePassword 커스텀 훅을 합하여 로그인 폼에 쓰일 커스텀 훅 제작
useEmail, usePassword, useNickname, useVerifyPassword 를 모아 회원가입용 커스텀훅을 제작
@itscold96 itscold96 requested a review from lisarnjs August 2, 2024 13:38
@itscold96 itscold96 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Aug 2, 2024
리팩토링 과정 중에 잔재하던 불필요한 useEffect 제거
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.

확실히 변경사항은 특별한 게 없어서 남겨주신 질문들 답변으로 대체할게요!

  1. 관심사의 분리를 선택하신 것은 너무 좋습니다, 훅을 분리한 이유가 그러하다면 더욱 좋습니다. 다만 기능별 훅을 불리하는 것이 아닌 로직과 UI 개념으로 관심사를 분리할수도 있을 것 같네요 👍
  2. 입력폼이 많으면 요즘 react-hook-form을 현업에서도 많이들 사용하시더라구요. 폼 컨트롤 제어 방법이 따로 있는 것은 아니지만 어느정도 사용법이라던지 , 리렌더링을 최소화시켜준다던지 등 장점이 많기 떄문에요.
  3. default를 초기값으로 설정하신 것 너무 잘하셨는데요? 저는 보통 -1이나 null을 초기값으로 사용하곤 합니다 👍 굳이 어렵게 돌아가고 계시지 않아요! 잘하셨습니다!

간단한 수정들이지만 민찬님들의 고민 흔적이 보여요..!
저도 민찬님처럼 공부하고 개발했으면 지금 더 잘했을 것 같은데 마인드도 본받아야겠어요!
최고입니다! 다음 코드도 기대되네요 :)

이번 한주도 화이팅이에요!

@@ -74,7 +74,7 @@ const StyledItemRegistSection = styled.section`

cursor: pointer;

@media (max-width: ${TABLET_MAX_WIDTH}px) {
@media ${MEDIA_QUERY_SIZE.underTablet} {
Copy link
Collaborator

Choose a reason for hiding this comment

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

확실히 사용하는 것도 덜 귀찮아지고, 직관적이여서 좋네요 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

useEmail, usePassword 들의 구조가 비슷한 것 같은데 (사용되는 state의 종류가 비슷하다던지?) 이 부분을 조금 개선해본다면 어떻게 해볼 수 있을지 고민해보면 좋을 것 같아요!

@lisarnjs lisarnjs merged commit ec23c35 into codeit-bootcamp-frontend:React-김민찬 Aug 5, 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