-
Notifications
You must be signed in to change notification settings - Fork 3
[ 코드리뷰용 pr ] typescript로 마이그레이션 #18
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
base: empty
Are you sure you want to change the base?
Conversation
5wintaek
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.
봐줘요
| <OnboardingPage isLoadingPage={false} /> | ||
| ); |
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.
isLoading 이랑 setIsLoading props 지정 안해줘두 돼 ?
| const ModalPortal = ({ children }: ChildrenProps) => { | ||
| const el: HTMLElement | null = document.getElementById('modal'); | ||
| return ReactDOM.createPortal(children, el as Element | DocumentFragment); |
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.
이쪽 부분 설명좀 ..! el을 뭘 위해서 썻는지 ?
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.
return ReactDOM.createPortal(children, el as Element | DocumentFragment); 이거 타입 지정 규칙 찾아보기
| const Button = styled.button<{ $isQuiz: boolean }>` | ||
| background-color: ${({ theme, disabled }) => | ||
| disabled ? theme.colors.gray300 : theme.colors.prime}; | ||
| color: ${({ theme }) => theme.colors.white}; |
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.
왜 button 에 $isQuiz를 썻는지 궁금!
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.
$ : 프리픽스
프리픽스를 사용하지 않으면 styled-component 에 props를 넘겨줄 때 에러가 뜬다
✅ 작업 내용
📌 이슈 사항
1️⃣ 함수 역할 분리
단일 책임 원칙에 따라 하나의 함수가 하나의 역할만 수행하도록 최대한 함수를 분리했습니다!
타입 지정하는 부분도 하나의 폴더로 분리해서 타입별로 모아뒀습니다.
2️⃣ nullable, any 사용 지양
3️⃣ Early Return
✍ 궁금한 것
1️⃣ tsconfig.json / tsconfig.node.json
vite로 프로젝트를 구현하면 초기 생성 시에 항상 생기던 파일들이라 큰 생각을 안했었는데 이번에 마이그레이션하면서 보니까 쟤네 둘의 차이가 뭘까 . . 궁금하더라구요 !
그래서 가볍게 찾아봤고, 파일 내부는 vite로 프로젝트 만들었을 때 기본 생성되던 옵션 위주로 넣었습니다.
tsconfig.jsontsconfig.node.json