[페이먼츠 미션 Step 2] 우디(류정우) 미션 제출합니다.

[jw-r]

페이먼츠 배포

스토리북 배포

동동 안녕하세요!
코드를 쓰고 지우고를 반복하다보니 제출이 늦어졌네요ㅠㅠ

그런데도 아직 부족한게 너무 많아요
특히 Step2를 진행하며 Styled Component, Typescript에 대해서는 거의 신경쓰지 못했어요 (any 있음..)
동동의 리뷰를 기다리며 공부해서 적용해 보도록 하겠습니다🥺

아래는 Step2를 진행하며 고민을 많이 했던 부분입니다

1. 컴포넌트 내에는 이벤트와 렌더링 관련 로직만 넣자

• 대체 어디까지가 렌더링, 이벤트 관련 로직인가

컴포넌트 내의 상태, set함수, on함수 이외의 로직은 모두 숨겨야할까?
set함수와 on함수의 구현부는 어쩌지? 이것 또한 숨겨야하나?

간단한 예시를 들어보겠습니다

const SelectBank = () => {
  const { updateBank } = useCardInfoActions();

  const onClick = (e) => {
    const { id } = e.currentTarget;
    updateBank(id);
  };

  return (
    <styled.SelectBank>
      <styled.Banks>
        {Object.keys(BANKS).map((key) => (
          <styled.Bank key={key} id={key} onClick={onClick}>
				{/* ... */}
          </styled.Bank>
        ))}
      </styled.Banks>
    </styled.SelectBank>
  );
};

위에서 onClick의 구현부를 굳이 숨기고자 한다면 아래와 같이 숨길 수 있었어요

const SelectBank = () => {
  const { onClick } = useSomething();

  return (
    <styled.SelectBank>
      <styled.Banks>
        {Object.keys(BANKS).map((key) => (
          <styled.Bank key={key} id={key} onClick={onClick}>
				{/* ... */}
          </styled.Bank>
        ))}
      </styled.Banks>
    </styled.SelectBank>
  );
};

컴포넌트가 간단해지는 장점이 있지만 이 컴포넌트의 코드만 봐서는 onClick 이벤트가 일어났을 때,
어떤 일이 일어나는지 전혀 알 수가 없어요
useSomething르 까봐야 알 수 있죠

• What을 드러내고 How를 숨기자?
• What도 How도 아무것도 없는 것 같은데...........

반대로 첫 번째 예시는 적어도 은행에 대한 무언가를 업데이트 한다정도는 알 수가 있었어요

그럼 on~~과 같은 이벤트 함수에 대한 �구현부는 컴포넌트 내부에 두는 것이 좋을까요?

아래는 또 다른 예시예요

const CardRegisterPage = ({ setCardList }) => {
  const cardInfo = useCardInfoValue();
  const { onSubmit, onChange, error } = useForm();

  return (
    <styled.CardRegisterPage>
      <CardPreview cardInfo={cardInfo} openModal={openModal} />
      <styled.CardRegisterForm onSubmit={onSubmit}>
        <input value={cardInfo?.something} onChange={onChange} />
      </styled.CardRegisterForm>
    </styled.CardRegisterPage>
  );
};

export default CardRegisterPage;

처음 예시와 같이 이벤트 함수에 대한 구현부가 모두 숨겨진 �상태이지만, 굳이 알 필요가 없어보여요
컴포넌트의 이름, form에 등록된 onSubmit라는 부가 정보들로 카드를 등록하겠구나정도의 정보는 알 수 있어요
Input에 등록된 onChange도 마찬가지이고요

역시 컴포넌트가 깔끔해지기 위해서 이벤트 함수에 대한 구현부도 숨기는 것이 좋을까요?

그럼 첫 예시에서 onClick에 대한 정보는 useSomething를 뜯어보게 하면 되는 걸까요

아니면 이 모든 걸 고려해서 이벤트 함수의 구현부를 숨길지 말지 상황에 맞게 판단하면 되는 걸까요

const { showModal, openModal, closeModal } = useModalSwitch();

위 처럼 구현부를 숨기더라도, 호출했을 때 어떤일이 일어날지 명백하게 의도가 들어나는 경우에만 숨기는 것이 좋을까요?

ㅠㅠㅠ동동의 의견이 궁금합니다😭

---

