-
Notifications
You must be signed in to change notification settings - Fork 42
[김성주] sprint8 #227
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
The head ref may contain hidden characters: "React-\uAE40\uC131\uC8FC-sprint8"
[김성주] sprint8 #227
Conversation
대신 레지스터 등록용 ValidRules 안에 validate의 두번째 인자로 비밀번호 값 받기
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.
수고하셨습니다!
잘하시는데 타입을 너무 어렵게 쓰시는 경향이 있네요 ㅎㅎ
코드 작성은 항상 쉽고 직관적이게! 그래야 나중에 유지보수할때 편하다는걸 잊지맙시다 :)
타입스크립트 활용 위주로 피드백 드려봤으니 보시고 수정해보시면 될것같아요!
주요 리뷰 포인트
- useValidate hook 타입 정의 리팩토링
- 생산성, 유지보수, 협업을 위해 객체나 함수에는 타입 정의를 꼭 해주기
- reack hook form + zod 활용법
- 불필요하고 복잡한 타입 구문을 제거하기
src/hooks/useValidate.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.
타입을 너무 어렵지 않게 쓰는게 좋을것같아요! ㅎㅎ
주석과 같이 정리해보면서, 이런식으로 좀더 쉽고 직관적이게 타입을 사용해보는건 어떨까요?
// 입력 필드 타입 정의
type FieldName = 'user-email' | 'user-password' | 'user-name' | 'user-password-check';
// 개별 필드 상태 타입
interface FieldState {
value: string;
error: boolean;
errorMessage: string;
isValid: boolean;
}
// 전체 폼 상태 타입
interface FormState {
fields: Record<FieldName, FieldState>;
}
// 액션 타입 정의
type ValidationAction = {
type: 'UPDATE_FIELD';
payload: {
name: FieldName;
value: string;
};
};
// 초기 상태 정의
const createInitialFieldState = (): FieldState => ({
value: '',
error: false,
errorMessage: '',
isValid: false,
});
const initialState: FormState = {
fields: {
'user-email': createInitialFieldState(),
'user-password': createInitialFieldState(),
'user-name': createInitialFieldState(),
'user-password-check': createInitialFieldState(),
},
};
src/pages/Auth/FormInput.tsx
Outdated
import clsx from 'clsx'; | ||
|
||
//레지스터 등록용 | ||
const VALID_RULES = { |
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.
혹은, react hook form도 쓰고계시니 zod 스키마를 같이 활용해보는 방법은 어떨까요?
스키마를 작성하면 타입을 스키마로부터 자동 추론하기때문에 타입 검사와 유효성 검사 처리를 간결하면서도 매끄럽게 처리할 수 있는 장점이 있습니다.
react hook form과 같이 쓰인 예시는 아래 아티클 읽어보시고 참고해보시면 좋을것같네요! :)
참고
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 { z } from 'zod';
// Zod 스키마 정의
export const authSchema = z
.object({
'user-email': z
.string()
.min(1, '이메일을 입력해주세요')
.email('이메일 형식에 맞지 않습니다.'),
'user-password': z
.string()
.min(1, '비밀번호를 입력해주세요.')
.min(8, '비밀번호를 8자 이상 입력해주세요.'),
'user-name': z.string().min(1, '이름을 입력해주세요.'),
'user-password-check': z.string().min(1, '비밀번호를 다시 입력해주세요.'),
})
.refine((data) => data['user-password'] === data['user-password-check'], {
message: '비밀번호가 일치하지 않습니다.',
path: ['user-password-check'],
});
export type AuthFormData = z.infer<typeof authSchema>;
const methods = useFormContext<AuthFormData>();
src/pages/Auth/FormInput.tsx
Outdated
type InferredT = typeof methods extends UseFormReturn<infer U> ? U : never; | ||
//원래 useFormContext가 프로바이더 내려줄 때 알아서 useForm<T>안에 넣어줬던 타입을 찾아오는 거 같은데 | ||
//그러면 name을 프롭으로 받을 때 무조건 Path<T>로 받아야 useFormState({name})에서 에러가 안 뜨더라구요... | ||
//그럼 Props<T>로 받아야 Path<T>로 설정해줄 수 있는데 | ||
//이러면 이 컴포넌트 호출 할 때마다 <T>를 또 붙여줘야하니까 | ||
//보기 지저분할 것 같아서 이런 식으로 T를 따로 추론해서 넣어봤습니다... | ||
const { errors } = useFormState<InferredT>({ name }); //여기서 name으로 따로 골라와야 개별로 감지 |
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.
이 경우에는 복잡한 타입 구문을 제거하고 추론되는 타입을 써주셔도 무방합니다.
타입 구문이 불필요한 경우에는 안써주시는것도 좋은 용법이랍니다 :)
질문에 대한 답변
타입 에러를 회피하려 쓰는것보다는 원래 목적에 맞게 사용하시는게 좋죠! |
-기존-> validRuleObj에서 keyof typeof로 뽑아 name타입 지정 -변경-> 리터럴 타입+유니온으로 명확하게 정하기
-없어도 에러는 뜨지 않지만 any로 추론됨 -InferredT를 통해 useFormState에 인자로 들어가는 name이 유효한 타입인지 (Path<T>에 해당하는지) 정확히 판단하기 위해서는 있어야 한다고 생각
-기존 InferredT 통해 useFormState에 인자로 들어가는 name이 유효한 타입인지 (Path<T>에 해당하는지) 정확히 판단하기 위해 사용 ->근데 이제 사라졌으니 필요x -조드 스키마 넣으면서 바뀐점들 ->기존 VALID_RULES에정의해둔 함수들을 레지스터에 2번째 인자로 넘겨줄 필요x ->리졸버가 대신 해주니까//근데 리졸버에 넣을 스키마랑 useForm<T>호출에 필요한 타입의 필드가 일치해야 하는 거 같음 -> 따라서 스키마를 해당 페이지에 맞게 새로 만들거나 가공이 필요
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게
마이그레이션이라서 더 그렇게 느껴지는 건지 모르겠지만
지금 타입스크립트를 쓰는 방식이 안정성을 위해서 쓴다는 느낌보다
에러가 뜨니까 이걸 회피하려고 쓰는 것처럼 느껴져서 뭔가 공부 방법이 잘못된 것 같다는 생각이 듭니다.
아예 처음부터 ts로 만들어보는 것도 좋은 방법일까요??
아니면 이렇게라도 계속 쓰다보면 자연스레 나아지는 현상인지 궁금합니다!
셀프 코드 리뷰를 통해 질문 이어가겠습니다.