[2단계 - 페이먼츠] - 버건디(전태헌) 미션 제출합니다. #371
Conversation
| .fill(null) | ||
| .map(() => useRef<HTMLInputElement>(null)); | ||
| return <RegisterCardNumber {...args} refs={refs} />; | ||
| }, |
There was a problem hiding this comment.
이번에 카드넘버를 부모 컴포넌트에서 ref 4개를 만들어서 RegisterCardNumber에게 props로 넘겨주고,
RegisterCardNumber 컴포넌트에서 받아서 map함수로 뿌려주면서 input 컴포넌트를 총 4개 만들어주는데요..!
이렇게 되니까 스토리북을 작성할때 ref를 props로 전달해야해서 이런식으로 스토리를 작성하게 되더라구요.
근데 이런식으로 스토리를 복잡하게 작성하는게 맞는건가? 라는 의구심이 들었습니다.
스토리북을 작성하기 힘들다.라는 것이 컴포넌트 자체가 독립적으로 존재하지 않는다. 즉 의존성이 많다. 라는 것으로 느껴지기도 합니다.
이렇게 되면 아예 useCardNumbers 커스텀 훅 자체를 변경해주어서 설계를 뒤바꾸어주어야 하나..? 라는 생각도 드는데요.
하루의 생각 한번 여쭤보고 싶습니다!
There was a problem hiding this comment.
오 너무 좋은 고민의 흐름이네요 👍 더 좋은 인터페이스를 위해 계속 고민하는 모습 멋져요 😎
refs는 <RegisterCardNumber /> 하위에서 관리하고,
cardNumbersChangeHandler(e, { index, nextRef })이런 식으로 핸들러에 인자로 전달하면 어떨까요?
There was a problem hiding this comment.
헉 맞네요!
말씀주신 방향대로 한번 수정해보았습니다.
짚어주셔서 감사합니다 하루~!
There was a problem hiding this comment.
- 책임이 작은 커스텀 훅들을 모아서 책임을 넓혀보는 것에 대한 방향성
❗️ 이번에 제일 신경쓰였던 부분입니다 ❗️
이번 미션을 진행하면서 처음에 그렸던 방향성은 가장 작은 커스텀 훅들이 뭉쳐지면서, 하나의 큰 커스텀 훅이 되는 것이었어요!
예를 들어서 useInput이라는 커스텀 훅에서는 유효성 검사를 제외하고 단순히 상태값만 받도록 하고
- useInput
const useInput = () => {
const [inputValue, setInputValue] = useState('');
const onChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const inputValue = e.target.value;
setInputValue(inputValue);
};
return { inputValue, onChange };
};- usePassword.ts
const usePassword = () => {
const { inputValue, onChange } = useInput();
const handlePassword = (e: React.ChangeEvent<HTMLInputElement>) => {
// 여기서 password 와의 관련된 유효성 검사를 진행
// 그 후에 상태 업데이트나 다음 인풋 포커스나 다른 기능 수행
onChange(e);
};
return {
passwordValue: inputValue,
handlePassword,
};
};이런식의 흐름을 원했었는데, 어차피 이 안에서 다음 포커스를 넘겨주어야하고, 다음 스텝(카드넘버 => 카드사 선택)을 렌더링 해주기 위해서는 인자를 넣어줄수 밖에 없지 않나? 라는 생각이 들었습니다.
즉, 작은 커스텀훅들을 모아서 책임이 큰 훅으로 점진적으로 넓혀가는것이 정말 재사용성이 있나? 라는 생각이 들었던거 같아요
그래서 현재 단일 인풋값을 체크해주어야할때는 useInput 하나로 재사용을 해준 상태인데요.
이 커스텀 훅내에 들어가는 인자가 많은거 같다는 생각도 듭니다.
저는 결국 단일 인풋값을 다룰때,
- 값을 입력한다.
- 유효성 검사를 해준다.
- maxLength 에 충족할때까지 정상적인 값을 입력한다면 다음 스텝으로 인풋 포커싱을 옮겨준다.
같은 거시적인 큰 틀의 기능이 같다고 판단해서 이렇게 구현해준것인데요.
하지만 자세히 살펴보면 각각의 유효성 검사가 다르다보니, 그냥 각각 섹션에 대한 커스텀훅을 따로따로 만들어주는게 코드 가독성이 더 높을까.. 라는 생각도 들어요
키워드로 따져보면, 코드 가독성 vs 재사용성 이라고 말씀드려볼수 있을거 같습니다!
하루의 의견 한번 여쭈어보고 싶어요! 제가 놓치고 있는 부분이 있다면, 짚어주신다면 새겨듣고 배우겠습니다 🙇
There was a problem hiding this comment.
아하 고민의 포인트를 이해한 것 같습니다.
이 커스텀 훅내에 들어가는 인자가 많은거 같다는 생각도 듭니다.
인자가 많다는 것은 책임이 많다는 것이겠지요. 질문에 적어주신 useInput과 달리, 현재의 useInput은 코드량이 꽤 되보이는데요, 이 코드를 모두 usePassword.ts로 보내면, 정말로 코드 가독성이 더 좋아질까 하는 생각도 드네요.
둘 중 하나만 사용하는 것이 아니라 둘 다 사용하는 방향으로 리팩터링 시도해주시면 어떨까요? useInput의 책임을 조금 덜고, 각 섹션의 특수성은 usePassword등을 통해 다루는 방향으로요.(말로는 참 쉽네요.. 🙄) 그렇게 변경했는데 useInput 없이도 코드 가독성이 좋을 것 같다고 판단되시면 그 때 아예 제거해주시면 어떨까 싶습니다 :)
There was a problem hiding this comment.
상태 관리해야할 인풋값을 기준으로 usePassword(인풋2개) useCardNumbers(인풋4개) useInput(인풋1개)
커스텀 훅을 분리해보았는데 아직도 확신이 서질 않네요.. 🥲🥲
뭔가 관심사 분리 + 재사용성까지 다 챙겨야 된다라는 강박이 있어서 그런거 같기도 합니다.
레벨1때 함수의 재사용성에 대해서 계속 집중해서 공부했다보니까 리액트 와서도 커스텀 훅 또한 무조건 재사용성이 있어야해! 라는 생각이 기저에 깔려있는거 같아요..
말씀 감사합니다 하루 ~!
There was a problem hiding this comment.
와와 감사합니다!!
잘 읽어보겠습니다 마지막까지 배움주셔서 감사해요 하루!
src/App.tsx
Outdated
| name, | ||
| }); | ||
|
|
||
| const handleNavigateToConfirmPage = () => { |
There was a problem hiding this comment.
해당 함수 같은 경우, 아예 ConfirmButton 이라는 컴포넌트를 하나 만들어준다음에, 거기 props 로 isSucceed, cardNumbers, cardIssuer 값을 props로 넘겨주고 해당 컴포넌트내 에서 핸들러 함수를 정의해줄까 했는데요..!
단순히 가독성을 위해 props로 3개의 값을 내려주어서 사용한다는 것 자체가 조금 더 비효율적이라고 판단해서, App 단에서 선언을 해주었습니다!
혹시 아예 props로 state 값들을 내려주어서 핸들러 함수를 내부에서 정의해주는것이 나을까요 ?
잘못 짚고 있는 점이 있다면, 말씀 주시면 감사하겠습니다! 🙇
There was a problem hiding this comment.
큰 차이는 없는 것 같지만, 굳이 고르자면 props로 작성해주시는 것에 데이터 흐름이 더 잘 보일 것 같다는 생각이 듭니다. (개인적으로 navigate의 state는 state로만 가능할 때 쓰는 편이에요.)
There was a problem hiding this comment.
답변 정말 감사합니다! 🙇 props 넘겨주는 방식으로 수정해보았어요!
| {INITIAL_CARD_ISSUER_INFO.map((cardIssuer) => { | ||
| return ( | ||
| <option key={cardIssuer.id} value={cardIssuer.value}> | ||
| {cardIssuer.issuer} | ||
| </option> | ||
| ); | ||
| })} |
There was a problem hiding this comment.
아래 Select 동작을 요구사항에 맞추어 개선해볼 수 있을까요?
| 현재 Select 동작 | 요구사항 |
|---|---|
| 선택 전 기본 BC카드 (다시 BC카드를 선택해도 이후 진행되지 않음) | 선택 전 카드사를 선택해주세요 표현 |
| 재현영상 |  |
아래 MDN 예제를 참고해서 추가로 구현해주시면 좋을 것 같습니다. (마크업 시 추가 스타일링도 필요해보입니다!)
There was a problem hiding this comment.
placeholder 값을 넣어서 구현해보았어요 하루!
제일 중요했던 부분인데, 이걸 빼먹었었네요.. 부족한 부분 많은데 양해해주시고 짚어주셔서 감사합니다 🥲
| onChange: React.ChangeEventHandler<HTMLSelectElement>; | ||
| } | ||
|
|
||
| const Select = forwardRef<HTMLSelectElement, PropsWithChildren<SelectProps>>((props, ref) => { |
There was a problem hiding this comment.
(Select 쪽 피드백 남기려고) 이 컴포넌트를 사용하는 곳을 찾아보려고 했었는데, 지금은 사용되지 않고 있는 것 같네용 혹시 활용해주실 예정인 걸까용?
There was a problem hiding this comment.
헛 아예 스타일드 컴포넌트를 사용해서 해당 Select를 사용하지 않게 되었네요..!
수정하도록 하겠습니다. 짚어주셔서 감사해요 하루~! 🙇
안녕하세요 하루~ 잘지내셨나요 ?
스토리북 배포 사이트 (재배포 완료)
깃허브 페이지 배포 사이트(재배포 완료)
날씨가 슬슬 더워지는데, 일도 일이지만 먹는것도 잘 챙기시면서 더위 대비 하셨으면 좋겠습니다..!
개인적으로 이번 step2가 훨씬 까다롭다고 느껴졌네요 🥲
특히나 코드를 짜면서도 이게 정말 최선인가? 라는 의구심이 계속 들었던거 같습니다.
완벽하다고 생각하지는 않지만, 한번 하루의 말씀 들어보면서 디벨롭해보고자 이렇게 리뷰 요청 드려봅니다.
궁금한점 코멘트로 남겨보았습니다.