2. 재사용 가능한(npm에 배포할 만한) Custom Hook을 만들자

• 고려해야 할 것이 너무 많다 ㅠㅠ (JavaScript로 하면 쉬울 것 같은데..)
  • 해당 프로젝트 안에서 재사용 가능하게 만드는 것 만으로 충분한 걸까

---

3. Custom Hook 안에서 Custom Hook?

• 너무 작게 쪼개면 Custom Hook안에서 Custom Hook을 사용해야 하는 일이 생긴다
  • 어떤 Custom Hook 안에서 사용되는 Custom Hook이 독립성이 강하다면 괜찮다고 생각한다
    • 의존성이 낮아지기 때문에 유지 보수가 크게 어려워지지 않기 때문이다

동동의 의견이 궁금해요!

---

3. Portal은 그저 스타일을 분리하기 위함인가

• 상위 컴포넌트의 스타일에 영향을 받지 않기 때문에 모달 창과 같은 컴포넌트를 만들 때 유용했다
• 하지만 테스트를 할 때마다 <div id='modal-root'></div> 태그를 만들어서 넣어줘야해서 불편하다
• Protal의 장점이 굳이 없어도 상관없는 거라면 사용할 필요가 있을까?

동동의 의견이 궁금합니다!

---

4. 폴더 구조

• 몇 없는 페이지 수를 가진 이 작은 어플리케이션에 왜 이렇게 폴더가 많지..
• 상하 관계로 폴더 구조를 정리하는 것이 현재로서는 가장 좋은 방법인 것 같다
• 하지만 import 가 너무 지저분해진다
• 같은 폴더에 존재하는 export를 모아서 다시 export 해주는 방식으로 뎁스를 조금이라도 줄여보려 했다
• 크게 달리지는 건 없다

동동은 사이드 프로젝트를 할 때 등, 어떤 식으로 폴더 구조를 나누는 것을 선호하시나요??

---

5. UX

나름대로 UX를 신경써보려 했습니다

• Form 컴포넌트 렌더 시, 가장 처음 입력해야 할 Input에 focus
• 입력한 글자 수가 maxLength와 같다면 다음 Input이 focus
• 제출 시, 올바르지 않은 값을 가진 Input이 있다면 에러 메세지 표시와 해당 Input으로 focus

TODO

모바일 환경에서 사용하는 애플리케이션인 만큼, 키패드에 대한 UX를 추가 할 예정입니다

---

Step2에서도 잘 부탁드립니다!!

동동 최근에 리뷰가 조금 늦었던 이유가 결혼 때문이라는 소문이 있던데 사실인가요!?
아주 매우 축하드려요🤩
밥 사주신다던 말씀은 잊지 않고 있습니다 ㅎ

[bigsaigon333]

리뷰 시작합니다~~

[bigsaigon333]

우디 안녕하세요~

고민을 많이 해보신게 코드에 보여서 좋았습니다.

useForm 같은 훅도 잘 작성해주신거 같아요.

정말 form의 끝판왕을 보고 싶다고 하면, react-hook-form의 API들을 참고해보시는걸 추천합니다~

그리고 타입스크립트는 저는 정말 중요하다고 생각해요. 어쩌면 리액트로 컴포넌트를 잘짜는거 그 이상으로요.

요 코멘트로 제가 하고 싶은 말을 대신하겠습니다...!

#204 (comment)

---

1. 컴포넌트 내에는 이벤트와 렌더링 관련 로직만 넣자

처음 예시와 같이 이벤트 함수에 대한 구현부가 모두 숨겨진 �상태이지만, 굳이 알 필요가 없어보여요
컴포넌트의 이름, form에 등록된 onSubmit라는 부가 정보들로 카드를 등록하겠구나정도의 정보는 알 수 있어요
Input에 등록된 onChange도 마찬가지이고요

const { showModal, openModal, closeModal } = useModalSwitch();
위 처럼 구현부를 숨기더라도, 호출했을 때 어떤일이 일어날지 명백하게 의도가 들어나는 경우에만 숨기는 것이 좋을까요?

이게 정답이라고 생각합니다.

어떻게 구현했는지 잘 모르겠지만, 대충 이렇게 되겠구나 라고 유추가 되고 의도가 명백한건 훅으로 분리해도 되고,
제일 처음 예시에서, 단순히 그냥 컴포넌트가 비대해지는게 싫어서 코드를 분리해낸 건 의미가 없다고 생각해요.

