-
Notifications
You must be signed in to change notification settings - Fork 62
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
[7팀 김주광] [Chapter 1-3] React, Beyond the Basics #9
base: main
Are you sure you want to change the base?
Conversation
src/@lib/equalities/deepEquals.ts
Outdated
if ( | ||
typeof objA === "object" && | ||
typeof objB === "object" && | ||
Object.keys(objA).length === Object.keys(objB).length | ||
) { | ||
return Object.keys(objA).every((key) => deepEquals(objA[key], objB[key])); | ||
} |
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.
오.. 재귀 함수를 잘 이용하셔서 깔끔하게 작성하셨네요! 코드 잘보고 갑니다~
src/@lib/hooks/useCallback.ts
Outdated
@@ -1,10 +1,12 @@ | |||
/* eslint-disable @typescript-eslint/no-unused-vars,@typescript-eslint/no-unsafe-function-type */ | |||
import { DependencyList } from "react"; | |||
import { useMemo } from "./useMemo"; | |||
|
|||
export function useCallback<T extends Function>( | |||
factory: T, | |||
_deps: DependencyList, | |||
) { | |||
// 직접 작성한 useMemo를 통해서 만들어보세요. |
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.
구현 잘 해주셔서 주석 지워주셔도 될 것 같아요!
- context 분리 - memo 추가 - useCallback 추가
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.
코드 너무 잘 봤습니다!
같이 성장해 나갈 수 있게 되어서 좋네요!
이번 주차도, 이후 남은 주차도 열심히 해봐요!
// 2. 둘 다 객체인 경우: | ||
// - 배열인지 확인 | ||
if (Array.isArray(objA) && Array.isArray(objB)) { | ||
return objA.every((v, i) => deepEquals(v, objB[i])); |
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.
이부분에서도 길이가 같은지 체크하시면 더 안정적인 코드가 될 것 같아요!
src/@lib/equalities/shallowEquals.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.
deepEquals와 코드의 구성이 조금 다른 걸로 보이는데 이렇게 구성하신 이유가 있을까요!
틀렸다라는 것보단 궁금해서 여쭤봅니다!
if (propsRef.current === null || !equals(props, propsRef.current)) { | ||
propsRef.current = props; | ||
// 4. props가 변경된 경우에만 새로운 렌더링 수행 | ||
componentRef.current = createElement(Component, props); |
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.
저는 아예 컴포넌트를 반환하도록 <Component {...props} />
로 했는데 저도 이 방법을 쓸 걸 그랬나봐요!
그러면 안전하게 ts로 반환이 되네요
src/@lib/hooks/useRef.ts
Outdated
export function useRef<T>(initialValue: T): { current: T } { | ||
// React의 useState를 이용해서 만들어보세요. | ||
return { current: initialValue }; | ||
const [ref] = useState<{ current: T }>({ current: initialValue }); |
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.
const [ref] = useState<{ current: T }>({ current: initialValue }); | |
const [ref] = useState<{ current: T }>(() => ({ current: initialValue })); |
찾아보니 이렇게 선언하는걸 추천하더라구요!
이유는 렌더링이 비싼 값이 선언되면 매번 리렌더가 발생하기 때문에 제안드린 것과 같이 함수로 구현하는 것을 추천드려요!
const ref = useRef<{ | ||
deps: DependencyList; | ||
value: T; | ||
initialized: boolean; | ||
}>({ deps: [], value: undefined as T, initialized: 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.
ref를 하나로 구현하셨군요! 좋은 인사이트를 주셔서 감사합니다!
src/App.tsx
Outdated
@@ -23,11 +24,6 @@ interface Notification { | |||
|
|||
// AppContext 타입 정의 | |||
interface AppContextType { |
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.
구현하신 방법이 가장 정석적인 방법이지만 저는 Pick 타입을 Nestjs의 dto를 구현할때 사용해본 적이 있어서 추천드립니다.
아래의 방법은 interface, type의 구성을 변경할 수 없을 때 유용해요. 기억해 두셨다가 필요하실 때 한 번 사용해보세요!
// 기존
const ThemeContext = createContext<ThemeContextType | undefined>(undefined);
// 새로운 방법
const ThemeContext = createContext<Pick<AppContextType, "theme" | "toggleTheme"> | undefined>(undefined);
- context 분리 - provider 분리 - component 분리 - interface 분리
- type error 를 해결하기 위해 isRecord 추가 및 params type 변경
과제 체크포인트
기본과제
심화 과제
과제 셀프회고
기술적 성장
새로 학습한 개념
기존 지식의 재발견/심화
구현 과정에서의 기술적 도전과 해결
코드 품질
특히 만족스러운 구현
첫번째는 useRef 입니다.
해당 코드는 저에게 두가지의 유용한 지식을 알게 해주었습니다. 하나는 useState 의 활용방법이고 다른 하나는 useState 초기값의 리렌더링 문제와 해결방법을 알게 해주었다는 점에서 저에게는 뜻 깊은 코드였습니다.
두번째는 useMemo 입니다.
해당 코드 하나를 구현함으로써 만들수 있는 useCallback, useDeepMemo 이 두 기능을 모두 동시에 만들 수 있다는 것에 여러가지를 배우게 되었습니다.
리팩토링이 필요한 부분
학습 효과 분석
( useMemo, useCallback, useRef 등등 )
( useState, useMemo, useCallback, useRef 등등 )
( useState 의 활용방법, Pick 의 사용방법 )
과제 피드백
과제에서 모호하거나 애매했던 부분
코치님께서 제시해주신 가이드와 달라서 사실 이게 맞게 구현한건지 잘 모르겠습니다.
과제에서 좋았던 부분
리뷰 받고 싶은 내용
피드백을 부탁드리고 싶은 내용은
첫번째 얕은 비교와 깊은 비교를 제가 아래와 같이 구현하였습니다.
제시해주신 가이드
제가 작성한 코드
해당 코드는 노션에서 제시해주신 가이드와 조금 다르게 구현이 되었습니다.
해당 코드가 테스트는 통과하였지만 코드가 코치님의 의도하신 대로 작성되었는지가 궁금합니다.
문제가 있다면 어떤 부분에서 문제가 발생할 수 있는지 알고 싶습니다.
그리고 두번째는 App.js 의 콜백 지옥에 대한 해결방법이 있는지 알고 싶습니다.
위 코드는 제가 리팩토링을 진행하여 작성된 App.js 코드입니다.
보시다시피 Provider가 많아 짐으로써 depth 가 깊어지고 점차 가독성이 안좋아지고 있는 것 같습니다.
이러한 부분은 해결할 필요가 없는 건지 아니면 더 좋은 해결 방법이 있는 건지 궁금합니다.
세번째는 리팩토링 방법에 대해 궁금합니다.
제가 리팩토링을 진행하면서 위와 같은 폴더 구조를 만들어 분리를 진행했습니다.
[ context.ts ]
[ provider.tsx ]
[ interface/user.ts ]
제가 이렇게 작성한 이유를 말씀드리자면,
context 폴더에서 context와 provider 를 나눈 이유는 사실 파일 이름을 정할 수 없어서 입니다.
context와 provider를 함께 넣은 파일 이름을 정할 수 없어서 나누었고 저는 그게 더 가독성이 좋다고 판단했습니다.
interface 의 경우는 공통된 주제를 기준으로 파일 하나를 통해 관리하는 것이 좋겠다고 생각이 들었습니다.
context 와 반대되는 이유이지만 context의 경우 context와 provider 두가지의 파일로만 나눠지지만
interface의 경우 각 interface 를 따로 파일로 관리하게된다면 너무 많은 파일을 생성할 가능성도 있고 주제를 기준으로 하나의 파일로 관리하는 편이 더 좋을 것 같다고 판단하였습니다.
저의 판단에서 어떠한 부분이 잘못됐는지 조언을 듣고 싶습니다.
감사합니다!