Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FE] refactor: 리뷰 url 생성 폼의 setState 전달 및 비밀번호 에러 메세지 렌더링 방식 변경 #996

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

ImxYJL
Copy link
Contributor

@ImxYJL ImxYJL commented Dec 10, 2024


🚀 어떤 기능을 구현했나요 ?

setState 전달 방식 수정

  • 부모(폼)의 setState를 자식(InputField)으로 넘길 때 Wrapper 함수를 사용했습니다.
  • 예전 코드리뷰에서 setState 함수를 props로 그대로 넘기면 위험하다는 코드리뷰를 반영했습니다.

비밀번호 에러 메세지 디스플레이 방식 변경

  • 비밀번호 에러 메세지는 onBlur 이벤트에 의존했는데, 이제 onChange를 사용합니다.
  • onBlur를 제거하고, password가 빈 문자열('')일 때 에러 메세지를 출력하지 않는 방식으로 구현헀습니다.
  • onBlur 이벤트 특성상 포커스를 따로 주지 않는 이상 에러 메세지 상태가 초기화되지 않기 때문에, 올바른 비밀번호를 입력했음에도 에러 메세지가 사라지지 않는 현상이 있다는 피드백을 반영했습니다.

🔥 어떻게 해결했나요 ?

  • handleInputChange라는 setState의 Wapper 함수를 통해 setState를 전달합니다.
const handleInputChange = (setter: React.Dispatch<React.SetStateAction<string>>) => (value: string) => {
    setter(value);
  };

// 전달 형식
<ProjectNameField ... setValue={handleInputChange(setProjectName)} />

// 사용

 <Input
        ...
        onChange={(event) => {
          setProjectName(event.target.value); // setValue: setProjectName으로 받아옵니다
        }}
      />

📝 어떤 부분에 집중해서 리뷰해야 할까요?

  • 큰 변화가 없으니 가볍게 봐주세요~
  • 다만 Wrapper 함수를 사용해야 하는지는 얘기해보고 싶습니다.

📚 참고 자료, 할 말

setState의 Wapper 함수 필요성에 대해 모호하게 넘어갔던 것 같은데요, 공부한 내용을 공유합니다!

setState를 왜 자식에게 전달하면 안 되는가?

여러분이 짐작하시는 그 이유가 맞습니다. 상태 관리 복잡성/예측 불가능성 증가, 상태 업데이트 로직의 캡슐화 불가능 등등이 있습니다.

Wrapping을 하면 뭐가 달라지나?

하지만 Wrapping을 해도, 결국 상태 업데이트를 자식에게 맡기는 건 같기에 큰 차이가 있는지 궁금했습니다.
결론은 상태 변경 로직의 책임을 부모가 진다는 점에서 큰 차이가 있었습니다.

Wapper 함수를 정의하면, 상태 업데이트 로직을 자식에게 위임하지 않고 자신이 갖고 있을 수 있다는 데 의의를 두는 것 같습니다.
자식은 단순히 부모에게 받은 함수를 트리거하기만 합니다.

하지만 이 코드에서는 부모가 자식에게 받은 상태를 추가로 가공하지 않고 그대로 사용하기 때문에 Wrapper의 의미가 크지는 않다고 생각합니다.
그래서 의미론적 & 확장성 면에서 이 코드를 유지할지, 가독성을 위해 원래대로 돌릴지 고민중입니다!
코드 몇 줄만 추가됐을 뿐 가독성이 크게 나빠진 건 아니라 아마 유지할 듯 싶긴 합니다.
더 복잡한 상태 업데이트 로직이 들어오면 Wrapper를 사용하는 게 확실히 좋을 것 같다는 의견입니다~

Copy link

Deploying 2024-review-me-release with  Cloudflare Pages  Cloudflare Pages

Latest commit: 93331a8
Status: ✅  Deploy successful!
Preview URL: https://78b8d870.2024-review-me-release.pages.dev
Branch Preview URL: https://fe-refactor-995-review-url-f.2024-review-me-release.pages.dev

View logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
1 participant