Conversation
There was a problem hiding this comment.
리액트, nextjs, typescript까지 학습하면서 붙여나가야 하니 쉽지 않은 플로우일텐데도 고생 많으셨습니다! 전반적으로 자바스크립트 로직 흐름이나 함수 형태등은 생각보다 너무 깔끔해서 놀랐어요!
다만 문원님 코드 베이스에 한가지 수정하면 좋을 습관이 있는데요.
콤포넌트가 너무 무겁습니다.
특정 페이지에서 동작하는 요소들을 그때그때 구현해나가다 보니 페이지 콤포넌트 하나에서 정말 많은 요소들을 처리하고 있어요. 따라서 콤포넌트 가독성 뿐만 아니라, 로직 관리하기도 쉽지 않은 형태로 보입니다.
이를 해결하기 위해 우리 멘토링때 이야기했던것처럼 조금 더 역할과 책임을 콤포넌트단위로 나누어 보고, 서비스 레이어와 컨트롤러 레이어까지 구분을 지어보는 시간을 한번 억지로 내서라도 살펴보면 어떨까 싶어요.
이렇게 하면 콤포넌트 뿐만 아니라 재활용되는 훅들도 조금 더 목적에 맞게끔 구분을 지어서 재활용성이 있도록 개발할 수 있을 것 같습니다.
아 그리고, 폴더의 경우 js 프로젝트 컨벤션에 따르면 react jsx인 경우 PascalCode 형태로 작성하고, 그 외에는 다 소문자와 - 의 조합으로 만들어 질 텐데요. 지금은 폴더 이름들도 pascal case인 경우가 많아 조금 난잡한 느낌이 들기는 합니다. 이 부분 한번 탐색해보시는걸 추천드려요!
고생 너무 많으셨습니다!
There was a problem hiding this comment.
package.json이 날라간것 같은데 괜찮나요?
| {modalIsOpen && ( | ||
| <LinkDeleteModal item={item} onClose={handleClosedModal} /> | ||
| )} | ||
| {addModal && <TestModal onClose={handleClosedModal} />} |
There was a problem hiding this comment.
모달이 열리고 닫히는 상태 제어를 모달 내부에서 해보면 어떨까요?
이런 형태로 진행하게 되면 이후에 모달이 추가되면 추가될 수록 상위 콤포넌트에서 어떤 모달이 펼쳐져있는지를 기록하기 위해
더 많은 로직들이 추가가 되어야 할 ㄹ것 같습니다
|
|
||
| export const AddFolderModal = ({ onClose }) => { | ||
| return ( | ||
| <ModalLayout onClose={onClose}> |
There was a problem hiding this comment.
공통 활용 가능한 모달을 구현한다면 다음과 같이 콤포지션 패턴을 활용해보면 어떨까 생각이 들어요
https://reactjs-kr.firebaseapp.com/docs/composition-vs-inheritance.html
https://dev.to/ricardolmsilva/composition-pattern-in-react-28mj
실제 use case는 다음 콤포넌트를 한번 참고해보면 좋을 것 같습니다!
https://www.radix-ui.com/themes/docs/components/dialog
|
|
||
| const cx = classNames.bind(styles); | ||
|
|
||
| export const AddFolderModal = ({ onClose }) => { |
There was a problem hiding this comment.
add-folder-modal의 경우 폴더를 추가하는 로직을 prop으로 전달받아야 할 것 같은데, 아직 구현이 안된게 맞는거죠?
또한 이 녀석이 받아올 prop으로는 폴더를 추가하는 로직만 받아오는게 관심사 분리 차원에서 더 좋을 것 같아요
이렇게 구현하기 위해 아래 설명할 컴포지션 패턴을 한번 살펴보시는걸 추천드립니다!
There was a problem hiding this comment.
프로젝트에서 활용되는 모달들을 다 components/modal 폴더에 관리하고 계시는데요
만약 같은 작업을 하는 모달이 화면의 다른 부분에서 다른 모양으로 보여져야 한다면 어떻게 해야 할까요?
위� 관점에서 공통 modal 콤포넌트를 확장시켜 구현한 모달 컴포넌트라 해도 실제 앱 상에서 위치되는 공간과 역할이 조금씩 다르게 됩니다.
따라서 이런 요소들을 공통으로 관리하기보단 콤포넌트들이 활용되는 곳을 표현하는 용도로 폴더 관리 해주는게 더 좋지 않을까 생각이 되어요
There was a problem hiding this comment.
반복적으로 사용되는 로직을 자동화 하기 위해 활용하는게 커스텀 훅이잖아요
useFetchFolder는 어떤게 반복되기 때문에 훅으로 된 건지를 한번 생각해보면 좋겠어요.
fetch(..folders)로 네트워크 요청을 보내서 원하는 형태의 에러핸들링을 하는걸 자동화 하고자 한것일까
네트워크 통신동안 발생하는 로딩 상태와 데이터를 state로 관리하는걸 자동화 하고자 한것일까
왜냐하면, 지금 형태로 가다가보면, 존재하는 모든 http request에 대해 다 훅을 만들어야 하는 모양새입니다.
폴더를 추가하는 로직을 작성한다고 하면 useCreateFolder 훅을 만들어야 할테고,
id에 해당하는 폴더를 반환하거나 삭제, 수정하는 로직을 작성한다고 하면 그에 따른 훅들을 다 만들어야 하겠죠.
이는 유지보수성 관점으로 좋아보이지가 않아요.
따라서 어떤 목적으로 훅을 만들건지 한번 고민해보면 좋을 것 같습니다
There was a problem hiding this comment.
이 경우에도 하드코딩된 users/1에 대해서만 할 수 있는게 아니라
전달받은 사용자 값에 대해 폴더를 조회할 수 있도록 확장해주는게 좋겠어요
There was a problem hiding this comment.
콤포넌트가 처리하고 있는 일이 너무 많습니다!
이런 형태를 개인적으로는 모든걸 다 처리하는 무적의 콤포넌트, 또는 silver bullet이라고 하는데요.
모든 요소들을 다 담는 콤포넌트 안에서도, 각 콤포넌트가 지닌 역할과 책임이 있을거에요.
그에 따라서 더 manageable하게 콤포넌트화 하는건 어떨까요?
There was a problem hiding this comment.
이 콤포넌트 활용 방식을 참고해 모달의 제어권을 역으로 제어할 수 있는 형태로 제공해주는 방식을 한번 고려해보면 어떨까요?https://www.radix-ui.com/themes/docs/components/dialog
| if (window.Kakao) { | ||
| const kakao = window.Kakao; | ||
| if (!kakao.isInitialized()) { | ||
| kakao.init(process.env.REACT_APP_KAKAO_SDK_KEY); | ||
| } |
There was a problem hiding this comment.
shareKakao 함수가 호출될때마다 init체크를 하기보다,
이녀석이야 말로 useEffect를 통해 최초 레이아웃 렌더가 된 후 kakao 인스턴스를 실행해 윈도우 객체에 등록시켜주는 형태로 구현하고
share 함수를 제공해주어 Link.sendDefault 함수가 내부적으로 호출되게 말아주는게 더 좋지 않을까 생각이 되어요
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게