2주간의 토이프로젝트가 끝나고, 코치님의 마지막 코드리뷰까지 받았습니다.
많은일들이 있었고, 많은 도움을 받아서 너무 감사했습니다. 피드백 받은 부분 수정하면서 또한번 부족한점을 많이 느꼈고, 더욱더 잘하고 싶다 라는 생각이 들었습니다. 지금부터 제가 코드리뷰 받은부분을 기록함으로써 오래오래 기억하기위하여 글을 적어 나가려 합니다.

코드리뷰 내용들

1. 불필요한 렌더링은 피하도록 코드를 작성하자!

코드리뷰내용:
Cardswiper.js에서 스와이퍼 라이브러리를 잘 사용해주셨는데요, 다만 SwiperCore.use([Navigation]); 코드의 위치는 반드시! 컴포넌트 바깥에 위치해야 합니다. Cardswiper 내부에서 저 함수가 실행이 된다는 뜻은 컴포넌트가 렌더링이 될 때마다 매번 저 코드가 실행이 된다는 뜻이거든요. 꼭 수정해주세요!

import { EffectCards } from "swiper";
import SwiperCore, { Navigation } from "swiper";

import axios from "axios";
//수정내용
SwiperCore.use([Navigation]);

const Cardswiper = () => {
  const [card, setCard] = useState([]);

  const getCardData = async () => {
    try {
      const response = await axios.get("/CardData.json");
      const { card } = await response.data;
      setCard(() => card);
    } catch (error) {
      console.log(error);
    }
  };
  useEffect(() => {
    getCardData();
  }, []);

  return (
    <CardSwiperContainer>
      <StyledSwiper effect={"cards"} grabCursor={true} modules={[EffectCards]}>
        {card.map((item) => {
          return (
            <CardDiv>
              <SwiperSlide
                key={item.id}
                style={{
                  backgroundImage: `url(${item.src})`,
                  backgroundSize: "cover",
                  borderRadius: "18px",
                  backgroundPosition: "center center",
                }}
              >
                <CardLetterBox>
                  <SwiperTitle>{item.name}</SwiperTitle>
                  <SwiperDesc>{item.desc}</SwiperDesc>
                  <SwiperDesc>{item.desc1}</SwiperDesc>
                  <SwiperDesc>{item.desc2}</SwiperDesc>
                </CardLetterBox>
              </SwiperSlide>
            </CardDiv>
          );
        })}
      </StyledSwiper>
    </CardSwiperContainer>
  );
};

위는 수정후 코드입니다. SwiperCore.use([Navigation]);는 컴포넌트 내부가 아닌 외부에 정의를 내려야 됩니다. 코치님 말씀대로 컴포넌트 내부에 정의를 내릴 경우 변화가 일어날때마다 매번 렌더링을 일으키는데요. 이번 피드백 덕분에 코드를 작성할때 불필요한 렌더링을 줄이는것을 생각하면서 작성해야겠다고 다짐했습니다.

2. 명심하자. Setstate는 비동기적으로 작동한다.

코드리뷰내용:
Itempage.js에서 다음과 같은 코드들이 있는데요,

  const handlePlay = () => {
    setPlay(!play);
  };
  const handleNextPlay = () => {
    setStart(start + 10);
    setEnd(end + 10);
  };
  const handlePrevPlay = () => {
    setStart(start - 10);
    setEnd(end - 10);
  };

setState하는 함수에는 이전 상태를 기반으로 새로운 상태를 세팅하도록 인자에 콜백을 넣어주는 것이 좋습니다. 그러니까 setPlay(!play)가 아니라 setPlay(play => !play)와 같이 이전 상태를 인자로 받는 콜백을 setState 함수에게 인자로 주라는 뜻이에요.
그 이유는 setState가 비동기적으로 작동해서 상태값을 batch 처리(일정 시간 동안 변화하는 상태를 일괄 처리)하기 때문입니다. 리액트는 생각보다 똑똑해서... setState가 한번씩 일어날 때마다 컴포넌트를 렌더링하기 보다, 어느 정도 setState 액션을 모아서 한꺼번에 일괄 처리하는데요, 그러한 상황에서 의도하지 않은 버그를 발생시키지 않기 위해 setState 함수에는 이전의 상태를 기반으로 업데이트 하도록 콜백을 인자로 넣어주셔야 합니다.

