feat: implement layout & side nav bar #6
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Icon Component 들은 무시하셔도 괜찮습니다! |
woo-jk
approved these changes
Apr 24, 2024
Comment on lines
+22
to
+44
| const navItems: NavItem[] = useMemo( | ||
| () => [ | ||
| { | ||
| href: '/', | ||
| title: '파워업 퀴즈', | ||
| Icon: PowerUpIcon, | ||
| segments: [['(quiz)']], | ||
| }, | ||
| { | ||
| href: '/review', | ||
| title: '복습 체크', | ||
| Icon: ReviewCheckIcon, | ||
| segments: [['review']], | ||
| }, | ||
| { | ||
| href: '/repository', | ||
| title: '공부 창고', | ||
| Icon: StudyRepositoryIcon, | ||
| segments: [['repository']], | ||
| }, | ||
| ], | ||
| [], | ||
| ) |
There was a problem hiding this comment.
혹시 이 데이터들을 useMemo가 아닌, 컴포넌트 외부의 변수로 빼는 방식은 어떤가요?
useMemo를 사용하면 매번 렌더링마다 비교 로직을 실행하기 때문에 미미하지만 성능의 차이가 있다고 알고있어서요!
There was a problem hiding this comment.
@woo-jk
제안해주신 아이디어는 굉장히 좋은 것 같습니다!
다만 다음의 문제 또한 생각해볼 만 할 것 같습니다.
- navItems를
전역 변수로 선언한다면findActiveNav의 인자로 navItems를 넘길 필요가 없어지면서activeItem을 찾는 로직이 분산된다 - 아래의 useMemo의 의존성 배열에 navItems을 지정할 수 없다 (전역 변수는 useMemo의 변경 감지를 벗어남)
const activeItem = useMemo(() => findActiveNav(navItems, segments), [navItems, segments])navItems의 의존성 배열은 빈 배열로 성능 이점을 기대하기 어려운 점
이 경우에는 2번 때문에라도 전역변수로 활용하기는 어려울 것 같습니다
There was a problem hiding this comment.
넵 확인했습니다~👍 말씀해주신 이유로 useMemo를 써야겠네요 ㅎㅎ (성능 문제는 제가 잘못 알고 있었군요)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
개요
세부 내용
관련 링크