[2단계 - 지하철 노선도 미션] 하루(김하루) 미션 제출합니다. #38
Conversation
| return ( | ||
| <> | ||
| <ErrorBoundary> | ||
| <Header> | ||
| <NavBar serverOwner={SERVER_LIST[serverId]?.nickname} /> | ||
| </Header> | ||
|
|
||
| <Main {...rest}>{children}</Main> | ||
|
|
||
| <ServerSelectButton onClick={handleServerSelectButtonClick}>서버선택</ServerSelectButton> | ||
| {isServerSelectOpen && <ServerSelect serverId={serverId} onSubmit={handleServerSubmit} />} | ||
| </> | ||
| </ErrorBoundary> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
질문드립니다! 🙋🏻♀️ - 에러 바운더리
리액트의 에러 바운더리를 학습차원에서 작성해보았습니다...!
에러 바운더리를 학습하면서 특정 에러 상황에서 깔끔하게 UI를 갈아낄 수 있다라는 점이 장점이라고 생각했습니다...!
그런데 비동기 로직의 error를 catch하지 못해서, 생각보다 적용할만한 곳이 없다(?)고 느껴졌습니다. 이번 미션에서는 단순하게 App 전체를 감싸듯이 사용해보았는데, 조금 더 잘 쓰고 싶은 마음에 질문 드려봅니다...!
에러 바운더리를 활용해서 갯수가 0개이다, 텅비었다는 표시의 UI를 그려주는 용도로 사용하고 싶었는데, 이 또한 GET 요청이 필요한 비동기 로직이기 때문에 에러바운더리를 적용할 수 없어 보였습니다...🥲 혹시 이런 경우에 에러 바운더리를 적용할 수 있는 방법이 있을지..(👉👈), 혹은 ErrorBoundary를 어떻게 활용하면 좋을지 간단하게 힌트 주시면 정말 감사하겠습니다..!!! 🙇🏻♀️🙇🏻♀️🙇🏻♀️💙
There was a problem hiding this comment.
추천해주신 글의 AsyncBoundary와 같이 Suspense를 ErrorBoundary를 묶어서 비동기 컴포넌트를 처리할 수 있군요...! 🤩🤩🤩
리액트 홈페이지에 Suspense가 experimental 이라고 나와있기 하지만, 많이 쓰시는 것 같군용! ㅎㅎ
추천해주신 방법 참고해서 다음 프로젝트에 Suspense도 도입해보고, Suspense + ErrorBoundary조합도 함께 도입해보겠습니다! 좋은 글 추천해 주셔서 감사합니다 😆
| {isLogin ? ( | ||
| <MenuList isVisible={isLogin}> | ||
| <MenuItem> | ||
| <button onClick={handleLogoutRequest}> | ||
| <IconPerson /> | ||
| 로그아웃 | ||
| </button> | ||
| ) : ( | ||
| <NavLink activeClassName="selected" to={ROUTE.LOGIN}> | ||
| <IconPerson /> | ||
| 로그인 | ||
| </NavLink> | ||
| )} | ||
| </MenuItem> | ||
| </MenuList> | ||
| </MenuItem> | ||
| </MenuList> | ||
| ) : ( | ||
| <MenuGroup | ||
| isVisible={!isLogin} | ||
| menuList={[{ content: '로그인', icon: <IconPerson />, route: ROUTE.LOGIN }]} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
이미 isLogin으로 조건부 렌더링을 하고있는데 isVisible이라는 props는 왜필요한가요?
There was a problem hiding this comment.
그것은,,,, 제가 추상화를 잘못 했기 때문입니다 😭 😭 말씀해주신 것처럼 조건부 렌더링을 하고 있기 때문에 isVisible 은 필요하지 않습니다.
전반적으로 Navbar 컴포넌트 내에 메뉴 추상화를 개선하고, 불필요한 isVisible props를 제거하였습니다...! 꼼꼼한 피드백 감사드립니다 💙
📌 관련커밋: b6aa604
src/hooks/useSignUp.js
Outdated
| const requestSignUp = async (email, age, password) => { | ||
| await requestPost({ | ||
| url: `${endpoint}/members`, | ||
| body: { | ||
| email: email.value, | ||
| age: age.value, | ||
| password: password.value, | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
requestSignup이라는 함수를 작성한사람이아닌 사용하는 입장에서 어떻게 사용할지 상상해볼까요?
이 함수의 내부구현을 봐야만 email, age, password를 순서대로 넘겨야하는구나! 를 알수있습니다.
const requestSignUp = async (data) => {
await requestPost({
url: `${endpoint}/members`,
body: data,
});
};처럼 작성하고 외부에서 엔드포인트로 넘겨줘야하는 body 형식에 맞게 인자로 넘겨주는건 어떨까요?
There was a problem hiding this comment.
requestSignUp 함수 입장에서는 필요한 값이 email 에 값이 담겨있는지 email.value 에 값이 담겨있는지는 알 필요가 없다고 생각되어, 리뷰어님께서 예시로 들어주신 코드처럼 필요한 data를 아예 묶어서 전달받는 것으로 변경해보았습니다.
아직 저에게 코드를 작성하면서, 코드 작성자의 입장이 아닌 사용자의 입장에서 생각하는 상상력/능력이 부족한 것 같습니다...! 작성할 때 생각하지 못했던 부분들이 리뷰어님의 피드백을 받고나서야 새롭게 보이네요...! 앞으로 이런 부분에 더더욱 신경쓰면서 좋은 코드를 작성하도록 노력해보겠습니다! 좋은 피드백 감사드립니다 :)
📌 관련커밋: 235b50c
src/pages/Map/index.js
Outdated
| /* eslint-disable react-hooks/exhaustive-deps */ | ||
| useEffect(() => { | ||
| requestGetMap(); | ||
| }, []); |
There was a problem hiding this comment.
처음 코드를 작성할 당시에는 컴포넌트가 렌더링 될 때 발생하는 모든 사이드이펙트(useEffect)를 컴포넌트 코드에서 볼 수 있어야 한다고 생각했고, 그래서 이 hook도 컴포넌트에서 남겨두었었습니다.
리뷰어님의 피드백을 듣고나서야, 이렇게 단순히 데이터를 현행화시켜주는 hook은 중요한 로직이 아니어서, 커스텀 훅 안으로 숨기는 것이 더 좋겠다는 생각을 할 수 있었습니다...!
요 부분을 포함해서 단순히 data fetching 해주는 hook 들을 커스텀 훅 안으로 쏙 넣어보았습니다 :)
📌 관련커밋: 63f53ca
src/utils/format.js
Outdated
| export const getFormattedNumber = (number) => { | ||
| return number?.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ','); | ||
| }; |
There was a problem hiding this comment.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat
이런 Browser API도 있으니 살펴보세요! 👍
There was a problem hiding this comment.
언급해주신 API로 복잡한 정규식 없이 훨씬 깔끔하게 코드를 작성할 수 있었습니다...! 👍
속도, 용량 등 웬만한 숫자 단위와 세계 화폐 단위도 다 지원되서 여러 경우에 유용하게 사용할 수 있을 것 같습니다.
추천 감사드립니다! 💙
export const getFormattedNumber = (number) => new Intl.NumberFormat().format(number);📌 관련커밋: e1af5c1
학습로그HTTP태그
|
📌 리뷰반영 요약표
크리스님 안녕하세요...! 이제 정말정말 미션에서 마지막으로 남기는 코멘트가 될 것 같네용,,, 🥲 시원한 여름 되시길 바라요 🍉
|
크리스님, 안녕하세요!
2단계로 인사드리는 🙋🏻♀️ 하루(@365kim) 입니다.
크고 작은 개선사항 코멘트로 전해주시면 열심히 개선해보겠습니다! 🙏
잘 부탁드리겠습니다! 🙇🏻♀️🙇🏻♀️🙇🏻♀️
🛒 2단계 데모 페이지
💪 새롭게 시도한 부분
ErrorBoundary컴포넌트를 작성해보았습니다.커스텀훅을 추가로 작성해보았습니다.✅ 구현 기능 목록
회원 기능
전체보기 기능
경로탐색 기능