-
Notifications
You must be signed in to change notification settings - Fork 1
[Refactor] react-hook-form 도입 #593
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
Conversation
- "Password field is not contained in a form" 에러로 인해 form 태그 사용
|
👀 코드리뷰 전! Storybook 미리보기: https://66bae48876a5167719ee1778-rharcadvqu.chromatic.com/ |
|
👀 코드리뷰 전! Storybook 미리보기: https://66bae48876a5167719ee1778-vesxeujohr.chromatic.com/ |
yonghyeun
left a comment
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.
전체적으로 수정하시느라 고생하셨습니다.
리액트 훅 폼 라이브러리의 전체적인 동작 방식을 모두 읽지는 않아서 자세힌 모르지만
ref 와 onChange 이벤트 핸들러를 이용해서 굳이 state 들을 선언하지 않고 사용 할 에러와 밸리데이션에 대한 값들만 선언적으로 jsx 안에서 regeister 메소드를 통해 설정해준는 멋진 라이브러리인 거 같군요
실제로 쏴님 말씀대로 이곳 저곳 상태를 탐방하지 않아도 되어서 좋은거 같습니다.
고생하셨습니다!!
이 부분은 작업과 크게 관련이 없는 이야기긴 하지만
상위에서 선언된 객체를 클로저 형태로 바라보고 있는 커스텀 훅을 생성하여 반환하는 것이 가능하더군요
이런 식으로요 (정말 신기하더군요)
const 어쩌구저쩌구 = ()=>{
let 어쩌구obj = {};
return ()=>{
const [...] = useState(()=> obj.어쩌구저쩌구)
return ..
}
}나중에 기회가 된다면 이 클로저 패턴을 이용해서 폼을 관리해봐도 좋을 거 같다는 생각이 들더군요
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.
현재 모든 PR 을 읽은게 아니라 순차적으로 읽고 있지만 이런 식으로 메시지를 관리하는 것이 나중에 테스트 코드를 작성하거나 수정 할 때도 좋은 거 같더군요
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.
상수로 관리 안하면 메세지 하나를 수정해도 폼이랑 테스트 코드 둘 다 고쳐야 하더라구요.
메세지를 변수화하면서 요구 사항이 변경되면 값만 수정하면 되니 편했습니다 👍
src/features/auth/lib/validate.ts
Outdated
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.
// 정규표현식
export const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;
export const passwordRegex =
/^(?=.*[a-zA-Z])(?=.*[0-9])(?=.*[!@#$%^&*()_+]).{8,15}$/;
export const nicknameRegex = /^[a-zA-Z0-9가-힣]{1,20}$/;에서 위 Regex 값들을 정규표현식 인스턴스로 생성했는데 그렇다면 lib 이 아니라 constants 로 옮기거나
메소드로 변경하는건 어떨까요 ?
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.
다른 사람들이 봐도 이해할만한 테스트로 작성하면 읽기 좋다고 생각해서 바꿔봤습니다!
테스트가 너무 많아 모두 다 뜯어 고치지 못한 점이 아쉽네요 😓
| passwordSetFormValidationMessage, | ||
| } from "../constants"; | ||
|
|
||
| const PasswordInput = forwardRef< |
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.
해당 컴포넌트의 명을 import 해오는 컴포넌트의 이름으로 짓느라 _PasswordInput 과 같이 기존 컴포넌트 명을 다르게 명명하여 사용하지 말고
newPasswordInput 이나 다른 이름을 붙혀 사용하면 어떨까요 ?
|
👀 코드리뷰 전! Storybook 미리보기: https://66bae48876a5167719ee1778-klmezggoxc.chromatic.com/ |
관련 이슈 번호
#589
설명
react-hook-form을 도입하게 된 계기
폼의 상태를 state로 관리했을 때 발생하는 불필요한 리렌더링과 props drilling으로 인해, 폼 상태를 전역 상태로 관리해왔습니다. 그러나, 전역 상태로 구현하면서 폼 상태는 지역 상태로 관리하는 것이 더 적합하다고 판단했습니다.
그 이유는 폼 상태는 폼이 포함된 컴포넌트에서만 유효하며, 해당 컴포넌트가 언마운트되면 초기화되기 때문입니다. 폼 상태는 특정 범위 내에서만 사용되기 때문에, 전역 상태보다 지역 상태로 관리하는 것이 더 적합하다고 생각합니다.
(zustand로 폼 상태를 관리할 경우, 폼 제출 후에도 상태가 유지되어 직접 상태를 초기화해야 했습니다.)
또한, 기존에는 폼 관련 코드가 분산되어 있어 코드를 이해하기 어려웠습니다. 필드가 어느 input에서 바뀌는지 알지도 못한 채 상태를 사용해야 했습니다. 반면, 필드 데이터를 props로 전달하면 의존성이 명확해져, 데이터가 어느 input에서 업데이트되는지 쉽게 확인할 수 있습니다. 뿐만 아니라, 재사용과 테스트하기 쉬워졌습니다.
react-hook-form은 지역 상태로 폼 상태를 생성합니다. 이 라이브러리는 비제어 컴포넌트 방식으로 구현되어 있기 때문에 렌더링 문제를 해결할 수 있습니다.
예를 들면, email input에 "a"라는 값을 입력했을 때 이메일 형식에 어긋나 한 번 렌더링되어 에러 메세지를 표시하고, 올바른 이메일 값을 입력할 경우 리렌더링되어 "사용 가능한 이메일입니다." 메세지를 표시합니다.
리팩토링
react-hook-form을 적용한 컴포넌트는 아래와 같습니다.
이미지를 추가/수정하는 폼은 이미지 압축하는 로직이 있어 리팩토링하지 않았습니다.
react-hook-form
가장 자세하게 나와있는 건 공식문서이지만, 제가 쓴 기능들은 다음과 같습니다.
handleSubmit(onSubmit, onError): 폼 제출하는 핸들러useStore.getState()와 같은 역할가끔 Controller 컴포넌트를 볼 수 있는데요. controlled 컴포넌트처럼 작동되기 위해 사용했습니다.
예를 들면, 제가 바텀 시트에서 클릭한 성별을 버튼에 바로 표시해야 하는 경우에 사용했습니다.
이 때 useForm에서 반환하는 control를 props로 넘겨줘야 합니다.