-
Notifications
You must be signed in to change notification settings - Fork 37
[김찬기] Sprint8 #275
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 #275
The head ref may contain hidden characters: "React-\uAE40\uCC2C\uAE30-sprint8"
Conversation
- 통신에러와 필드에러값을 넘겨줄수있도록 변형
- 기본값 수정 - type 수정
- 리액트훅폼에 맞게 변경
- 리액트훅폼 적용 - 스키마 작성
- 테스트용으로 썻던 스키마 옵션 정리 - 리팩토링하면서 불필요해진 프롭스 삭제
- ref를 안쓰더라도 적으라는 경고 처리
| function handleSumbitWithError(submitFn: (data: T) => Promise<void>) { | ||
| return handleSubmit(async function (data) { | ||
| try { | ||
| setFormError(null); | ||
| await submitFn(data); | ||
| } catch (error) { | ||
| setFormError(error as 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.
react hook form에서는 폼제출의 에러상태는 따로 관리해야하는것 같아서 커스텀훅에 같이 포함시켰습니다.
- 공통 타입 분리 - controlled 컴포넌트는 controller를 써서 작성하도록 변경 - custom react hook form에서 watch값 삭제
src/types/common.ts
Outdated
| export type ListQueryParams = { | ||
| page: number; | ||
| pageSize: number; | ||
| keyword: string; | ||
| orderBy: string; | ||
| }; | ||
|
|
||
| export type PaginationResponse<T> = { | ||
| totalCount: number; | ||
| list: T[]; | ||
| }; | ||
|
|
||
| export type BaseData = { | ||
| updatedAt: string; | ||
| createdAt: string; | ||
| id: number; | ||
| }; | ||
|
|
||
| export type DefaultFieldState = { | ||
| error?: FieldError | undefined; | ||
| invalid: boolean; | ||
| isTouched: boolean; | ||
| isDirty: boolean; | ||
| isValidating: 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.
추후 재사용하거나, 공통적으로 쓰이는것 같은 타입들을 분리했는데,
최대한 직관적이게 이름을 지어보려고했는데, 아직 좀 부족한것 같습니다.
- 멘토링시간에 배운 Record 활용
| if (!error) { | ||
| return null; | ||
| } | ||
| export function Error({ error }: { error: 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.
react hook form의 FieldError의 message를 전달받아서 랜더링하는 컴포넌트는
그냥 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.
넵~ react hook form에 종속적이지 않게 Error ui에 필요하다고 생각하는 인터페이스로 만드시면 좋아요.
- zod로 벨리데이션 옮김
withyj-codeit
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.
수고 많으셨어요~👍
| - useForm의 모드를 'onBlur'로 설정할때에는 필드에 전달된 props중 onBlur가 실행이 되어야 벨리데이션이 된다. | ||
| - 직접 값을 업데이트해줄때는 (특수한 필드의 경우), useForm의 반환값중 setValue를 이용하면 값 업데이트와 벨리데이션이 작동한다. | ||
| - react hook form에 컴포넌트를 연결할때에는 한겹의 어뎁터 레이어를 설정하여 컴포넌트 내부에서 react hook form의 의존성이 없도록 작성하는 방법을 사용하자. | ||
| - watch를 통해 각 필드에 value값을 전달하면 하나의 필드가 업데이트 될때마다 전체가 리랜더링이 되어버린다. (Controller, useController 등을 통해서 전달해야함) |
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 { | ||
| await onCommentSubmit(data); | ||
| alert(message); | ||
| window.location.reload(); |
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.
reload() 는 어떤 맥락에서 필요해요?
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.
코멘트 작성후에 데이터 갱신 및 폼초기화를 한번에 하려고 새로고침을 해버렸습니다.
기초프로젝트 이후에 다시 생각해보니 그냥 새로고침은 앱전체를 다시 리셋하는거라
선언적으로 state를 변화시켜서 앱ui상태를 업데이트하는방식으로 교체하겠습니다.
| if (!error) { | ||
| return null; | ||
| } | ||
| export function Error({ error }: { error: 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.
넵~ react hook form에 종속적이지 않게 Error ui에 필요하다고 생각하는 인터페이스로 만드시면 좋아요.
| // 특정한 행동에 onChange를 활용해 값을 업데이트할때에는 | ||
| // onBlur를 실행하지 못해서, onChange가 호출될때 함께 onBlur가 호출되도록 개선했습니다. | ||
| // | ||
| // (정상적으로 연결되는 컴포넌트에서도 한번더 벨리데이션을 호출한다고 해서 문제가 되지 않을것 같아서) |
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.
이 정도 이슈일 경우 onBlur를 묶어주는 것도 좋다고 생각해요.
이후에 특정 커스텀 컴포넌트 지원을 위해 이슈 케이스가 다양해지면 하나의 FieldAdapter에서 전부 처리하기보다 특정 케이스에 맞는 Adapter로 분리하는 것도 좋을 것 같아요.
| if (onClick) { | ||
| onClick(); | ||
| } | ||
| onClick && onClick(); |
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.
간결함을 위한 변경인 것 같아서 onClick?.() 도 권해봅니다.
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.
vite에 기본적으로 setting된 tsconfig에서는
onClick && onClick(); 이게 eslint 오류로 안잡혔는데
next프로젝트로 넘어와서는 이부분이
표현식의 결과값을 사용하지 않은 케이스라
no-unused-expressions rule에 위반이 된다고 해서
코멘트해주신방향으로 수정중에 있습니다 ㅠ
요구사항
기본
심화
주요 변경사항
멘토에게
react hook form을 쓰다보니 좀 궁금한점이 생겼습니다.자꾸 컴포넌트들이 특정 라이브러리의 인터페이스에 의존하게 되는것 같은데, 이렇게 수정을 진행도 맞는 방향일까요?리액트 훅폼에서 제공하는 폼상태, 필드상태, 에러등에 대한 인터페이스를 활용해서 컴포넌트를 수정해가고 있어서 든 의문점입니다.