인상적인 코드리뷰 모음 #2 (~23년 4월)

최원빈·2023년 4월 17일
3

개강을 하고 난 뒤에도, 조금은 바쁘지만 개발을 쉬지 않고 있다.
동아리 교육도 하고있는터라 뭔가 하고있는 건 많지만 개강 이전처럼 진득하게 블로깅을 작성하기는 쉽지 않더라.

그래서 이번주도 인상적인 코드리뷰를 정리해보려고 한다.

새로운 팀원들도 생겼기에, 처음 적응을 잘 할 수 있도록 기초적인 부분부터 집어서 살펴주었다.


# Figma로 받아오는 CSS값들은..

우리 동아리의 UI/UX 팀은 디자인 툴로 Figma 를 사용한다.
Figma는 해당 요소에 적용된 디자인 값을 CSS로 변환하여 볼 수 있게끔 도와주는데, 알아서 변환해주는 부분이다보니 실제 개발에는 사용하지 못하는 값들도 생긴다.

position, width, top.. 등등은 사실상 프레임에서 절대적인 위치를 잡기 위함이니 사용해선 안되고,
font관련 속성등 또한 기본 폰트를 따라가는게 대부분이기에 font-size 정도만 적용하게 된다.
flexbox 를 통해 center등의 배치를 사용하다 보면, line-height 이 정렬을 깨는 상황도 흔하게 볼 수 있다.

리뷰에 작성한 border-radius 도 간단하게 50%만 줘도 끝이 완전하게 둥글게 보이니 알아두면 간결하게 작성하기 좋다.


# Modal 컴포넌트의 관심사는?

모달 컴포넌트는 사실 특정 상태(isOpen)가 true일 때 보이게 될테니, 모달 내에서는 isOpenfalse 로 바꾸는, 쉽게 모달창이 닫히게 외부의 상태를 false로 바꾸는 함수만 있으면 된다.

가독성을 위해 컴포넌트 내부에서 closeModal 이라는 이름의 함수를 정의해 사용한 것 같은데, 사실 props로 닫는 함수를 넘겨주면 더 간결하게 작성할 수 있을것이라 생각했다.

해당 상태 또한 boolean 상태의 핸들링을 캡슐화한 훅을 사용하면 더욱 간결하게 사용할 수 있을 것 같았다.

그렇지만 props를 setFalse 라는 외부 상태에 의존적인 변수명을 유지할 필요는 없다.
해당 컴포넌트에서 필요로 하는 것은 Modal을 닫는 함수지, 상위 컴포넌트의 boolean형 상태를 false로 바꾸는 상태가 아니기 때문에 이를 구분지으면 좋겠다고 생각했다.


# 서드 파티 라이브러리는..

동아리 프론트엔드 파트에서 진행하고 있는 프로젝트 중에는, 잘 사용중이고 앞으로도 꾸준히 유지보수할 프로젝트의 완전한 리코드 작업을 진행중이다.

React를 처음 다뤄볼 때 만들었던 프로젝트인지라, 고려해야할 점들을 많이 놓친데다가 이것저것 긁어온 부분들도 많다보니 유지보수가 너무 힘들어 최대한 앞으로의 상황을 고려해 오래 유지할 수 있는 소프트웨어를 만들어보자는 의도로 새로 만들고 있었다.

프로젝트에서는 네이버맵을 사용중이었고, npm에는 React에서 사용하기 쉽게끔 기능들을 모듈화한 react-naver-maps 라는 라이브러리가 있어 활용했지만, 위 의도와는 맞지 않았다.

물론 서드 파티 라이브러리를 사용하면 개발 속도는 줄겠지만,
naver-map, react-naver-map 두가지 의존성을 동시에 관리해야 하고 원하는 기능이 라이브러리에 포함되지 않았다면 직접 구현에 어려움이 발생하므로 경험도 늘릴 겸 서드 파티는 제거하는 방향으로 진행하게 되었다.


# 추론된 타입을 따라가야 하는가?

useParams 에서 반환하는 파라미터는 string | undefined 타입을 가진다.
해당 변수를 그대로 사용하다가 타입을 끼워맞추게 된 것 같은데, 훅 입장에서는 id 값이 undefined 가 가능한지 여부를 신경쓸 필요는 없으므로, 해당 관심사에 대한 분리를 확실히 해야한다고 생각했다.

생각해보면 useParams 의 타입이 참 불편하다고 생각이 드는데, type safety한 url parameter를 제공하는 라이브러리(Tanstack Router 등..)도 몇개 본 게 있어서 추후 프로젝트에는 고려해보는것도 좋겠다는 생각이 들었다.


# 통일된 Suspense?

<React.Suspense /> 는 suspense를 지원하는 데이터가 로딩중인 경우 fallback 에 작성한 컴포넌트를 대신 보여준다.
이를 위해 만들어둔 로딩 스피너가 있다보니 똑같이 활용하는게 어떨까 했는데, 데이터가 반드시 꽉 찬 테이블 구조다보니 스켈레톤이 더 어울리는 UI라 통일하지 않았다고 했다.

코인에서 제공하는 시간표 기능. 시간표 테이블은 꽉 차기에 스켈레톤을 보여주는게 적절하다.

코드의 통일, 반복의 제거만을 신경쓰기보다는 사용자에게 어떻게하면 더욱 적절한 경험을 제공할 수 있을지 생각하는게 중요하구나 생각했다.


# dangerouslySetInnerHTML을 써야만 하는가?

UI를 만들다보면 언제나 같은 위치에 같은 텍스트가 들어갈 수 없는 꼴이다.

이번에 개발하는 헤딩 텍스트가 딱 그런 꼴이었는데, 어떤 음식, 기름진 음식 부분을 강조하고 나머지 부분은 일반적인 텍스트가 나와야 했다.

처음에 했던 생각은 아래처럼 JSX 표현식을 온전한 텍스트로 적어 사용하는 방법이 떠올랐고, 추천해줬다.

const RECOMMEND_TEXT = [
  '<h1>오늘은<em>어떤 음식</em>이 땡기나요?</h1>',
  '<h1><em>기름진 음식</em>이 그리운 날!</h1>'
]

React는 DOM의 innerHTML 과 유사한 동작을 하는 dangerouslySetInnerHTML 을 제공한다.
문제가 있다면...

ESLint에서 이를 막았던 것, 이유를 들어보니 내가 무언가 놓친 것이 떠올랐다.

React 공식 문서의 초반에 나오는 부분인데, 생각해보면 .tsx, .jsx 확장자에서는 JSX표현식을 굳이 문자열이 아닌, JSX그 자체로 활용할 수 있었다.

아래 수정한 결과처럼 문구를 작성할 수 있었고, 간단하게 출력할 수 있었다.

  // 값 자체가 JSX 표현식이 된다.
  <h1 className={styles.search__heading}>
    {RECOMMEND_TEXT[randomValue]}
  </h1>

profile
FrontEnd Developer

0개의 댓글