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

[1단계 - 페이먼츠 모듈] 초코(강다빈) 미션 제출합니다. #21

Merged
merged 56 commits into from
May 7, 2024

Conversation

00kang
Copy link
Member

@00kang 00kang commented May 2, 2024

안녕하세요 블링! 초코🍫입니다.
자동차경주 온보딩 미션 이후 오랜만이에요. 잘 지내셨나요?
페이먼츠 모듈 1단계 미션 리뷰 잘 부탁드립니다. !!

🔗 배포 링크

모달 컴포넌트 npm 라이브러리 바로가기

모달 스토리북 링크 바로가기

훅 npm 라이브러리 바로가기


🔑 키워드 및 학습 목표

  • Module
  • 재사용 가능한 모듈화된 컴포넌트(모달, 커스텀 훅)를 개발하고 npm에 배포할 수 있다.
  • Storybook과 RTL을 활용한 컴포넌트 문서화 및 테스트 시나리오를 작성할 수 있다.

😮‍💨 상황공유

저는 우아한테크코스를 시작하며 웹 프론트엔드에 입문했에요. 그래서 자바스크립트에 대한 이해도도 낮고, 리액트도 아주 얕게 학습해보았고, 기본 훅인 useState와 useEffect만 사용해본 상태에요. 이 점 참고하여 리뷰해주시면 감사하겠습니다!

+) 현재는 훅에 대한 테스트 코드 오류를 잡지 못해 올리지 못했습니다. 코드 리뷰받고 리팩토링하면서 같이 올릴 수 있도록 하겠습니다!!

❓ 질문사항

  • npm library로 만들기 위해 필요한 요소들을 lib 디렉토리 하위에 모두 존재해야 하는지 모르겠어서 일단은 모든 소스코드를 하위에 두었어요. 그런데 결국은 index.ts를 기준으로 빌드가 될 텐데 lib/index.ts 에 export만 잘되어 있다면 상관없는가 싶었어요. 어떻게 코드 구조를 가져가면 될까요? 더불어, 처음으로 라이브러리를 만들고 배포해보는 입장에서 고려해야 할 요소들은 무엇이 될까요?

  • 모달 컴포넌트와 유효성검사 훅 라이브러리를 만들어보면서 사용자에게 어디까지 맡겨야 하는가? 열어놔야 하는가? 고민이 되었어요. 사용자에게 자율성을 줘야하는지, 개발자가 원하는 동작을 위해 어느 정도 지정해둬야 하는지 고민했어요. 예시로 파란색 버튼을 제공하고 있지만, 누군가는 빨간색 버튼으로 지정하고 싶다면 색상을 지정할 수 있도록 받아야 하는가? 싶어요.

  • 열기, 닫기 동작이 이루어지는 useModal 커스텀 훅을 storybook으로 테스트하려면 어디까지 해야할까요.? 모달 컴포넌트는 어떠한 버튼을 클릭하면 나타나는데, 그러면 스토리북에서 버튼 클릭하여 열고, 버튼 클릭하여 닫는 동작까지 하나의 스토리로 볼 수 있어야 할까요, 모달 컴포넌트만 보여주면 될까요?

  • 이번 미션 요구사항 중 RTL이라는 개념을 처음 알게되었어요. Jest는 Javascript를 위한 테스트 도구이고, RTL은 React를 위한 테스트 도구이다. 이외에 다른 점이 있을까요? 어떻게 학습하면 이 차이를 알 수 있을까요. 추천하는 학습 자료 알려주시면 참고하여 학습해보겠습니다.

  • 지금 수준에서 딱 한 단계 나아갈 수 있다면, 현재 제 코드에서 개선하는 포인트를 어디로 잡아야 할까요? 블링이 보시기에 리팩토링 시 꼭 해야 할 포인트를 알려주세요!

0jenn0 and others added 30 commits April 30, 2024 15:06
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
x 버튼 이미지 추가

Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Co-Authored-By: river <130737187+0jenn0@users.noreply.github.com>
Co-Authored-By: 00kang <70834044+00kang@users.noreply.github.com>
Copy link

@uk960214 uk960214 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 초코 오랜만에 뵙네요!
이번에도 역시 리드미도 꼼꼼하게 구성해주시고 사용자에게 친절하게 만들어주셨네요 😄

