-
Notifications
You must be signed in to change notification settings - Fork 0
[서인] State-Management #5
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
base: main
Are you sure you want to change the base?
Conversation
사용 경험은 있었어요. 저도 Redux를 써본 적이 있었고 (Redux, RTK 둘다 사용해봄), Zustand도 중앙 집중식 상태관리라고 생각해요. 결국 상태를 한 곳에서 관리한다는 맥락은 같다고 생각해요. 리덕스는 강제하는 패턴이 있다보니 보일러 플레이트가 느는게 단점이라고 생각하긴 하지만, 결국 하지만 요즘은 더 간결한 것을 선호하는 시대인것 같아요. Zustand 처럼요! 배우기도 쉽고, 코드도 짧은게 개발자들한테 사랑 받는 이유지 않을까 싶네요 |
D5ng
left a comment
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.
저는 개인적으로 이번 PR이 조금 아쉬웠어요. 왜냐하면 서인님은 Redux를 최근에 직접 써보셔서 더 Redux 스럽게 만들 줄 알았어요.
제 주관적으로는 해당 아티클이 정답은 아니라고 생각해요. Redux의 큰 틀을 설명해주지만 여전히 많은 문제가 있다고 생각해요.
- 객체만 상태를 등록할 수 있다. (제가 찾아본 상태관리에선 이런건 없었어요.)
- store가 observer를 제공하지 않는게 좋지 않다고 생각해요. (store랑 observer랑 따로 분리된 느낌이 강해요, 특히 import할 때 따로 불러오는게 이상합니다. DX적으로 좋지않은것 같아요.)
또 하나 아쉬운건, 스토어는 결국 어떤 상태도 관리할 수 있도록 해줘야하는데 지금은 객체의 키 값이 a, b 만 가능한게 아쉽습니다.
현재 아티클에서 많은 의심을 가지면 더 좋을것 같아요
| [key: string]: number; | ||
| } | ||
|
|
||
| export default class Component< |
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.
IState와 TComponentData가 어색해보입니다. 하나로 통일 해야하지 않을까? 라는 생각이 들어요.
요즘은 인터페이스 앞에 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.
오 찾아보니 헝가리안 표기법을 지양하는 개발자들이 많군요 참고할게요
| (state?: IReducerState, action?: TAction): IReducerState; | ||
| } | ||
|
|
||
| export const createStore = (reducer: IReducer) => { |
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.
| @@ -0,0 +1,28 @@ | |||
| import { createStore } from './core/ReduxStore'; | |||
|
|
|||
| export type TAction = { | |||
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.
type은 옵셔널이 되면 안돼요. 왜냐하면 Action을 사용할 때 type은 필수여야만 합니다! 리덕스랑 타입스크립트를 사용해보시면 type이 옵셔널이 아니라는것을 알 수 있어요.
| fn(); | ||
| currentObserver = null; | ||
| }; | ||
| export const observable = <T extends Record<string, any>>(obj: T): T => { |
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.
옵저버 패턴에는 subscribe, unsubscribe, notify가 보통은 필수적으로 들어가는데, 현재 코드에서는 unsubscribe가 없네요. 이런 부분도 고민해보면 좋을것 같아요.
| export const SET_A = 'SET_A'; | ||
| export const SET_B = 'SET_B'; | ||
|
|
||
| export const store = createStore((state = initState, action: TAction = {}) => { |
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.
TAction = {} 부분에 기본 값으로 빈 객체가 들어가면 안될것 같아요. 결국 액션은 dispatch로 스토어의 값을 변경하는 목적이지만, 작성 규칙에서 엄격하다고 볼 수 없을것 같아요.
| <!doctype html> | ||
| <html lang="en"> | ||
| <!DOCTYPE html> | ||
| <html lang="ko"> |
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.
좋아요 👍

공부한 것
고민점