-
Notifications
You must be signed in to change notification settings - Fork 165
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
[페이먼츠 미션 Step 2] 가브리엘(윤주현) 미션 제출합니다. #254
Conversation
Co-authored-by: noah <nlom0218@users.noreply.github.com>
Co-authored-by: noah <nlom0218@users.noreply.github.com>
Co-authored-by: noah <nlom0218@users.noreply.github.com>
Co-authored-by: noah <nlom0218@users.noreply.github.com>
Co-authored-by: noah <nlom0218@users.noreply.github.com>
Co-authored-by: noah <nlom0218@users.noreply.github.com>
Co-authored-by: noah <nlom0218@users.noreply.github.com>
안녕하세요 헤인티! 일단 리뷰가 늦는다고 생각한 적이 한번도 없었습니다!! Prettier 및 코드 일관성일단 말씀해주신대로 프리티어가 적용되지 않던 문제는 해결되었습니다. 코드에서 일관성을 보이지 못하는 표현법들은 대부분 통일했습니다. useSyncExternalStore 관련말씀해주신대로 지금은 삭제된 store 코드(creditCardListStore.tsx)는 말씀해주신 것 처럼 실제로 vanilla js가 맞습니다. 실제로도 useSyncExternalStore는 non-React 환경(ex. 서드파티 라이브러리, SSR 등)에서 발생할 수 있는 티어링을 방지하기 위해 나온 기술이고, 그에 따라 외부에서 주입하는 store가 vanilla js 코드로 설계 되어야만 합니다. 말씀해주신 것 처럼 옵저버 패턴으로 구현하도록 되어있는 것도 사실상 의무화 됩니다. 따라서 제가 목적에 전혀 맞지 않게 구현한 것이 맞습니다 🥲 다만 널리 알려진 전역상태관리라이브러리 중 리덕스가 전역으로 상태가 관리될 수 있는 이유도 내부적으로는 옵저버 패턴으로 동작하고 있다고 알려져있고, 이를 손쉽게 모방할 수 있는 방법이 없을지 고민하다가 useSyncExternalStore를 손 대게 되었습니다. 아무리 이 기능이 전역상태관리 기능을 포함하고 있더라도, 공식 문서에서는 non-React 환경이 아닌 경우에는 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 를 다시 한 번 볼 수 있었습니다. 실무에서는 이미 개발된 프로젝트들을 운영해나가기 때문에 최신 기능을 많이 쓰지 못하는 경우가 많은데 덕분에 리프레시 했어요 ㅎㅎ
3단계에서도 많은 시도해보는 가브리엘님이 되셨으면 좋겠네요! 고생 많으셨습니다 🙇🏼♀️
안녕하세요 헤인티!
이번 step2 미션에서는 카드사 선택, 카드 별칭 설정 등의 신규 기능 구현이 포함되었습니다.
사실 리팩터링을 좀 더 진행하고 싶었으나, 하루 빨리 리뷰를 요청드리는 것이 좋을 것 같아 일단 보냅니다 🥲🥲
배포 페이지
주요 변경 사항
궁금한 점
useSyncExternalStore를 전역 상태 관리에 활용해도 괜찮을까요?
처음에는 미션을 구현하다가 페이지 간에 상태를 공유해야한다는 점 때문에 Context API를 활용하여 상태를 공유하려고 했습니다. 하지만 제가 생각하기에 Context는 항상 Provider가 최상단(혹은 그에 준하는 위치)에 있어야 한다는 점 때문에 너무 잦은 렌더링이 광범위하게 일어난다고 생각하여 피하고 싶었습니다.
특히 상태가 상시 바뀌는 위치에는 사용을 하고싶지 않았습니다. 하지만 일단 기능 구현을 위해 카드 등록 폼에 활용하게 됐는데요, 미션을 마무리 하다가 useSyncExternalStore의 가능성을 알게 되어
카드 목록
에 활용해보니 생각보다 괜찮은 기능이었습니다!제가 알기로는 애초에 리액트에서는 전역 상태 관리에 대한 상세한 가이드를 주지 않아서 다양한 전역 상태 관리 라이브러리들이 리액트 생태계에 등장하였고, 그 영향력 또한 엄청나다고 생각하는데요, 이런 여러 가지 이유로 인해 (라이브러리 없는) 리액트에서는 전역 상태 관리에 대한 기능이 있기는 해도 어떤 스탠다드한 패턴이 없다고 생각했습니다.
하지만 공식 문서에서 읽은 정보에 따르면 useSyncExternalStore가 리액트 바깥에 상태를 관리할 수 있는 서드파티 라이브러리의 기능을 포함한다고 되어있다는 점에서 영감을 받아 이런 저런 테스트를 해본 결과, 전역 상태를 관리할 수 있는 기능을 지원함은 물론이고, 이 기능에 필요한 store를 구독하는 상태(?)가 업데이트 되더라도 다른 컴포넌트를 재 렌더링 하지 않고 부분 업데이트 되는 것을 확인하였습니다.
하지만 이 기능은 리액트에서 관찰(?)하지 못하는 바깥의 영역과의 소통을 위해 출시된 것으로 알고 있습니다.
제가 목적에서 벗어난 사용을 하고 있는 것인지 궁금하고, 아예 폼도 context를 걷어내고 이 기능으로 대체하는 것이 어떤지 궁금합니다.
아쉬운 점
비제어 컴포넌트를 언제 쓸 수 있을지 고민입니다.
거의 모든 인풋이 상태 없이 동작하기 어렵다고 생각하여 비제어 컴포넌트를 사용하지 못했습니다 ㅠㅠ
카드 별칭에서 사용하기에는 ref를 전달해줘야 한다는 점 때문에 막혀서 포기하였습니다.
미완성같은 디자인
다른 크루들을 보면 모달에 애니메이션이 달려있거나, 카드가 뒤집히는 등 여러 가지 시도를 하였으나, CSS를 적용할 시간이 부족하여 일단 제출하였습니다. 이 부분은 로직에 영향이 없도록 시간이 나는 대로 틈틈이 작성해보겠습니다. (스텝 2가 끝날 때 쯤 디자인 개선이 추가적으로 있을 것 같습니다.)
덜 된 리팩터링
아직도 �리팩터링이 덜 된 부분이 눈에 보입니다! 기능 동작에 이상이 없어서 일단 제출한 느낌이 없지 않아 있어 찝찝한 것 같아요 ㅠㅠ
좀 더 다듬고 싶었으나 시간 관계 상 일단 먼저 요청드리고, 피드백이 도착하는대로 빠르게 수정&반영 해보겠습니다!