태초의 송재한은 어리석었다

[Wisesaturn]

안녕하세요 더러미 끝판왕, 송재한입니다.
제가 생각하기에 최적의 더러미 코드는 바로 첫 프로젝트 당시 작성한 코드 가 아닐까 생각합니다 😃
그래서 부끄럽지만 저의 첫 프로젝트를 뜯어보아 여러분들께 한 접시 대접해드리려 합니다

코드

interface OnBoardingType {
  setNext: Dispatch<SetStateAction<boolean>>;
}

export default function OnBoardingPage(props: OnBoardingType) {
  const { setNext } = props;
  const [last, setLast] = useState(false);
  const onNext = () => setNext(true);

 ...(생략)...

  return (
    <Layout
      header
      hideTitle
      customRightChild={
        !last && (
          <div className="cursor-pointer" onClick={onNext}>
            건너뛰기
          </div>
        )
      }
    >
      <OnBoardingContainer>
        <Swiper
          className="h-[calc(100%-70px)]"
          onReachEnd={() => setLast(true)}
          {...OnBoardingProps}
        >
          {OnBoardingData.map((data, idx) => (
            <SwiperSlide
              key={idx}
              className="flex flex-col gap-1 justify-center items-center pt-10"
            >
              <h3 className="text-primary mb-5">{data.title}</h3>
              <p className="text-secondary">{data.sub}</p>
              <p className="text-secondary">{data.sub2}</p>
              <OnBoardingAnimation index={idx} />
            </SwiperSlide>
          ))}
        </Swiper>
        <Button
          onClick={onNext}
          className={last ? 'visible' : 'invisible'}
          width="full"
          label="시작하기"
        />
      </OnBoardingContainer>
    </Layout>
  );
}

화면

구현 내용

모바일 웹앱 프로젝트 당시 온보딩 화면을 구현했습니다.
애니메이션은 써드 파티(Lottie) 로 구현했고, 슬라이더를 통해 사용자에게 어플리케이션 설명을 담았습니다.

넌 왜 이렇게 짰니?

interface OnBoardingType {
  setNext: Dispatch<SetStateAction<boolean>>;
}

export default function OnBoardingScreen(props: OnBoardingType) {
  const { setNext } = props;
  const [last, setLast] = useState(false);
  const onNext = () => setNext(true);

이 코드는 실제로 프로젝트 할 때 좀 뚜들겨 맞았었지만 다시 좀 정신차리기 위해... 복기해보았습니다.
문제점을 두 가지만 뽑아보겠습니다.

1. Props로 useState의 setter 함수를 그냥 넘겨버린다 (도대체 Why)
2. onNext의 용도를 도저히 모르겠다 (네이밍도 구리고, 왜 만든건지 모르겠음)

흠... 제가 이 코드를 봤을 때 별로인 감정이 드는 이유는 바로 불친절함 때문 같습니다.
어떤 네이밍으로 동작하는지 확인이 안되고, 컴포넌트 자체에 너무 많은 기능을 담고 있습니다. (관심사 분리 필요)
여러분들의 생각은 어떠실까요?

[h1un]

(🐣리액트입문자 : )

1. 보면서 든 생각은 '첫' 프로젝트다 보니 리뷰어가 다양한 방법에 대해서 제시해준게 아닐까 하는 생각도 듭니다.
   컴포넌트의 재사용성과 데이터흐름을 명확하게 하는법을 알려주기 위한것이 아닐까 ..

2. 질문 : onNext의 왜만든건지 모르겠다고 했는데, onNext 안만들었다면 어떤식으로 사용할예정이었나요?

[qhahd78]

• onNext 가 문제가 되는 이유는 해당 컴포넌트 내에서 관리되어야하는 state setter인데 부모에서 내려주어서 문제가 되는건가요? 그렇다고 한다면 useState 를 컴포넌트 내부로 옮기는 것이 좋아보입니다!

• 그런데 부모 컴포넌트에서도 이 useState를 써야한다면? props 로 setter 를 직접 전달 받는 것은 그리 잘못된 것은 아닌 것 같아보이는데....

  • (궁금증) setter 를 직접 넘겨주었을 때 문제점이 있을까요?

• onNext 함수도 딱히 setState 말고 하는 작업이 없으니 onNext 로 함수를 따로 빼지 않고 setNext 를 바로 써도 될 것 같습니다.

결론 : 저도 재한님의 생각처럼 네이밍 (last, setNext 와 같이) 과 관심사 분리가 필요할 것 같습니다.

코드 잘 봤습니다 bb

[regularPark]

송재한은 태초부터 뛰어났다.

• 네이밍 측면: 확실히 onNext는 어떤 함수인지 추측하기 어려운 네이밍인 것은 맞습니다.
• setter를 props로 넘겨주는 것: 크게 문제되지는 않을 것 같습니다. 리뷰어 분의 말씀처럼 onNext 함수를 만들어 내려주는 것도 좋지만, 익명함수로 바로 사용하는 것도 괜찮을 수 있겠네요.

[bbearcookie]

첫 프로젝트로 작성하신 코드임에도 엄청 깔끔하군요.. 역시 태초부터 뛰어나신 재한님.. 👍👍

저는 Setter를 직접 외부에 노출하는 것에 어떤 문제점이 잠재할 수 있을지 궁금해서 찾아보았는데, Setter를 지양해야 하는 이유라는 글이 있었어요! 내용을 간단하게 훑어보았는데, 프로퍼티에 어떤 값이든 대입해서 변경할 수 있게 되는 것이기 때문에 변경의 의도가 무엇인지를 파악하기 어려워질 수 있다네요.. (Setter에 접근할 수 있는 모듈이 많아질수록, 상태 변화 추적이 어려워 질 수도 있겠다는 생각이 들었어요 😅)

React 공식 문서의 useReducer 잘 사용하는 법 파트에서도 각 action은 데이터 안에서 여러 상태 변경들이 있더라도 하나의 사용자 상호작용을 설명해야 한다는 가이드가 있었는데, 상태(프로퍼티)를 변경하는 각각의 Setter를 직접 노출하기보다는, 보다 명확한 의도를 드러내는 상호작용을 외부로 노출하는 걸 권장하는 부분과 유사한 맥락이지 않을까 싶어요! 😁

onNext라는 프로퍼티명으로 말미암아 퍼널과 연관이 있는 로직이라는 생각이 들어서, 퍼널 toss/slash 라이브러리에서 제공하는 Hook 중 useFunnel을 살펴보았어요. 가이드 문서 중에 setStep을 prop으로 컴포넌트 안으로 주입하여 Imperative하게 스텝을 전환하는 로직은 지양하는게 좋습니다. 라는 문장이 있는데, 이 부분도 상태를 변경할 권한이 있는 모듈의 갯수를 줄이고, 상태 추적을 용이하게 하자는 아이디어로 느껴졌어요.

[KyuTae98]

앞에 분들이 모두 좋은 말을 써주셨네요,,,
상훈님이 조사 해주신 것처럼 setter를 넘길 때 변경 의도를 파악하기 어렵다고 해주셨는데 반성하게 되네요...

요즘 레거시와 함께 코딩하고 있는 입장에서 아무리 네이밍으로 설명을 해도 완벽하게 이해하기 어려울 떄가 있다고 생각합니다!!

다시 리뷰로 돌아와서 저였어도 onNext로 굳이 싸는 이유를 잘모르겠습니당.. 그냥 setter가 필요한 상황이라면 그냥 setter로 전달해도 되지않을까 싶어요!

굳이 굳이 더 본다면 props를 비구조화 할당으로 받는 부분과 정확한 네이밍 입니다!!