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.

Sign up for GitHub

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

Jump to bottom

refac: 최적화 적용 #40

Merged
merged 19 commits into from
Apr 2, 2023
Merged

refac: 최적화 적용 #40

merged 19 commits into from
Apr 2, 2023

Conversation

nno3onn
Copy link
Collaborator

@nno3onn nno3onn commented Mar 18, 2023

memo, useMemo, useCallback 훅을 사용하여 최적화 진행 중입니다.

#36 by @nno3onn

@nno3onn nno3onn added Front End Front End Issues Refactor 코드 리팩토링 및 수정 labels Mar 18, 2023
@nno3onn nno3onn requested a review from krokerdile March 18, 2023 12:46
@nno3onn nno3onn self-assigned this Mar 18, 2023
@nno3onn nno3onn linked an issue Mar 21, 2023 that may be closed by this pull request
memo, useCallback, useMemo 적용 #42
Open
krokerdile
krokerdile reviewed Mar 27, 2023
View reviewed changes
Copy link
Collaborator

@krokerdile krokerdile left a comment

Choose a reason for hiding this comment

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

icon에 memo 확인 했습니다.
는 text={}에서 안에 넣어주는 걸로 바꼈네요? 이게 더 좋은 건가?

@nno3onn
Copy link
Collaborator Author

nno3onn commented Mar 27, 2023 •
edited
Loading

icon에 memo 확인 했습니다. 는 text={}에서 안에 넣어주는 걸로 바꼈네요? 이게 더 좋은 건가?

처음에 DefaultForm 컴포넌트에서 text 타입으로 string | React.ReactElement를 주고 몇몇 부분에서 ReactElement 타입을 전해줄 때 fragment(<></>)로 감싼 형태로 전달하는 방식보다는

const LoginForm = forwardRef<HTMLInputElement, LoginFormProps>(
  ({ text, type = "text" }, ref) => (
    <>
      <DefaultForm
        text={
            <>
                {text}
                <SpanText text=" *" color="red" />
            </>
        }
        ref={ref} 
        type={type}
      />
    </>
  )
);

children을 컴포넌트가 감싸는 형태가 더 직관성이 좋다고 생각이 들어서 시작하게 되었습니다!

const LoginForm = forwardRef<HTMLInputElement, LoginFormProps>(
  ({ text, type = "text" }, ref) => (
    <>
      <DefaultForm ref={ref} type={type}>
        {text}
        <SpanText text=" *" color="red" />
      </DefaultForm>
    </>
  )
);

그리고 이에 따라 Text를 가져오는 컴포넌트들은 모두 동일한 형태로 가져오는 것이 일관성이 있을 것 같아 Text 컴포넌트도 동일한 형태로 변경했습니다.


또한 이렇게 바꾸니 Text에 text라는 Props가 있을 때는 text로 전달해주는 텍스트 길이가 긴 경우가 많은데 이런 경우에

<Text
  text={...}
  property1={...}
  property2={...}
  ...
/>

와 같이 줄이 길어져서 가독성이 좋지 않더라구요. text props를 제거하니

<Text property1={...} property2={...} >
  텍스트
</Text>

와 같이 한줄에 Props를 나타낼 수 있어서 깔끔해지는 효과도 가져올 수 있었습니당

@nno3onn
Copy link
Collaborator Author

nno3onn commented Mar 27, 2023

icon에 memo 확인 했습니다. 는 text={}에서 안에 넣어주는 걸로 바꼈네요? 이게 더 좋은 건가?

최적화 적용에 해당되는 부분은 아니지만.. 최적화하는 과정에서 눈에 밟혀서 변경하게 되었습니다

@nno3onn nno3onn merged commit 500ed0c into main Apr 2, 2023
@nno3onn nno3onn deleted the refac/optimization branch April 2, 2023 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krokerdile krokerdile krokerdile approved these changes

Assignees

@nno3onn nno3onn

Labels
Front End Front End Issues Refactor 코드 리팩토링 및 수정
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

memo, useCallback, useMemo 적용
2 participants
@nno3onn @krokerdile