-
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
[11팀 박근백] [Chapter 1-3] React, Beyond the Basics #6
base: main
Are you sure you want to change the base?
Conversation
src/app/App.tsx
Outdated
<ThemeConsumer> | ||
{(context) => ( | ||
<div | ||
className={`min-h-screen ${context?.theme === "light" ? "bg-gray-100" : "bg-gray-900 text-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.
Provider로 감싸준 컴포넌트 내부에서 바로 사용하려면 Consumer를 쓰면되는군요.
상위에 useContext 훅을 사용시 아직 생성되지 않은 context 조회를 하게 되어 자식컴포넌트를 하나 더 만들어 사용했는데, consumer의 사용법을 이 코드를 보고 학습해서 처음 알게 되었네요.
전 구시대 잔재 인줄로만 알았어요..ㅋㅋㅋㅋ
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 valueA = Reflect.get(objA, key); | ||
const valueB = Reflect.get(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.
Reflect라는게 있는걸 이번에 처음알았네요..! 정말 몰라서 여줍는건데, 혹시 이렇게 하는 것과, objA[Key]와는 어떤 차이가 있는지 여쭤봐도 괜찮을까요...?
정말 몰라서 신기해서 여쭙습니다..
return useSyncExternalStore(store.subscribe, () => { | ||
const next = selector(store.getState()); | ||
if (prevRef.current === null || !shallowEquals(prevRef.current!, next)) { | ||
prevRef.current = next; | ||
} | ||
return prevRef.current; | ||
}); | ||
}; |
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.
리뷰하러 왔다가 배우고만 가는 것 같네요.. useSyncExternalStore를 이렇게 활용하는구나를 배우고 갑니다..! 👍
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.
한가지 궁금한게 있어서 여쭙습니다. useStore에서 전역 공간 확보를 위해서 useSyncExternalStore를 쓰신 것 같은데 혹시 제가 이해한게 맞을까요...?
또한, useContext 같은 방법으로도 전역 상태를 관리할 수 있다고 생각하는데.. 혹시 이 훅을 사용하신 이유 있으신지 궁금합니다..
생소한 훅이라서 정말 궁금하여 여쭙습니다...!
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.
오... useSyncExternalStore은 뭔가요??
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.
저도 useSyncExternalStore가 뭔지 궁금하네요,, 주로 전역으로 관리하기 위해 사용되는 건가요??? 어떨때 주로 사용되나용?
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.
오... useSyncExternalStore은 뭔가요??
저도 useSyncExternalStore가 뭔지 궁금하네요,, 주로 전역으로 관리하기 위해 사용되는 건가요??? 어떨때 주로 사용되나용?
useSyncExternalStore는 외부 데이터 저장소와 React 컴포넌트를 동기화하는데 사용하는 훅입니다.
const state = useSyncExternalStore(
subscribe, // 구독 함수
getSnapshot, // 현재 상태를 반환하는 함수
getServerSnapshot // (선택적) 서버 렌더링용 초기 상태
);
또한, useContext 같은 방법으로도 전역 상태를 관리할 수 있다고 생각하는데.. 혹시 이 훅을 사용하신 이유 있으신지 궁금합니다..
context api 와 함께 이용한 이유는 context api 를 사용해 지역적인 상태들을 관리하곤 하는데 이와 전역상태와 구분하기 위해 사용합니다. ( 해당 context 내부에서만 해당 상태를 사용할 수 있도록 )
저는 zustand를 자주 사용하고 있어서 진짜 전역적인 변경이 있는 경우 zustand 를 그 외에는 context api + zustand를 사용합니다.
위와 같이 함께 사용하는 이유는 context api 만을 사용하면 따로 최적화 해주는 과정이 필요한데 ref를 통해 store를 바라보고 selector 를 통해 필요한 상태만 뽑아쓰는 구조를 사용하면 별다른 최적화 없이 같은 스토어라도 해당 상태를 사용하는 컴포넌트만 업데이트 할 수 있기 때문입니다 !
const { theme } = useThemeState(); | ||
const { toggleTheme } = useThemeAction(); | ||
const { user } = useUserState(); | ||
const { login, logout } = useUserAction(); |
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.
state와 action을 분리하는게 인상적이네요. 이 역시도 배워갑니다..!
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.
한가지 궁금한 점이, state와 액션을 모아서 하나의 코드로 전달해도 괜찮아보이는데, 혹시 state와 action을 분리하신 이유가 있으실까요?
const { theme, toggleTheme } = useTheme(); 이런느낌으로 쓸 수 있을것 같은데 어떤 장점이 있는지 여쭤봐도 괜찮을까요...?
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.
큰 이유는 없었는데 재도님의 말을 듣고보니 그런식으로 구현하는 것이 더 사용하는데에 편리할것 같습니다 !
let memoizedComponent: ReactNode | null = null; | ||
let memoizedProps: P | null = null; | ||
return (props: P) => { | ||
if (!_equals(memoizedProps, props)) { | ||
memoizedProps = props; | ||
memoizedComponent = React.createElement(Component, props); | ||
} | ||
return memoizedComponent; | ||
}; |
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.
크.. 👍 useRef
사용 안 하고 클로저를 사용해서 푸셨다니!
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.
그런데 멘토링때 들은 바로는 위와같이 클로저를 이용하여 구현한 경우에는 메모리 누수가 발생할 수 있고 그것을 개발자가 따로 신경써 처리해야하는데 useRef를 이용해 구현하면 메모리 관리를 react 측으로 넘길 수 있다는 장점이 있다고 하셨습니다!
return useSyncExternalStore(store.subscribe, () => { | ||
const next = selector(store.getState()); | ||
if (prevRef.current === null || !shallowEquals(prevRef.current!, next)) { | ||
prevRef.current = next; | ||
} | ||
return prevRef.current; | ||
}); | ||
}; |
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.
오... useSyncExternalStore은 뭔가요??
import { useState } from "react"; | ||
import { ItemList } from "./ItemList"; | ||
import { ComplexForm } from "./ComplexForm"; | ||
import { generateItems } from "@/utils"; | ||
import { useCallback } from "@lib/hooks"; | ||
|
||
export const MainSection = () => { | ||
const [items, setItems] = useState(() => generateItems(1000)); | ||
|
||
const addItems = useCallback(() => { | ||
setItems((prevItems) => [ | ||
...prevItems, | ||
...generateItems(1000, prevItems.length), | ||
]); | ||
}, []); | ||
|
||
return ( | ||
<div className="container mx-auto px-4 py-8"> | ||
<div className="flex flex-col md:flex-row"> | ||
<div className="w-full md:w-1/2 md:pr-4"> | ||
<ItemList items={items} onAddItemsClick={addItems} /> | ||
</div> | ||
<div className="w-full md:w-1/2 md:pl-4"> | ||
<ComplexForm /> | ||
</div> | ||
</div> | ||
</div> | ||
); | ||
}; |
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.
오 MainSection
을 별도의 컴포넌트로 분리하고 items
와 addItems
를 ItemList
에서 작성하지 않고 props
로 전달해주신 이유가 궁금해요!
import { createStore, Store } from "@/storeUtils"; | ||
import { Notification } from "@/types"; | ||
import { createContext } from "react"; | ||
|
||
interface NotificationState { | ||
notifications: Notification[]; | ||
} | ||
|
||
interface NotificationAction { | ||
addNotification: (message: string, type: Notification["type"]) => void; | ||
removeNotification: (id: number) => void; | ||
} | ||
|
||
export type NotificationType = NotificationState & NotificationAction; | ||
export type NotificationStore = Store<NotificationType>; | ||
|
||
export const createNotificationStore: () => NotificationStore = () => | ||
createStore<NotificationType>((set) => ({ | ||
notifications: [], | ||
addNotification: (message, type) => { | ||
const newNotification: Notification = { | ||
id: Date.now(), | ||
message, | ||
type, | ||
}; | ||
|
||
set((prev) => ({ | ||
...prev, | ||
notifications: [...prev.notifications, newNotification], | ||
})); | ||
}, | ||
removeNotification: (id) => { | ||
set((prev) => ({ | ||
...prev, | ||
notifications: prev.notifications.filter( | ||
(notification) => notification.id !== id, | ||
), | ||
})); | ||
}, | ||
})); | ||
|
||
export const NotificationContext = createContext<NotificationStore | undefined>( | ||
undefined, | ||
); |
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.
State
와 Action
을 분리했다가 다시 하나의 Store
로 합치신 이유가 궁금해요!
분리를 한 건 각각의 Provider
를 만들기 위해서 라고 생각했는데 하나의 Store
로 합쳐서 사용한 쪽에서는 하나의 Provider
의 value
로 한 번에 넘겨주는 것 같아서 질문드립니다.
테스트 코드 통과 때문에 분리하려다가 합치신 건가요??
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.
state와 action을 분리한 코드는 context api 만으로 최적화를 하기 위해 분리했습니다 ( app-enhanced )
그리고 현재 외부 스토어를 이용한 최적화 부분은 selector를 사용해서 해당 상태를 직접 사용하고 있는 컴포넌트만 리렌더링이 되도록 구현하였기 때문에 따로 분리하지 않아도 최적화가 가능합니다!
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.
안녕하세요 근백님 :) 담당 학습메이트 오소현입니다 ~!
항상 근백님은 토요일부터 마지막날까지 과제에 손을 놓지 않으시고 계속해서 고민하시면서 고도화하시는 모습이 너무 멋있습니다,,bb 항상 매주차 과제를 어떻게 하실지 정말 기대하면서 리뷰하는 것 같습니다 이번주 과제도 너무 고생많으셨습니다 :)
그리고 PR 초안 단계에서 open으로 열어주시고, 리뷰 받고 싶은 내용도 채워주시면 좋을것 같아용:)
전역 상태 관리가 가장 인상깊었습니다 많이 배우고 갑니다 근백님 고생많으셨습니다 ㅎㅎㅎ
const valueA = Reflect.get(objA, key); | ||
const valueB = Reflect.get(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.
저는 Reflect가 Proxy 객체를 함께 사용할 때 객체의 특정 키에 접근할 때 의도하는 로직을 실행할때 사용하는 것으로만 알다가 실제 사용은 근백님 덕분에 알아갑니다 bb 다양하게 시도해서 코드를 간결하게 구현하신게 너무 좋습니다 bb
return useSyncExternalStore(store.subscribe, () => { | ||
const next = selector(store.getState()); | ||
if (prevRef.current === null || !shallowEquals(prevRef.current!, next)) { | ||
prevRef.current = next; | ||
} | ||
return prevRef.current; | ||
}); | ||
}; |
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.
저도 useSyncExternalStore가 뭔지 궁금하네요,, 주로 전역으로 관리하기 위해 사용되는 건가요??? 어떨때 주로 사용되나용?
describe("최적화된 App 컴포넌트 테스트", () => { | ||
beforeEach(() => { | ||
renderLogMock.mockClear(); | ||
generateItemsSpy.mockClear(); | ||
}); |
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.
진짜 bb 최적화도 따로 하시고 그에 맞는 테스트 코드도 맞춰서 리팩토링하시다니 최고입니다 bb
onAddItemsClick: () => void; | ||
} | ||
|
||
export const ItemList = memo(({ items, onAddItemsClick }: ItemListProps) => { |
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.
컴포넌트에 memo 처리 bb 좋습니다
NotificationStateContext, | ||
} from "./NotificationContext"; | ||
|
||
export const NotificationProvider = ({ children }: PropsWithChildren) => { |
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와 Provider로 둘을 역할대로 둘을 정확하게 분리하셨군요,, bb
const removeNotification = useCallback((id: number) => { | ||
setNotifications((prev) => | ||
prev.filter((notification) => notification.id !== id), | ||
); | ||
}, []); | ||
|
||
const notificationState = useMemo<NotificationState>( | ||
() => ({ | ||
notifications, | ||
}), | ||
[notifications], | ||
); | ||
|
||
const notificationAction = useMemo<NotificationAction>( | ||
() => ({ | ||
addNotification, | ||
removeNotification, | ||
}), | ||
[addNotification, removeNotification], | ||
); | ||
|
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.
이렇게 로직 내부에 각 역할에 맞게 함수 분리해서 작성해주신점 너무 좋습니다 bb
@@ -0,0 +1,40 @@ | |||
import { shallowEquals } from "./@lib"; |
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.
여기서 shallowEquals를 사용하신 이유가 있나요? deepEquals 함수로 사용하면 어떻게 되는지 궁금합니다
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를 사용하면 상태의 depth 가 깊어진다거나 하는 상황에 과도한 비교연산이 발생할것으로 생각하여 shallowEquals를 사용하였습니다.
const subscribe = (listener: () => void) => { | ||
listeners.add(listener); | ||
return () => { | ||
listeners.delete(listener); | ||
}; | ||
}; |
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.
소현님의 의도를 제대로 이해한게 맞는지는 모르곘지만 해당 코드는 useStore에 작성되어 있습니다!
useSyncExternalStore(store.subscribe, () => {
const next = selector(store.getState());
if (prevRef.current === null || !shallowEquals(prevRef.current!, next)) {
prevRef.current = next;
}
return prevRef.current;
});
과제 체크포인트
기본과제
심화 과제
과제 셀프회고
기술적 성장
코드 품질
학습 효과 분석
과제 피드백
리뷰 받고 싶은 내용
저는 현업에서도 app-plus 에 구현된 방법처럼 zustand 와 context api 를 활용해서 지역적인? context를 구현하는데요. 처음에 사용하게 된 계기는 메모이제이션 훅들을 크게 신경안써도 되는게 가장 큰 이유였는데 지금 생각해보면 뭔가 전역적인 상태를 관리하는 라이브러리 와 함께 사용되어 전역상태인가? 하는 혼동을 일으킬 것 같은 생각도 드는데요. 혹시 어떻게 생각하시나요?