[도리] 1주차 pr요청

[kimyouknow]

안녕하세요 Daisy! 프론트엔드 도리입니다. 1주차 첫 번재 리뷰 요청합니다~

진행상황

1. 자판기 구성

• 상품주문: 상품주문 + 액션로그
• 상품관리: 잔돈관리 + 재고관리

2. 상태관리

• contextapi + useReducer로 페이지 이동과 상관없이 상태유지하는 방법 시도

궁금한점

1. 컴포넌트 상단에 순수함수를 선언

컴포넌트 내부에서 선언한 함수가 길어져서 분리할 때, 컴포넌트 상단에 순수함수로 분리해 컴포넌트가 리렌더링되어도 함수는 그대로 사용할 수 있게 해봤습니다. 모든 함수를 useCallback으로 감싸고 컴포넌트 내부에 선언하는 방법이 가독성이 더 좋을까요?

2. 스타일 컨벤션

styled-componet를 사용할 때 react 컴포넌트와 styled컴포넌트가 구분되지 않아 구분하는 방법으로 다음과 같은 방법을 고려해봤습니다.

import * as S from "./style";

function App(){
  return (
    <S.Container>
      <Child />
    </S.Container>
  )
}

3. 상태관리

이번 미션에서는 context api + useReducer를 합친 context 컴포넌트를 만들어서 상태를 관리하는 방법을 시도하고 있습니다. 이전에 react-redux를 사용해본 적이 있는데 형태가 비슷해보여서 비교해 볼 계획입니다.

[damilog]

어떤 change를 handling 하는 건지 더 구체적으로 네이밍 해보면 좋을 것 같네요~

[damilog]

이 state도 어떤 상태인지 알 수 있도록 구체적으로 네이밍 해보는게 어떨까요?

[damilog]

action도 상수화 해볼 수 있을 것 같아요!

[damilog]

targetCount..?

[damilog]

컴포넌트 정의할 때는 함수 선언문을 사용하시는데, 함수 선언문과 표현식을 사용을 결정하는 도리만의 규칙이 있는지 궁금하네요

[kimyouknow]

컴포넌트는 함수 선언문으로 사용했고, 컴포넌트 내부 함수나 기타 함수들은 표현식으로 사용하고 있습니다. 일관성을 위해 통일하는 것이 더 좋을까요?

(+ 컴포넌트 내부 함수의 경우 useCallback 같은 hooks를 쓸 때, 표현식만 가능하다고 알고 있는 맞을까요? )

[damilog]

숫자를 가져와 천 단위 콤마를 찍어 리턴하는 유틸함수이군요.
값을 리턴하는 함수명에 set이 들어가니 조금 어색하게 느껴지네요!

[damilog]

삼항 연산자 사용해서 한 줄로 작성해도 괜찮을 것 같네요!

[damilog]

잔돈 처리하는 로직에 처리해야할 case가 많아 보이는데요. 이 부분 로직 분리해도 괜찮을 것 같네요.

[damilog]

안녕하세요 도리. 리뷰를 맡은 Daisy입니다~ 2주간 잘 부탁드려요!
Context API 사용하셔서 전역 상태관리를 고민해보셨군요.
이번 미션이 상태 변화가 잦을 것 같은데 구현하신 상태 관리 방법이 많은 도움이 될 것이라 생각되네요.

또 styled components import 할 때 S라는 이름으로 import 해서 쓰는 컨벤션은 저도 전에 사용해본 적이있는데,
일반 컴포넌트랑 스타일드 컴포넌트랑 구분하기 쉬워서 좋더라고요! 좋은 시도라고 생각합니다.

질문에 답변을 드리자면,

1. 컴포넌트 상단에 순수함수를 선언
   컴포넌트 내부에서 선언한 함수가 길어져서 분리할 때, 컴포넌트 상단에 순수함수로 분리해 컴포넌트가 리렌더링되어도 함수는 그대로 사용할 수 있게 해봤습니다. 모든 함수를 useCallback으로 감싸고 컴포넌트 내부에 선언하는 방법이 가독성이 더 좋을까요?

개인적으로 저는 컴포넌트 안에서 사용되는 함수는 다 해당 컴포넌트 안에서 정의하는 걸 선호합니다.
팀 컨벤션에 따라 다르겠지만, 코드 상단에 컴포넌트에 대한 내용이 정의 되어 있어야
해당 파일을 열었을 때 컴포넌트가 사용하는 props, children 등을 한 눈에 확인 할 수 있어서 편하더라고요.

컴포넌트가 리렌더링 될 때 함수가 새로 생성 되어 퍼포먼스 측면에서 좋지 않을 거라는 우려하시는 부분은
저도 고민했던 내용이기도 하기에 공감이 됩니다.

리액트 공식문서에 따르면 useCallback은 이미 React.memo 등으로 최적화된 자식 컴포넌트의 콜백함수로 전달하는 함수에 적용했을 때 유용하다고 하는데요. 저도 이렇게만 알고 useCallback을 쓰면 함수 생성 자체를 막는거라고 생각했었는데, 이 질문을 받고 더 찾아보니

useCallback을 쓴다고 해서 의존성이 바뀌지 않았을 때 무조건 함수가 새로 생성되지 않는게 아니라, 매 렌더마다 함수 생성은 되지만 의존성이 바뀌지 않았다면 새로 생성된 함수를 무시하고 기존 함수를 반환한다고 하네요.

따라서 useCallback 자체가 함수 생성을 줄여주는 효과는 없기에 자식 컴포넌트에 넘겨주는 props가 바뀌지 않게 하는 용도로 사용하는 게 아니라면 큰 효과를 기대하기는 어려울 것 같아요. 질문 덕분에 저도 많이 배웠습니다. 혹 제 답변에 잘못 전달된 내용이 있다면 코멘트 부탁드려요ㅎㅎ

이를 참고하셔서 컴포넌트 내부에 useCallback 으로 함수를 wrapping 하고자 하실 때는 해당 함수가 단순히 컴포넌트 안에서 호출되는 함수인지, 아님 최적화된 자식 컴포넌트의 props로 넘겨지는 함수인지를 확인하시고 사용하시면 될 것 같습니다.

[damilog]

단순히 case 따라 color값을 리턴하는 거라면 함수가 아니라 객체로 정의해도 괜찮을 것 같은데 어떠신가요?

[damilog]

theme 사용 너무 좋네요 👍

[damilog]

brace 없이 바로 return 하도록 해도 되겠네요!

[kimyouknow]

@damilog 자세한 답변 감사해요😄. useCallback이 의존성이 바뀌지 않으면 무조건 함수가 새로 생성되지 않는다고 알고 있었는데 설명해주신 내용과 함께 걸어주신 링크를 참고해서 다시 찾아봐야겠네요!

그 외에 코멘트 남겨주신 부분들도 고려해서 다음 pr에 반영해보겠습니다.