👍 이런 점이 좋았어요

  • 코드가 간결하고 이해하기 좋았어요.
  • 전반적으로 좋은 구조로 구성해주신 것 같아요! 훅에 대한 이해도 부족하다고 하셨는데, 이 정도면 충분히 잘 연습이 되셨다고 생각합니다!!

🤔 이런 점은 더 생각해볼까요?

  • 테스트도 얼른 보고 싶습니다!
  • Modal의 Storybook은 Docs 페이지에서는 너무 높이가 낮아서 modal이 뜨는지 파악하기가 어려운데요, 스토리북의 문서를 찾아보면 이 높이를 고정하거나 조정할 수 있는 방법이 있을 거에요!
  • 훅에서 어떤걸 관리하고 반환을 해야할지, 사용자가 모르게 해야할 로직과 커스텀하게 해주어야 할 로직은 어떤 것일지 잘 정리해보면 좋을 것 같아요.
  • 자바스크립트, 리액트에 대해서 익숙하지 않다고 하셨는데요, 가장 중요한 것은, 이 언어/프레임워크가 지금 해결해야하는 과제에 어떻게 활용할 수 있는가에 대해서 고민하고 탐구하는 것이라고 생각합니다! npm 배포를 위한 디렉토리 구조도, RTL과 Jest도 마찬가지에요. 필요한 정보는 공식문서와 블로그를 통해서 이미 자세히 공유되고 있고, 그게 개발자에게는 아주 소중한 자산이라고 봅니다! 이 지식을 쌓아가는 방법과 과정도 나중을 위해서 아주 중요하구요.

나머지 세세한 피드백은 코멘트로 남겼습니다! 확인하고 다시 리뷰 요청 주세요!

import { ButtonWrapper } from "./CloseButton.style";

interface CloseButtonProps {
children: React.ReactNode | string;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReactNode 타입은 어떤 타입인가요? ReactNode도 여러 타입의 Union인데 어떤 타입들이 포함되는지 알고 계신가요??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReactNode 타입은 React에서 렌더링 가능한 모든 타입을 포함하는 Union 타입입니다.

  • ReactElement: JSX 문법으로 작성된 React 요소를 나타냅니다.
  • string | number: 문자열이나 숫자 타입을 나타냅니다.
  • ReactFragment: React.Fragment 컴포넌트를 나타냅니다.
  • ReactPortal: ReactDOM.createPortal()을 통해 생성된 Portal 요소를 나타냅니다.
  • null | undefined | boolean: null, undefined, boolean 값을 나타냅니다. 이들은 실제로 렌더링되지 않습니다.
  • ReactNodeArray: ReactNode의 배열을 나타냅니다.

ReactNode 타입을 찾아보니, 타입을 더 명확하게 특정하는 게 좋을 것 같다고 생각이 들었습니다. 저는 CloseButton이 이미지나 텍스트가 오기를 기대하므로 ReactElement가 적절할 것 같아서 수정했습니다!

Comment on lines 25 to 27
`
justify-content: space-between;
`}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

단순한 궁금증인데 여기서는 왜 일반 따옴표/큰 따옴표를 사용하지 않고 백틱을 사용해서 문자열을 나타내주셨나요?
조금 어색하게 느껴지긴 하네요!

다른 곳을 보니 아마 줄바꿈 때문에 이렇게 하신거 같은데 요기는 한 줄이라서 조금 더 어색하게 느껴지나보네요

Comment on lines 38 to 54
${({ modalPosition }) =>
modalPosition === "center" &&
`
top: 50%;
left: 50%;
transform: translate(-50%, -50%);
width: 304px;
`}

${({ modalPosition }) =>
modalPosition === "bottom" &&
`
bottom: 0;
left:0;
right:0;
width: 100%;
`}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 똑같은 modalPosition 변수를 사용한다면 둘로 나눠서 사용하지 않고 하나로 사용할 수도 있을거 같아요!

if (!isOpen) return null;