커스텀 훅은 유틸함수랑 똑같습니다. 리액트 컴포넌트의 라이프사이클만 추가된 거죠.
함수 이름만 봐도, 인자로 무엇을 전달하는지만 봐도, 반환값이 무엇인지만 봐도 어떻게 사용하면 되겠는지 알게끔 작성하는게 중요하다고 생각해요.

---

2. 재사용 가능한(npm에 배포할 만한) Custom Hook을 만들자

이건 학습목적이니까 너무 고민안해도 된다고 생각합니다. npm 에 배포까지는 아니지만, 다음 미션에도 써먹을 수 있겠다 싶을 정도까지 고민해보면 좋을거 같아요. (근데 그게 많은 상황을 겪지 않고 바로 그런 유연한 훅을 만드는게 쉽지 않습니다 ㅎㅎ 저도 마찬가지..)

---

3. Custom Hook 안에서 Custom Hook?

훅 안에 훅이 당연히 들어올 수 있죠 ㅎㅎ 커스텀훅안에 useState, useEffect 다 쓰지 않나요?
useState와 useEffect와 같이 독립적으로 작게 잘 만들면 문제가 없다고 생각해요~

---

4. Portal은 그저 스타일을 분리하기 위함인가

지금은 무조건 div#modal-root가 있다는 가정하에 모달을 만들고 있는데요,
없으면 div#modal-root를 생성해서 DOM에 추가하게끔 코드를 수정해보세요.
그리고 모달이 꺼지면 div#modal-root를 없애버려도 되구요 ㅎㅎ

Portal의 장점이 필요없다면 안해도 되죠 ㅎㅎ
근데 앱이 커지면 스타일 간섭이 일어날 수 있기 때문에,
나중에는 쓰게 되는거 같습니다.

이런것도 useModal 같은 훅을 만들어서 컴포넌트 쪽에서는 Portal을 쓰는지 안쓰는지 모르게끔 해버릴 수도 있어요~
뭐 대충 이런 느낌으로...

const modal = useModal();

const handleClick = () => {
  modal.open(({isOpen, open, close}) => <div>모달 오픈 무야호~</div>});
};

---

5. 폴더 구조

저도 이런고민을 예전에 많이 했는데요.. 솔직한 제 답변은 그게 뭐 중요한가? 입니다 ㅎㅎ..
일관되게 쓰는게 젤 중요하고, depth가 깊어지는것도 뭐 이정도면 괜찮지 않나? 라고 생각해요.
../../../ 같은 상대경로가 보기 불편하다면 절대경로로 import 하는걸 도입해보는것도 방법이겠네요.

더 줄이려면, 디자인시스템을 도입해서 styled 컴포넌트를 많이 날리고,
공통 라이브러리를 도입해서 hook도 많이 날리고 그러면 됩니다 ㅎㅎ
(저희 회사코드가 그렇...)

딴얘기로 살짝 새어보면..
저는 사이드프로젝트 할 때 아무 생각 안합니다 ㅎㅎ
가끔씩 JS도 씁니다. 타입 신경 안쓰고 자유롭게 해서 너무 기분 좋아요~~ ㅋㅋ

[bigsaigon333]

Suggested change

	background-color: ${(props) => (props.bgColor ? props.bgColor : 'black')};
	background-color: ${(props) => (props.bgColor || 'black')};

요런건 어떤가요?!

[bigsaigon333]

그냥 Object.entries로 key랑 value를 다 가지고 와도 좋지 않았을까 싶네요~

[bigsaigon333]

Suggested change

	{cardList?.map((cardInfo: CardInfo) => (
	<div key={cardInfo.id}>
	<Card cardInfo={cardInfo} />
	<styled.Nickname>{cardInfo.nickname}</styled.Nickname>
	</div>
	))}
	</styled.CardList>
	{cardList.map((cardInfo) => (
	<div key={cardInfo.id}>
	<Card cardInfo={cardInfo} />
	<styled.Nickname>{cardInfo.nickname}</styled.Nickname>
	</div>
	))}
	</styled.CardList>

1. cardList가 nullable할 때가 있나요?
2. CardInfo 타입이라고 명시하지 않아도 될거 같아요

[jw-r]

1. 빈 배열일 수는 있어도 nullable할 때는 없네용.. 수정하겠습니다
2. cardList 변수가 CardInfo[] 타입을 가지고있고, cardInfo 변수는 cardList의 요소이니 CardInfo로 타입을 추론할 수 있군요
   감사합니다!!

[bigsaigon333]

Suggested change

	const CardListValueContext = createContext<CardInfo[]>([] as CardInfo[]);
	const CardListValueContext = createContext<CardInfo[]>([]);

[jw-r]

아무 의미없는 타입 단언을 사용하고 있었네요😭

[bigsaigon333]

{}에 setCardList 속성이 없는데 있다고 이 타입스크립트는 거짓말을 하고 있네요 ⛔

CardListProvider 에서 무조건 actions를 넣어주니까 런타임에서 에러는 안날거 같긴 하지만,
as를 안쓰고 하는 방법을 적용해주세요!!

힌트: https://kentcdodds.com/blog/how-to-use-react-context-effectively#typescript

[jw-r]

오 좋은 자료 감사합니다!!
다른 파일에서는 상태와 액션에 대한 타입을 알고 있도록 context 내에서 네로잉 해줬어요

[bigsaigon333]

Suggested change

	const formElements = target.closest('form')?.elements;
	const formElements = target.form?.elements;

이렇게 하면 안되나용?

[jw-r]

이렇게 찾는 방법도 있군요!
리터럴이 안들어가니 훨씬 보기 좋은 것 같아요!!

[bigsaigon333]

DOM api를 많이 알아두면 좋습니다 ㅎㅎ

DOM api와 유사하게 props를 지으면, 모든 FE 개발자들이 이해하기 편해져서요~

[bigsaigon333]

Suggested change

	elements.forEach((elem, index) => {
	if (elem !== target) return;
	elements[index + 1]?.focus();
	});
	const targetIndex = elements.findIndex((elem) => elem === target);
	if (targetIndex === -1) return;
	const nextIndex = targetIndex + 1;
	if (nextIndex >= elements.length) return;
	elements[nextIndex].focus();

이렇게 하는건 너무 짜치나요? ㅎㅎ..

[jw-r]

elem === target인 상황을 찾아서 elements[index + 1]?.focus();를 실행했다고 하더라도 forEach 특성상 모든 요소를 확인해야하니 동동의 로직이 더 효율적인 것 같아요!

const targetIndex = elements.findIndex((elem) => elem === target);
if (targetIndex === -1) return;

const nextIndex = targetIndex + 1;
elements[nextIndex]?.focus();

하지만 위의 코드와 같이 존재한다면 focus해만으로도 충분히 이해가 가능한대,
굳이 한번 더 조건문을 통해서 return을 해주는 이유가 있을까요?

[bigsaigon333]

하지만 위의 코드와 같이 존재한다면 focus해만으로도 충분히 이해가 가능한대,
굳이 한번 더 조건문을 통해서 return을 해주는 이유가 있을까요?

별로 깊게 생각해보진 않았어요 ㅎㅎ

그냥 if문으로 return 해주는게 더 명시적으로 나타낸다고 생각해서 그렇게 짰었는데,
우디가 한것처럼 해도 좋을거 같아요 👍🏻

[bigsaigon333]

#204 (comment)

[bigsaigon333]

GlobalStyles만 있을 때에는 뭐 이렇게 쓸 수도 있지 라는 생각이라서 코멘트를 안남겼는데요,
Provider가 막 들어오니까 뭔가 좀 일반적이지 않다고 느껴지는거 같아서 코멘트 남겨봅니다..!

Provider도 App의 일부여서 Provider가 없으면 App이 지금 정상적으로 동작 안하기 때문에
App 컴포넌트 안에 작성되어야 하지 않나? 라는 생각이 좀 드는거 같아요.

제가 생각했을 때에 index.tsx에는 센트리 등 아예 도메인 로직과 상관없는 인프라 성격의 것들이 오는것이 좀 일반적이지 않나? 라고 생각합니다.

근데 그냥 제 생각이라서, 우디도 우디만의 기준을 한번 정립하고 넘어가셨으면 좋겠네요~

[jw-r]

전 동동이랑 오히려 반대로 생각했던 것 같아요

현재 GlobalStyles와 Provider가 없으면 App이 정상적으로 동작하지 않기 때문에 이들이 App의 상위라고 생각했어요
Provider에서 지정한 value를 "App"이 사용하니까요
그래서 App을 감싸주어야겠다고 생각했습니다!

제가 생각했을 때에 index.tsx에는 센트리 등 아예 도메인 로직과 상관없는 인프라 성격의 것들이 오는것이 좀 일반적이지 않나? 라고 생각합니다.
제가 아직 인프라 성격의 것들을 많이 보지 못해서 이렇게 생각하지 못한 것 같아요

앞으로 점점 겪어가면서 인프라 성격의 것들을 추가해나가면서, GlobalStyles, Provider와 성격이 다르다고 느껴지면 동동의 말대로 바뀔지도 모르겠어요!

[bigsaigon333]

useState의 setState는, 컴포넌트가 리렌더링되어도 참조값이 변하지 않죠.

커스텀 훅과 같은 유틸성 훅도 이러한 포인트를 잘 지켜줘야한다고 생각해요.
그래야 믿고 deps array에 넣을 수 있다고 생각합니다

[jw-r]

이 코맨트가 잘 이해가가지 않아요😭

set함수에 값만 넘겨주는 경우 참조값이 변하지 않으니 다른 파일에서 state를 참조할 때 업데이트 되지 않은 값을 참조할 수도 있다는 걸까요?

때문에 아래와 같이 함수를 넘겨주는 것이 적절하다는 의도셨는지 이해가 잘 가지 않습니다ㅠㅠ

  const turnOn = () => {
    setState((prev) => !prev);
  };

  const turnOff = () => {
    setState((prev) => !prev);
  };

만약 위의 의도가 맞다고 하신다면 아래와 같은 경우는 어떨까요

const useLocalStorage = (key) => {
  const [localData, setLocalData] = useState(JSON.parse(localStorage.getItem(key) || '[]'));

  const setValue = (value) => {
    setLocalData(value);
    localStorage.setItem(key, JSON.stringify(value));
  };

  return [localData, setValue];
};

외부에서 localData를 참조하고 있는데 set 함수를 이전 상태 값을 변경하는 것이 아니라, 새로운 상태 값을 생성하도록 했습니다
이럴 경우에도 문제가 발생할 수 있을까요?

setLocalData((prev) => value);

그렇다고 위와 같이 사용하지도 않는 이전 값을 불러와서 반환해주는 건 이상하게 느껴져요

[bigsaigon333]

ㅎㅎ 제가 처음 코멘트를 잘못적은거 같네요

저는 useSwitch가 호출될 때마다 turnOn과 turnOff 의 참조값이 변경된다는 점을 말하고 싶었어요

  const 컴포넌트 = () => {
    const { turnOn } = useSwitch();
  
      useEffect(() => {
        // 생략
      }, [turnOn]);

    // 생략

어떠한 이유로 컴포넌트가 리렌더링이 된다면, turnOn의 참조값은 매번 바뀔 것이고, 그렇다면 useEffect는 매번 실행되겠쬬

그에 반해 아래의 코드에서는 컴포넌트가 리렌더링 되더라도 useEffect는 최초 한번밖에 실행되지 않습니다.
setState의 참조값은 항상 동일하기 때문이에요.

  const 컴포넌트 = () => {
    const [state, setState] = useState();
  
      useEffect(() => {
        // 생략
      }, [setState]);

    // 생략

그래서 저는 이런 함수(객체)를 반환하는 low-level의 커스텀훅들은 참조값을 불변하게 만들어서 내보내주는게
중요하다는 점을 말하고 싶었어요 😄

(참조값 불변 방법은 아시죠? ㅎㅎ)

[bigsaigon333]

요거는 기존 코드대로 다시 수정하는게 좋겠네용~

[bigsaigon333]

+참조값 불변도 적용해보면 좋을거 같구요~

[bigsaigon333]

(step3 에서요~~)

[bigsaigon333]

우디 안녕하세요~

변경하신 코드 봤는데, 아래 하나 빼고는 다 이견없습니다~

#267 (comment)

요거는 좀 미스커뮤니케이션이 있었던거 같아요. 코멘트 확인해주시고 질문있으시면 더 남겨주세요~

수정은 step3 하시면서 해도 충분할거 같습니다~

그럼 step3도 화이팅 💪🏻 💪🏻 💪🏻