코치님의 피드백으로 setstate사용한부분 전부 콜백으로 수정하였습니다. 코드리뷰의 내용이 대부분 렌더링 관련 내용이네요.. setstate가 비동기적으로 사용된다는점은 이번에 처음 알게 되었는데, 이렇게나 모르는 점이 많았다니 반성을 많이 하게 되었습니다. 기억해요 우리!! setState는 비동기적으로 작동한다는점을!!

3. styled components를 효율적으로 사용하자!

코드리뷰내용:
styled components를 더 효율적으로 사용해보면 좋을 것 같아요. SoundBar.js 파일에서

  & > *:nth-child(1) {
    animation-delay: 0.2s;
  }
  & > *:nth-child(2) {
    animation-delay: 0.3s;
  }
  & > *:nth-child(3) {
    animation-delay: 0.4s;
  }
  & > *:nth-child(4) {
    animation-delay: 0.5s;
  }
  & > *:nth-child(5) {
    animation-delay: 0.8s;
  }

이런 코드가 있는데요, 사실 nth-child라는 selector가 유용하기는 하지만... css의 유지보수 차원에서는 굉장히 어려운 코드거든요.
그래서 저렇게 컨트롤하기보단, Line이라는 스타일드 컴포넌트에 props로 index를 전달하고 그 index에 따라 animation-delay를 계산해서 사용하도록 하는 게 어떨지 제안드리고 싶습니다.

아래는 수정내용입니다.

  return (
    <>
      <Box id={window.innerWidth} onClick={() => handleClick()}>
        {[1, 2, 3, 4, 5].map((idx) => (
          <Line key={idx} click={click} delay={idx * 0.1 + 0.1} />
        ))}
        <audio src='good.mp3' ref={ref} autoPlay />
      </Box>
    </>
  );
};

const Line = styled.span`
  border: 1px solid ${(props) => props.theme.body};

  animation: ${play} 1s ease infinite;
  animation-play-state: ${(props) => (props.click ? 'running' : 'paused')};
  animation-delay: ${({ delay }) => `${delay}s`};
  height: 2rem;
  width: 3px;
  background-color: black;
  margin: 0 0.1rem;
`;

기존에는 "Line" 태그를 하드코딩으로 5줄 작성후 css 작업으로 nth-child를 5개 작성하여 작동하게끔 하였습니다. 당연히 위와같이 수정함으로써 값이 변해도 수정하기 좋고, 코드도 짧아지니 일석이조가 아닐수 없습니다! 항상 어떻게해야 좀더 효율적으로, 유지보수가 좋게끔 작성해야되는지 생각 또 생각하고 코드를 작성하겠다 다짐했습니다.

4. setTimeout함수는 clearTimeout을 꼭해주자!

코드리뷰내용:
Main.js에서 setTimeout 함수를 이용해서 잘 구현해 주셨는데요, 더 올바른 사용법은 컴포넌트 언마운트 시점에 타이머를 클리어하도록 클리어 펑션을 추가해 주는 겁니다. 클리어하지 않으면 컴포넌트에 memory leak이 발생할 수 있습니다. 여기를 참고해서 리팩터링하면 좋을 것 같아요 :)

useEffect(() => {
const timer = setTimeout(() => console.log('Initial timeout!'), 1000);
return () => clearTimeout(timer); // 타이머 클리어
}, []);

clearTimeout을 사용하는 이유
컴포넌트가 언마운트될 때 타이머를 클리어하지 않으면, 컴포넌트가 보이지 않는 상태임에도 불구하고 콜백함수가 작동할 지 몰라요.
이는 어플리케이션에서의 메모리 부족 현상으로 이어질 수 있습니다. 컴포넌트가 언마운트 된 이후에도 타이머는 활성화되어 있기 때문에, 가비지 콜렉터가 컴포넌트를 수집하지 않을 겁니다.

5. useMemo, useCallback을 사용하여 불필요한 렌더링을 막자!

코드리뷰내용:
추가적으로 useMemo, useCallback을 보다 적극적으로 사용해 불필요한 렌더링을 막아보면 어떨까요? 두 개의 차이점은 무엇이고, 어떤 상황에서 사용하면 좋을지 고민하여 리팩터링하실 때 보완하시면 좋을 것 같습니다.