return (
<ModalDeem isOpen={isOpen} onClick={(e) => onClose(e)}>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

뒷 배경에 어둡게 깔리는 부분에 대한 컴포넌트 명칭일까요? 사소한 부분이지만 영어로는 Dim 단어가 맞을거 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그렇군요! 추가적으로 찾아보니 Dim, Dimmed, Scrim, BackDrop 등 여러가지 명칭이 있는 것 같은데 보통 어떤 단어를 많이 사용하나요?
이렇게 어떤 단어가 통상적으로 사용되는지 모를 때는 어떻게 정보를 찾으면 될까요?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

모두 다 많이 사용하는 것 같아요! 제가 가장 많이 본 두 가지는 Dimmed랑 Backdrop입니다.
네이밍은 원래 어렵지만 지금처럼 용어가 어떤게 더 많이 쓰이는지가 궁금하면 grep.app이 수십만 깃 레포에 걸쳐서 검색을 할 수 있게 해주는데, 여기서 각각의 용어들의 사용 빈도를 비교하는 것도 방법이 됩니다! 혹은 유명한 라이브러리의 용어를 차용해도 되고요!
아래는 grep.app에서 js/tsx로 필터링하고 dimmed와 backdrop을 검색해본 결과 수 차이입니다!
image
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grep 이용하겠습니다 ㅎㅎ

if (!isOpen) return null;

return (
<ModalDeem isOpen={isOpen} onClick={(e) => onClose(e)}>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<ModalDeem isOpen={isOpen} onClick={(e) => onClose(e)}>
<ModalDeem isOpen={isOpen} onClick={onClose}>

로 줄여도 될거 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

불필요한 함수 래퍼를 생성하지 않는 게 좋겠군요!


### ModalComponent

`ModalComponent`는 다음 props를 받습니다:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return > prop 순으로 문서를 구성하신 이유가 있나요? input > output의 개념이라면 prop > return 이 더 자연스럽지 않을까 하는 생각이 들어서요!

Copy link
Member Author

@00kang 00kang May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

input ouput 개념으로 생각해보니 이렇게 문서를 구성하는 게 더 자연스러울 것 같네요. 이건 생각하지 못한 부분이라 짚어주셔서 감사합니다.
추가로 라이브러리 문서화에서 꼭 챙겨야 할 포인트 혹은 고려할 점이 있을까요??

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문서화는 지금 정도만 해도 충분하다고 생각합니다! 보통 널리 쓰이는 라이브러리들은 이 라이브러리가 어떤 목표를 가지고 만들어졌고, 어떤 것을 중점적으로 다루고 있는지 정도도 추가해주는 것 같습니다! '가볍다'던지 '쓰기 쉽다'던지 컨셉 느낌으로요!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 라이브러리의 목적을 나타내면 사용자에게 더 좋은 경험을 제공할 수 있겠군요! 추가해보도록 하겠습니다~

Comment on lines 22 to 28
const cardTypes = [
{ name: "현대카드" },
{ name: "국민카드" },
{ name: "신한카드" },
{ name: "우리카드" },
];

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런 정보들은 상수로 따로 분리해도 좋을 것 같아요!
또 name 하나의 속성만 가지는 객체의 배열이라면 차라리 cardNames로 해서 문자열의 배열로 만드는 편이 더 좋지 않을까요?

Copy link
Member Author

@00kang 00kang May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리팩터링하다가 코드가 간결해졌는데 문자열 배열로 바꾼다는 걸 놓쳤네요. 문자열 배열로 수정했습니다!

return (
<>
<h1>Hooks Modules</h1>
{/* 카드 소유자 이름 입력 */}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 주석들은 불필요할 것 같아요!

Copy link
Member Author

@00kang 00kang May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 커스텀 훅을 하나씩 만들 때마다 App.tsx에서 직접 사용해보았기 때문에 주석을 표현해서 구분했는데요.
주석이 없더라도 그 영역들을 구분할 수 있는 것 같아 삭제했습니다!
그렇다면 코드 내에서 주석은 언제, 어떻게 활용하는게 좋을까요?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주석은 코드를 아무리 가독성 좋게, 명시적으로 짠다고 하더라도 파악하기 힘든 것들에 대해서 기록을 남길 때 필요하다고 생각합니다.
제 개인적인 경험으로는 다음과 같은 상황들에는 주석이 유용했다고 생각합니다.

  • 코드가 복잡해서 한 위치에서의 코드가 전혀 다른 곳의 코드에 영향을 미칠수 밖에 없을 때, 그래서 이 파일에서는 지우거나 수정해야할 것처럼 보이지만 수정하면 오류가 발생할 때 다른 작업자들에게 경고를 남기기 위해
  • 업무적으로 복잡한 이유로 인해서 (백엔드 개발자나 기획자의 특별한 요구, 지금 절대 고칠 수 없는 레거시로 인해서 등등) 겉보기엔 어색하지만 현재는 꼭 필요한 로직이 들어가야 할 때
  • 미래에는 어떤 모습이어야 하지만, 아직 구현 이전이라 TO DO 항목을 코드에 남길 때

물론 개발 시 사용하기 편하라고 변수/함수/타입에 호버 시에 설명이 뜨는 주석을 달아주는 jsdoc/tsdoc도 존재하지만 이것은 또 완전 별개의 영역이라고 봅니다!

@@ -0,0 +1,25 @@
export const ERROR_MESSAGE = {
NO_INPUT: "인풋은 비어있을 수 없습니다.",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인풋이라는 말이 그대로 사용자에게 노출될텐데 괜찮을까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NO_INPUT: "입력필드는 비어있을 수 없습니다.",
인풋이라는 말이 사용자에게 친절하지 않을 수도 있겠다는 생각에 위와 같이 수정했습니다!

setPassword(value);
}

return [password, handlePasswordChange, validatePassword(password)];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 훅들의 값은 배열로 반환하기로 한 이유가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

객체로 할 수 있지만, 저의 경우에는 다음과 같은 이유로 배열로 반환하는 것도 괜찮다고 생각했습니다!
블링은 객체로 반환하는 게 더 적합하다고 보신걸까요?

  • React의 내장 훅(useState, useEffect 등)들도 배열 형태로 값을 반환합니다. 따라서 배열 형태를 유지하는 것이 React의 관례를 따르는 셈이 됩니다.
  • 반환 값의 수가 많지 않기 때문에, 배열의 인덱스를 통해 값에 접근하는 것이 코드의 가독성을 크게 해치지는 않을 것입니다.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 이펙트 훅은 아무것도 반환하지 않습니다!
  2. 배열로 반환하는 것과 객체로 반환하는 것의 가장 큰 차이는 반환하는 값을 사용하는 방식에 있습니다.
    배열은 반환 순서를 기억해서 변수를 사용해야하는 단점이 있는 반면, 그 변수의 이름을 자유롭게 선언할 수 있다는 장점이 있습니다. 그래서 state 훅 처럼 반환 값이 항상 두 개지만, 그 활용성은 무궁무진해서 상태의 이름은 매번 다르게 활용해야 할 때에 적합하겠죠?
    그리고 객체로 반환하는 경우, 반환되는 변수가 모두 객체의 속성이기 때문에, 순서에도 구애받지 않고, 사용할 변수만 선언해서 사용해도 된다는 장점이 있습니다. 다만 이름을 바꾸려면 별도의 변수에 담아주거나 식별자를 별도로 선언해주어야 한다는 불편한 점이 있죠! 그래서 객체로 반환하는 경우에는 반환할 값이 아주 많아서 순서를 기억하기 어렵다거나, 활용처는 많지만 활용처마다 사용하지 않는 변수도 있을 때에는 더 유용하게 사용됩니다.
  3. 저는 개인적으로 커스텀 훅이 반환하는 값이 3개 이상이라면 (두 개보다 많다면) 순서를 기억해서 사용처에서 사용하는 것보다, 객체로 반환하는게 더 편리했었다고 생각합니다. 선택은 초코의 몫으로 남겨둘게요!!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

객체 형식으로 수정했습니다 :~)

00kang added 25 commits May 3, 2024 19:37
@00kang
Copy link
Member Author

00kang commented May 7, 2024

블링~ 초코입니다.
조금 늦게 리뷰요청드려 죄송합니다. ㅠ
확인하시고 수정이 필요한 부분 혹은 추후 학습하고 개선할 수 있는 부분이 있다면 짚어주시면 감사하겠습니다!

🔗 배포 링크

라이브러리 업데이트가 필요한 것 같아 기존에 페어와 함께 진행하던 choriver-~~ 라이브러리에서 각자의 라이브러리로 분리하였습니다. 업데이트된 개인 라이브러리 링크 연결해놓겠습니다!


✅ 반영 사항별 커밋 바로가기


❓ 질문사항

  • 이전에 달린 피드백에 이어서 몇가지 질문 남겼습니다! 확인해주시고 답변 부탁드립니다.

  • 이번 2단계 미션이 오늘 공개되면서 추가 질문할 게 생겼습니다. 카드번호를 16자리가 아닌 14~16자리의 숫자를 인풋창 하나에서 받고, 이를 각 브랜드사로 식별하고, 카드 프리뷰 이미지에는 4*4 형식이 아니라 각 자리수마다 다른 포맷팅을 해줘야 합니다. 이렇게 되면서 어떠한 순서로 구현을 해야 하나 고민이 생겼습니다. 일단 제가 생각하는 구현 설계는 다음과 같습니다.

    • 카드번호 인풋을 1개로 합치기
    • 카드번호 중 일부를 확인하여 카드사 식별하기
    • 전체 숫자를 다 입력받은 뒤 포맷팅 하기

그런데 이때 포맷팅을 어떻게 해줘야 할까요? 몇자리의 숫자가 들어올지 예상할 수 없을 텐데 자릿수에 따라서 포맷팅을 해줘야 할때는 어떻게 접근해야 구현방식을 생각해볼 수 있을지 힌트를 얻고 싶습니다!

Copy link

@uk960214 uk960214 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

피드백 반영하느라 수고 많으셨습니다!
반영하면서 고민도 많이 하시고 연구도 많이 하신 것 같아서 좋았습니다!
그 결과가 2단계에는 더 많이 드러났으면 좋겠네요!

시간 상 이번 단계는 여기서 머지하지만 남긴 코멘트들은 꼭 확인 부탁드립니다!
2단계는 리팩토링이 조금 미흡하더라도 요청을 조금 빨리 주시면 함께 보고 수정해나가 보시죠 😄

다음 단계에 대한 고민은 제가 직접적으로 도움을 드리는 것은 좀 어렵겠지만
길이에 따라서 유동적으로 그 때마다 해주면 되지 않을까요?
초반에는 네자리까지 분리하다가 특정 자리수를 만족하면 그에 따라서 포맷팅 해주는대로요!
분명 잘 해내실 수 있을 겁니다!

2단계도 화이팅입니다!! 💪


### ModalComponent

`ModalComponent`는 다음 props를 받습니다:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문서화는 지금 정도만 해도 충분하다고 생각합니다! 보통 널리 쓰이는 라이브러리들은 이 라이브러리가 어떤 목표를 가지고 만들어졌고, 어떤 것을 중점적으로 다루고 있는지 정도도 추가해주는 것 같습니다! '가볍다'던지 '쓰기 쉽다'던지 컨셉 느낌으로요!

import { ButtonWrapper } from "./CloseButton.style";

interface CloseButtonProps {
children: React.ReactElement | string;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReactElement 타입이 이미 string을 포함하고 있을 것 같은데 string은 생략해도 좋지 않을까요?

<h1>{title}</h1>
{closeButtonPosition === "top" && (
<CloseButton onClick={(e) => onClose(e)}>
<img src="/public/image/closeButton.png" />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 아직 해소가 되지 않은 것 같네요!

Comment on lines +20 to +21
"choco-payments-validation-hooks": "^0.0.1",
"choriver-payments-validation-hooks": "^0.0.1",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이것들은 dependencies에 필요하지 않은 것들이겠죠?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 직접 App.tsx에서 라이브러리를 설치해서 사용해보느라고 남은 것 같습니다!

if (!isOpen) return null;

return (
<ModalDeem isOpen={isOpen} onClick={(e) => onClose(e)}>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

모두 다 많이 사용하는 것 같아요! 제가 가장 많이 본 두 가지는 Dimmed랑 Backdrop입니다.
네이밍은 원래 어렵지만 지금처럼 용어가 어떤게 더 많이 쓰이는지가 궁금하면 grep.app이 수십만 깃 레포에 걸쳐서 검색을 할 수 있게 해주는데, 여기서 각각의 용어들의 사용 빈도를 비교하는 것도 방법이 됩니다! 혹은 유명한 라이브러리의 용어를 차용해도 되고요!
아래는 grep.app에서 js/tsx로 필터링하고 dimmed와 backdrop을 검색해본 결과 수 차이입니다!
image
image

Comment on lines 10 to 16
const Button: React.FC<ButtonProps> = ({ content, backgroundColor, fontColor, onClick }) => {
return (
<ButtonWrapper backgroundColor={backgroundColor} fontColor={fontColor} onClick={onClick}>
{content}
</ButtonWrapper>
);
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 지금 상황에서 이 코드를 읽는 사람이 꼭 해당 정보를 하위 컴포넌트로 전달하는 것을 알지 않아도 된다고 생각해서 그렇습니다! 만약에 prop으로 어떤 것들이 전달되는 지를 자세히 알고 싶다면 ButtonProp 타입을 참조하면 될테니까요! 보통은 Button 같이 범용적인 컴포넌트들은 prop이 갈수록 많아질텐데요, 아마도 나중에는 가독성 측면에서 훨씬 더 낫지 않을까 생각합니다!

return (
<>
<h1>Hooks Modules</h1>
{/* 카드 소유자 이름 입력 */}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주석은 코드를 아무리 가독성 좋게, 명시적으로 짠다고 하더라도 파악하기 힘든 것들에 대해서 기록을 남길 때 필요하다고 생각합니다.
제 개인적인 경험으로는 다음과 같은 상황들에는 주석이 유용했다고 생각합니다.

  • 코드가 복잡해서 한 위치에서의 코드가 전혀 다른 곳의 코드에 영향을 미칠수 밖에 없을 때, 그래서 이 파일에서는 지우거나 수정해야할 것처럼 보이지만 수정하면 오류가 발생할 때 다른 작업자들에게 경고를 남기기 위해
  • 업무적으로 복잡한 이유로 인해서 (백엔드 개발자나 기획자의 특별한 요구, 지금 절대 고칠 수 없는 레거시로 인해서 등등) 겉보기엔 어색하지만 현재는 꼭 필요한 로직이 들어가야 할 때
  • 미래에는 어떤 모습이어야 하지만, 아직 구현 이전이라 TO DO 항목을 코드에 남길 때

물론 개발 시 사용하기 편하라고 변수/함수/타입에 호버 시에 설명이 뜨는 주석을 달아주는 jsdoc/tsdoc도 존재하지만 이것은 또 완전 별개의 영역이라고 봅니다!

setPassword(value);
}

return [password, handlePasswordChange, validatePassword(password)];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 이펙트 훅은 아무것도 반환하지 않습니다!
  2. 배열로 반환하는 것과 객체로 반환하는 것의 가장 큰 차이는 반환하는 값을 사용하는 방식에 있습니다.
    배열은 반환 순서를 기억해서 변수를 사용해야하는 단점이 있는 반면, 그 변수의 이름을 자유롭게 선언할 수 있다는 장점이 있습니다. 그래서 state 훅 처럼 반환 값이 항상 두 개지만, 그 활용성은 무궁무진해서 상태의 이름은 매번 다르게 활용해야 할 때에 적합하겠죠?
    그리고 객체로 반환하는 경우, 반환되는 변수가 모두 객체의 속성이기 때문에, 순서에도 구애받지 않고, 사용할 변수만 선언해서 사용해도 된다는 장점이 있습니다. 다만 이름을 바꾸려면 별도의 변수에 담아주거나 식별자를 별도로 선언해주어야 한다는 불편한 점이 있죠! 그래서 객체로 반환하는 경우에는 반환할 값이 아주 많아서 순서를 기억하기 어렵다거나, 활용처는 많지만 활용처마다 사용하지 않는 변수도 있을 때에는 더 유용하게 사용됩니다.
  3. 저는 개인적으로 커스텀 훅이 반환하는 값이 3개 이상이라면 (두 개보다 많다면) 순서를 기억해서 사용처에서 사용하는 것보다, 객체로 반환하는게 더 편리했었다고 생각합니다. 선택은 초코의 몫으로 남겨둘게요!!

@uk960214 uk960214 merged commit a20f3a5 into woowacourse:00kang May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants