MISSION3 / 김동완 #20
MISSION3 / 김동완 #20
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
어... 오늘도 취준의 길을 걷고 있는 임채승입니다. 작성하신 코드를 보고 맨 처음 같이 개발을 하던 그 김동완이 맞나? 라는 생각을 해서 기분이 저도 좋아진거 같습니다. 저도 보고 많이 배우고 갑니다.
저도 최근 form에 대해서 고민을 좀 많이 하게 된 일이 있었는데 제 생각을 맨 밑에 추가하도록 하겠습니다.
Comment
객체로 관리한다... 정말 좋은 방법인 거 같아요! 특히 구조분해 할당은... 크.... 먼가 알기전과 알고나서의 코드가 크게 변한다라고 할까?
답변
현재 저는 카드 폼의 state{}, 결제자 모달을 위한 modalState{}, setter함수 들로 함수를 분리했는데 이마저도 조금 복잡해 보입니다. 상태를 성능과 가시성 관점에서 좀 더 compact하게 관리할 수 있는 관점이 있다면 어떤 것이 있을까요?
아래의 comment에 일부 적어두긴 했는데 react는 사실 거의 모든 대부분의 데이터를 쉽게 적용할 수 있게 하였습니다. 예를 들면 useState가 아니라 useReducer를 사용하는 것이죠. flux패턴을 사용하면 복잡한 데이터를 조금 더 쉽게 다룰 수 있다고 생각합니다. 이 조차도 store들을 사용하지 않고 먼가 더 개선된 방안을 사용하고 싶다면 immer.js를 엮는 것도 좋아 보입니다.
사실 이 말고도 useSyncExternalStore도 있고 useContext, useRef도 하나의 방법이겠네요. react-qeury에서는 observer를 통해서 데이터를 refetch하거나, useSyncExternalStore을 통해 데이터를 관리해요.
리뷰어가 리뷰이에게
어...일단 useCardForm이긴 한데 form이 없어서 당황스러웠긴 했어요. 물론 없어도 되긴 합니다.
개인적으로는 이쪽에서는 form으로 했으면 더 쉬웠겠구나. 라는 생각을 했어요. form 라이브러리 중 가장 유명한 것으로 react-hook-form가 있어요. 여기서 가장 중요시 되는 함수 중 하나가 register이에요. 현재와 같이 코드를 작성하면 단점이 한개의 field만 리렌더링이 되는 것이 아니라 모든 component가 리렌더링이 될 것이에요. 하지만 위의 register를 사용한다면 어떻게 동작할지 생각을 해보게 되더라구요.
이 함수의 동작을 예상해보고 구현해보는 것도 좋을 것 같아요.
ps. eslint에서 react props의 타입이 없다고 아우성을 치네요. PropTypes를 사용하면 vscode의 intellisence의 도움을 받는데 더 수월합니다.
ps. vercel이 보이지 않아요... 일단 로컬에서 돌렸습니다.
| <Suspense fallback={<div>Loading...</div>}> | ||
| <div className="card-component-container"> | ||
| <CreditCard /> | ||
| </div> | ||
| </Suspense> |
There was a problem hiding this comment.
이곳에 suspense를 넣은 이유는 무엇인가요?
|
|
||
| export const CreditCardPage = () => { | ||
| return ( | ||
| <Layout> |
There was a problem hiding this comment.
Layout을 components가 아닌 common폴더에 넣은 이유가 있나요?
저는 Layout자체는 하나의 components로서 작동하는거 같아 components를 봤었는데, common안에 있어서 기대하는바가 달라 당황했었어요.
| const payerDetail = modalState.payerDetail; | ||
| const payerName = payerDetail.cardOwner; | ||
| const payerCardNumber = payerDetail.cardNumber; |
There was a problem hiding this comment.
꿀팁!
| const payerDetail = modalState.payerDetail; | |
| const payerName = payerDetail.cardOwner; | |
| const payerCardNumber = payerDetail.cardNumber; | |
| const { cardNumber: payerCardNumber, cardOwner: payerName } = | |
| modalState.payerDetail; |
| <label htmlFor={id}> | ||
| <span>{labelText}</span> | ||
| <input className={className} id={id} type={type} placeholder={placeholder} onChange={onChange} /> | ||
| </label> |
There was a problem hiding this comment.
input을 label안에 넣으셨는데 htmlFor을 쓴 이유는 무엇인가요?
| <CreditCardInput | ||
| className={"card-number-input"} | ||
| labelText={"카드 번호"} | ||
| inputValue={state.cardNumber} | ||
| onChange={(e) => setCardNumber(e.target.value)} | ||
| type="text" | ||
| placeholder="카드 번호를 입력해주세요" | ||
| /> |
There was a problem hiding this comment.
이건 모두 구현의 차이라고 생각하긴 하지만, 저희는 조금더 사용자에게 접근성과 보안을 책임져주는 사람으로서 한번 고민을 해보면 좋을 것 같아요!
카드번호의 경우에는 민감한 정보에 들어가는거 같아요. 그렇기 때문에 숫자로 보이는 것이 아니라 type에 password를 넣어 가려주어야 한다고 생각합니다.
다른 실제로 서비스 하는 PG사들의 구현들을 보면 사용자의 UX를 섞기 위하여 맨 앞 8자리나, 앞 4자리 + 뒤 4자리 이런식으로만 숫자를 보여주고 나머지는 가리더라구요.
구현을 할 때 다른 서비스를 한번 참고해보는 것도 좋은거 같아요!
There was a problem hiding this comment.
이건 모두 구현의 차이라고 생각하긴 하지만, 저희는 조금더 사용자에게 접근성과 보안을 책임져주는 사람으로서 한번 고민을 해보면 좋을 것 같아요! 카드번호의 경우에는 민감한 정보에 들어가는거 같아요. 그렇기 때문에 숫자로 보이는 것이 아니라 type에 password를 넣어 가려주어야 한다고 생각합니다. 다른 실제로 서비스 하는 PG사들의 구현들을 보면 사용자의 UX를 섞기 위하여 맨 앞 8자리나, 앞 4자리 + 뒤 4자리 이런식으로만 숫자를 보여주고 나머지는 가리더라구요. 구현을 할 때 다른 서비스를 한번 참고해보는 것도 좋은거 같아요!
이 부분 리뷰하시기 상당히 불편하셨을것 같아요...!
사실 작성할 때는 프리티어가 잘 적용되어 문제를 못느끼는데 이런 식으로 올라오게 된다면 tab setting을 줄이면 해결이될까요?
There was a problem hiding this comment.
file에 prettier가 없어서 아마 적용이 안되거나 default로 적용이 되어 있어서 그렇네요.
tab setting의 경우에는 tab대신 space가 들어가도록 설정하면 대부분 사람들이 고정된 넓이로 볼 수 있습니다!
| if (!isValid) { | ||
| return; | ||
| } |
| return; | ||
| } | ||
| noticeSubmitSuccess(); | ||
| getPayDetail(cardOwner, cardNumber); |
There was a problem hiding this comment.
어떤 기대값을 하는 코드일까요?
저는 개인적으로 get이 접두사로 붙으면 어떠한 데이터를 내뱉을지 기대하고 있어요. 그래서 payDetail이라는 애를 만들어서 return을 해주겠구나 라고 생각했지만, 아래의 코드를 보다보면 이는 set에 더 가까운 코드인거 같았어요.
추가적으로 일반적으로 can이나 is, has의 경우에는 일반적으로 boolean을 기대고, 끝에 복수형인 s를 붙이면 array like를 기대하는 거 같더라구요.
| } | ||
| noticeSubmitSuccess(); | ||
| getPayDetail(cardOwner, cardNumber); | ||
| openModal(); |
There was a problem hiding this comment.
오 좋은 코드인거 같아요. modal을 연다는 일로 명시적으로 나눈 것은 좋은 코드인 것 같습니다.
| getPayDetail, | ||
| setPayerDetail, |
There was a problem hiding this comment.
제가 사용하는 것을 보지 못해서... 제가 발견하지 못한... 어떤 확장성을 고려하셨는지 알려주시면 감사할꺼 같아요!
| const isValidCardOwner = isCardOwner(cardOwner); | ||
| const isValidCardNumber = isCardNumber(cardNumber); | ||
| const isValidCardDate = isCardDate(cardDate); | ||
| const isValidCardCvc = isCardCvc(cardCvc); | ||
| const isValidCardPassword = isCardPassword(cardPassword); | ||
| if (!isValidCardOwner) { | ||
| alert("성명을 다시 확인해주세요."); | ||
| return false; | ||
| } | ||
| if (!isValidCardNumber) { | ||
| alert("카드 번호를 다시 확인해주세요."); | ||
| return false; | ||
| } | ||
| if (!isValidCardDate) { | ||
| alert("유효 기간을 다시 확인해주세요."); | ||
| return false; | ||
| } | ||
| if (!isValidCardCvc) { | ||
| alert("CVC를 다시 확인해주세요."); | ||
| return false; | ||
| } | ||
| if (!isValidCardPassword) { | ||
| alert("비밀번호를 다시 확인해주세요."); | ||
| return false; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
정석? 적인 코드 중 하나인거 같아요. 저도 주어진 조건에 의하면 저렇게 짤꺼 같아요!
하지만... 혹시 다른 방법이 있다면 어떤 방법이 있을까요? 먼가... 조건이 늘어나면 어려울꺼 같아서.
There was a problem hiding this comment.
그리고 추가적으로 alert뿐 아니라 어떤 조건이 있는지 알려주시면 좋을 것 같아요.
예를 들면 비밀번호를 다시 확인해주세요. 하지만 어떤 조건인지 사용자는 전혀 모르기 때문에 답답해 할꺼 같아요.
ps. 참고로 alert는 side effect가 있는 함수랍니다.
1. 구현 모습
2. 해결 과정
https://donut-study-j271z843f-dxwwans-projects.vercel.app/mission3 (배포)
가장 고민을 많이 했던 부분은 input요소들을 얼마나 분리해야할까? 였습니다.
기준은 태그 요소들만 분리를 해주고 로직은 한 컴포넌트에서 이루어지도록 분리하고, 페이지에선 해당 컴포넌트만 보여주는 방식을 선택했습니다.
미션을 하면서 항상 드는 생각은 어디까지 구현해야할까? 였습니다.
이번 미션을 진행하면서 배열로 상태를 관리하는 것 보다는 '객체'로 관리해야 안전하고 접근하기 쉽다는 것이었습니다. 처음에는 모달을 위한 구매자 상태와 카드 번호를 배열로 관래했는데, 접근이 안될 뿐더러 리액트에서는 이를 지양하고 있었습니다. 하지만 객체로 관리하니 접근도 쉬웠습니다.
3. To 리뷰어에게