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.

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

[3팀 김나연][Chapter 1-3] React, Beyond the Basics #34

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Blue-Kite
Copy link

@Blue-Kite Blue-Kite commented Jan 2, 2025

과제 체크포인트

기본과제

  • shallowEquals 구현 완료
  • deepEquals 구현 완료
  • memo 구현 완료
  • deepMemo 구현 완료
  • useRef 구현 완료
  • useMemo 구현 완료
  • useDeepMemo 구현 완료
  • useCallback 구현 완료

심화 과제

  • 기본과제에서 작성한 hook을 이용하여 렌더링 최적화를 진행하였다.
  • Context 코드를 개선하여 렌더링을 최소화했다.

과제 셀프회고

이번 과제는 리액트 코드라서 조금 더 재미있었습니다. 타입 에러는 항상 마주할 때마다 당황스러운 거 같습니다.

기술적 성장

  • 리액트 훅의 개념, 사용하는 이유, 주의점에 대해 복습할 수 있었습니다.
  • 최적화에 대해 고민해볼 수 있었습니다.

코드 품질

  1. AppContext과 테마, 유저, 알림 기능이 모두 있는 거 같아서 관심사 분리했습니다.
  2. useCallback, useMemo 훅을 사용해 메모제이션 했습니다. context로 넘겨주는 함수라서 자식 컴포너트에 전달할 때 사용하면 유용하다는 케이스에 부합한 거 같았습니다.
  3. useState에서 초기화함수를 사용하면 매 랜더링마다 객체를 생성하지 않을 수 있다는 걸 배웠습니다.

학습 효과 분석

과제 피드백

이전 과제와 달리 commit 할때 tsc 에러가 나서 슬랙을 참고해 아래 부분을 변경했습니다. 근데 다른 분 PR를 확인하니 바꾸지 않으신 분들도 있으셔서 원인이 궁금했습니다...

"lint-staged": {
        "*.{js,jsx,ts,tsx}": [
            "tsc --noEmit --jsx react-jsx  --allowJs --skipLibCheck",
            "prettier --write",
            "eslint --fix"
        ]
    },

리뷰 받고 싶은 내용

  • 마지막 테스트 코드를 통과하기 위해 다음과 같이 변경했습니다.
image

useState 초기화할 때 아래처럼 했는데 이렇게 작성하는게 좋을까요
실무에서 useCallback, useMemo를 자주 사용하시는지 사용 기준이 있으신지 여쭈어보고 싶습니다.
그리고 React.memo() 컴포넌트를 감싸면 굳이 개별적으로 함수나 객체를 메모제이션을 할 필요가 없을 거 같은데... 훅을 사용하는 이유가 있을까요? 두 방식의 차이가 궁금했습니다.

  • equalities 폴더의 두 함수에서 사용한 이 코드를 개선할 방법이 있을까요. any -> unknown -> as 로 바꾸었는데 타입 단언 외의 방법은 모르겠습니다. any처럼 타입 단언도 가능하면 사용하지 않으신가요?
image

@Blue-Kite Blue-Kite changed the title [3팀] 김나연 [3팀][Chapter 1-3] React, Beyond the Basics Jan 2, 2025
@Blue-Kite Blue-Kite changed the title [3팀][Chapter 1-3] React, Beyond the Basics [3팀 김나연][Chapter 1-3] React, Beyond the Basics Jan 2, 2025
Copy link

@2dowon 2dowon left a comment

Choose a reason for hiding this comment

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

types, hooks 등 최대한 관심사 별로 코드를 잘 나눠주신 것 같아요! 👍👍
이번 주차도 고생 많으셨습니다! 새해 복 많이 받으세요!

(+)
deepMemo, useDeepMemo 관련 파일이 없는 것 같아요!

Comment on lines +1 to +6
export * from "./Theme/ThemeContext";
export * from "./Theme/ThemeProvider";
export * from "./User/UserContext";
export * from "./User/UserProvider";
export * from "./Notification/NotificationContext";
export * from "./Notification/NotificationProvider";
Copy link

Choose a reason for hiding this comment

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

이렇게 따로 index 파일 만들어서 export 하는 이유가 있을까요??? 궁금해서 여쭤봅니다!!

Copy link
Author

@Blue-Kite Blue-Kite Jan 3, 2025

Choose a reason for hiding this comment

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

import문을 조금 더 깔끔하게 하려고 했는데 App.tsx를 제외하고는 다른 context와 함께 사용하는 경우가 없어 큰 효용은 없는거 같습니다...

Comment on lines +6 to +13
if (
typeof objA !== "object" ||
objA === null ||
typeof objB !== "object" ||
objB === null
) {
return false;
}
Copy link

Choose a reason for hiding this comment

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

typeof null이 object이기 때문에 objA === nullobjB === null 경우는 조건에 없어도 될 것 같아요!
image

Copy link

Choose a reason for hiding this comment

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

저도 궁금해하던 부분이었는데 나연님 코드와 도원님 피드백에 도움 얻고 갑니다!!

Copy link
Author

@Blue-Kite Blue-Kite Jan 3, 2025

Choose a reason for hiding this comment

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

엇 근데 null 체크를 안 하면 const keysA = Object.keys(objA); 에러가 나는데 if 문에서 체크를 안하고 타입 단언을 사용하는게 좋은가요?

src/@lib/equalities/shallowEquals.ts Outdated Show resolved Hide resolved
src/components/ItemList.tsx Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants