코드 리뷰

[ori-kim]

안녕하세요. 셩님 플린님의 아이들 소속 김민우입니다!

셩님이 작성하신 코드를 전반적으로 읽어봤는데요. 점점 구조도 생각하시는 모습이 보이고, 꾸준이 공부하시는 모습에 자극을 많이 밭습니다.

사실 요즘 저도 리액트를 잘 알고 사용하나? 라고 생각하는 입장이라… 완전 주관적인 내용으로 간단히 리뷰해보겠습니다.

저라면 이렇게 만들지 않았을까? 라는 느낌으로 리뷰를 작성해 봅니다.

export const API_URL =
  "https://marvel-proxy.nomadcoders.workers.dev/v1/public/characters";

export const LIMIT_PARAMS_CHARACTERS =
  "?limit=10&orderBy=modified&series=24229,1058,2023";

export const LIMIT_PARAMS_GENRES = "?limit=5&orderBy=modified";

일단 주소를 상수로 사용하는 거 너무 좋네요! 항상 저도 이렇게 써야지 하는데… 몸이 안따라 주는거 같습니다.

React.FC

---

const CharacterDetail: React.FC<CharacterDetailProps> = ({
  name,
  coverImg,
  comics,
  series,
  stories,
  events,
}) => {
...
}

서영님의 코드를 보면 타입으로 React.FC를 주었는데요. 제가 듣기로 이 타입은 지양해야하는 타입이라고 들은 기억이 있습니다.

https://emewjin.github.io/why-not-fc/

그냥 props 타입만 지정해줘도 충분하지 않을까 생각합니다.

Data fetching

---

서영님의 Detail 컴포넌트를 보면, 데이터를 불러오는 로직이 다음과 같습니다.

function Detail() {
  const { id } = useParams();

  const [loading, setLoading] = useState(true);
  const [data, setData] = useState<CharacterDetails | null>(null);
  const [comics, setComics] = useState([]);
  const [series, setSeries] = useState([]);
  const [stories, setStories] = useState([]);
  const [events, setEvents] = useState([]);

  const getData = async () => {
    const json = await (await getCharacter(id)).json();
    const jsonComics = await (await getComics(id)).json();
    const jsonSeries = await (await getSeries(id)).json();
    const jsonStories = await (await getStories(id)).json();
    const jsonEvents = await (await getEvents(id)).json();

    console.log("Character : ", json.data.results[0]);
    console.log("jsonComics: ", jsonComics);
    console.log("jsonSeries: ", jsonSeries);
    console.log("jsonStories: ", jsonStories);
    console.log("jsonEvents: ", jsonEvents.data.results);

    setData(json.data.results[0]);
    setComics(jsonComics.data.results);
    setSeries(jsonSeries.data.results);
    setStories(jsonStories.data.results);
    setEvents(jsonEvents.data.results);
    setLoading(false);
  };

  useEffect(() => {
    getData();
  }, []);

	return ...
}

이 것은 Chareacters 컴포넌트입니다.

function Characters() {
  const [loading, setLoading] = useState(true);
  const [data, setData] = useState<MarvelCharacter[]>([]);

  const getData = async () => {
    const json = await (await getCharacterList()).json();
    setData(json.data.results);
    setLoading(false);
  };

  useEffect(() => {
    getData();
  }, []);

  return ...
}

두 코드에서 데이터를 불러오는 방식은 다음과 같은 코드가 반복 되는거 같아요.

1. loading상태를 나타내는 state
2. data fetching 함수를 이용해 data를 가져와 state를 변경함.

다음과 같은 동작들을 getData() 함수에서 반복하고 계신다고 느껴졌어요.

저라면 이런 상황에서는 데이터와 로딩을 처리해주는 새로운 커스텀 훅으로 분리해줄것 같아요!

예를 들자면 이런 코드가 될 수 있겠죠?

const useFetch = <T>(fetchFn: () => Promise<Response>)=>{
    const [isLoading, setIsLoading] = useState(true)
    const [data, setData] = useState<T>()

    const fetching = async ()=>{
        const {data} = await (await fetchFn()).json()

        setData(data)
        setIsLoading(false)
    }

    useEffect(()=>{
        fetching()
    }, [])

    return {data, isLoading}
}

이러면 사용처에서 useFetch 훅만 불러준다면, 데이터 페칭과 관련된 로직을 더 작성해줄 필요가 없지 않을까요?
(물론 코드에서는 에러처리와 같은 코드는 제외 되어 있기 때문에 그부분도 한번 고려해보세요. 추후에 사용할 react-query를 사용해 보시는 것도 좋겠네요.)

또한 서영님의 Detail 컴포넌트에서는 로딩 state 한개로 comics, series, stories, events가 전부 응답이 완료될 때까지 기다려 주는데 각각 따로 사용해서 먼저 불러 온 데이터를 사용자에게 먼저 표시해 줄 수 있다면 좋지 않을까요?

사실 서영님이 지금 리팩토링 하는 과정이라 제가 할 수 있는 이야기는 이정도인 것 같아요!

리액트 10주 스터디 동안 점점더 발전할 셩님의 코드를 볼 생각을 하니 너무 기대되네요!!

궁금한 점있으시면 언제든지 질문해주세요! 같이 고민해봐요.

그럼 이만 줄이겠습니다.

[shionpark]

민우님! 상세한 리뷰 감사합니다! 민우님의 피드백을 받아 코드를 수정하고 있습니다 :)
처음 받아보는 코드 리뷰에 부끄럽지만 덕분에 몰랐던 사실을 배울 수 있었어요!

다양한 사람들의 코드를 읽는 것이 얼마나 도움이 되는지 이번 스터디를 통해 느끼고 있는 중입니다.
거기에 코드를 보고 리뷰할 수 있는 건 또 다른 수준이더라고요..
저도 최대한 빨리 민우님의 코드 리뷰를 해드리고 싶은데 조금만 기다려주세요..!

제가 코드 리뷰를 하는 입장이 되다보니 민우님이 얼마나 정성 가득 리뷰해주셨는지도 느낄 수 있었습니다.
항상 응원해주셔서 감사합니다! 민우님 짱.