불필요한 렌더링을 피해야되는 이유:
성능상으로도 떨어지게 되고 메모리누수및 사이드이펙트가 발생할 가능성이 있어 피해야된다고 생각합니다. 혹시라도 내용이 틀리거나, 추가할 내용이 있으면 댓글 부탁드립니다♥

Memoization 이란?

메모이제이션은 기존에 수행한 연산의 결과값을 어딘가에 저장해두고 동일한 입력이 들어오면 재활용하는 프로그래밍 기법입니다. 이것을 적절하게 활용하면 중복 연산을 피할 수 있기 때문에 메모리를 조금 더 쓰더라도 애플리케이션의 성능을 최적화 할 수 있습니다.

useMemo란?

사용법은 useEffect와 동일하다.

useMemo(() => fn, [deps])

여기서 deps로 지정한 값이 변하게 된다면 () => fn 함수를 실행하고, 그 함수의 반환 값을 반환해줍니다.

deps는 dependency의 약어로, 의존성을 뜻하며 useMemo가 이 deps에 의존하고 있다는 것을 말합니다.

리액트의 state는 값이 변할때마다 리렌더링이 발생합니다. 따라서 a라는 state가 변경되지 않았는데, b라는 부모컴포넌트에 있는 state의 상태 변화로 인하여 a라는 state도 계속 렌더링이 된다면 이는 성능상 매우 좋지 않습니다. 따라서 useMemo로 등록된 state는 해당하는 state가 변화할때만
연산을 실행할 수 있도록 useMemo를 사용해 ex라는 변수에 의존하도록 등록하는 것입니다.

useCallback이란?

useMemo는 메모이제이션된 값을 반환 했다면, useCallback은 메모이제이션된 함수를 반환합니다.

useCallback(fn, [deps])

useCallback 또한 deps, 의존성이 있는 값이 변하면 fn에 등록한 함수를 반환하는 기능을 가지고 있습니다.

const memoizedCallback = useCallback(() => console.log(), [test])

useCallback은 함수를 반환하기 때문에, 그 함수를 가지는 const 변수에 초기화하는 것이 일반적인 모양이라 합니다.

그렇다면 useCallback은 어떨때 사용할까요?

1) 자식 컴포넌트에 props로 함수를 전달할 경우

먼저 함수는 값이 아닌 참조로 비교된다!는 점을 알고 있어야합니다.

const functionOne = function() {
  return 10;
};
const functionTwo = function() {
  return 10;
};
// 서로의 참조가 다르기 때문에 false
console.log(functionOne === functionTwo);

동일한 값을 반환하지만 참조가 다르기 때문에 false가 됩니다. 이러한 고유함수가 생성될때 부모컴포넌트에서 props를 전달받는 자식컴포넌트는 props가 변경되었다고 생각해 리렌더링이 발생하게 됩니다. 부모 컴포넌트만 수정하려고 했지만 연쇄적으로 하위 컴포넌트들 모두 렌더링이 일어나게 되어버리지요. useCallback을 사용하지 않을 경우, 이러한 불필요한 리렌더링이 발생하기 때문에 사용하는것이 좋습니다.

2) 외부에서 값을 가져오는 api를 호출하는 경우

   const [user, setUser] = useState(null);

  const fetchUser = () =>
    fetch(`https://your-api.com/users/${userId}`)
      .then((response) => response.json())
      .then(({ user }) => user);

  useEffect(() => {
    fetchUser().then((user) => setUser(user));
  }, [fetchUser]);

위의 코드는 fetchUser 함수가 변경될 때만 외부에서 api를 가져와 useEffect가 실행됩니다.

사실 이 코드는 정상적인 코드가 아닌데요. Profile이라는 컴포넌트가 리렌더링이 발생할 경우 fetchUser 함수에는 새로운 함수가 할당되게 됩니다.그러면 useEffect()함수가 호출되어 user 상태값이 바뀌고, state 값이 바뀌었기 때문에 다시 리렌더링이 일어납니다.
무한루프에 빠져버리게 되는 것이지요. 이때 useCallback을 사용할 경우 fetchUser 함수의 참조값을 동일하게 유지시킬 수 있습니다.

정리

지금까지 제가 코드리뷰 받은 내용이었습니다. 혹시라도 잘못된 내용 있으면 댓글 부탁드립니다.
React를 공부하며 성능에 신경쓰면서 코드를 작성해야된다는 점을 또한번 깨닫게 되었습니다.