[점심 뭐 먹지 미션 Step 2] 윤생(이윤성) 미션 제출합니다. #64

2yunseong · 2023-04-16T07:15:56Z

안녕하세요 케빈!

이번 2단계 미션은 클래스형 컴포넌트를 함수형 컴포넌트로 변경하고, Custom Hook으로 분리하는 것 위주로 진행했어요.

클래스형 컴포넌트에서 함수형 컴포넌트로 변경하니 중복되는 코드가 많이 사라지는 경험도 하고 간결해짐을 느꼈어요..!!

다만 그만큼 componentDidMount, ~DidUpdate 등을 useEffect로 사용하다 보니 리액트 생명주기에 대해 공부를 좀 해봐야겠다 느꼈습니다!

이번 미션에서 Custom Hook을 분리할 때는 RestaurantList 와 RestaurantDrawer에서 사용하는 로컬스토리지에서 데이터를 불러오는 로직이 같다고 생각해 재사용성 측면에서 분리하였습니다.

저번처럼 궁금한 부분은 댓글에 남겨두겠습니다! 잘부탁드립니다 :)

미션 구현 사이트

2yunseong

저번처럼 질문 남깁니다!!

2yunseong · 2023-04-16T07:25:00Z

src/RestaurantDetailDrawer.tsx

+      <button
+        type="button"
+        className="button button--secondary text-caption"
+        onClick={() => onToggleDrawer()}


onToggleDrawer는 App으로 부터 받은 녀석인데요..
다음과 같이 구현되어 props로 전달되어 집니다.

// App.tsx const onToggleDrawer = (id: number = NO_SELECT_ID): void => { setDrawerSelectId(id); setIsOpenDrawer(!isOpenDrawer); };

네이밍이 이벤트핸들러 스럽긴 한데, 따로 한번 감싸주어야 할까요?

// RestaurantDetailDrawer.tsx const onClick = () => { onToggleDrawer(); } // ... 코드 생략 <button type="button" className="button button--secondary text-caption" onClick={onClick}

onToggleDrawer 외에 다른 이벤트가 없다면 굳이 안 감싸줘도 될거 같아요!

2yunseong · 2023-04-16T07:27:13Z

src/RestaurantItem.tsx

 type RestaurantProps = {
-  readonly restaurant: Omit<Restaurant, 'link'>;
+  restaurant: Omit<Restaurant, 'link'>;
  onToggleDrawer: (id?: number) => void;
 };


저번에 케빈께서 type을 공통적으로 빼면 어떠냐고 제안해주셨어요! 거기에 대한 제 생각입니다! props로 비슷하다고 생각해 저번
#33 (comment) 에 대한 답변을 듣고 싶어 이곳에 남깁니다!!

type 은 다른 파일에서 공통적으로 사용할 수 도있고, 확장해서 사용할수도 있어서 따로 빼놓는건 어떨까요?

저는 이런 App 컴포넌트의 상태를 명시해주는 건 오히려 외부 파일로 분리하면 App.tsx 를 읽을 때 번갈아서 봐야될 것 같다는 생각이 들어 함께 넣어 두었습니다. 또한 AppState는 App의 state가 가질 타입이라고 생각이 들어 다른 곳에서는 사용하지 않을 것 같아서 같은 파일에 넣어두었습니다! 이것에 대해 케빈은 어떻게 생각하시나요??

그런 이유라면 같은 파일에 둬도 좋을거 같아요!
하지만 저는 타입이 general하게 사용되는 타입이 아니고 해당 컴포넌트의 Props에서만 사용하기 때문이라면 같은 파일보단 같은 파일에 두지 않고, 뷰에 집중하기 위해 ~~~.type.tsx 이렇게 파일을 만들어서 나누어서 사용할 거 같아요! 윤생이 말한 방법은 ide에서 타입을 추론해주기 때문에 마우스 커서만 올려두면 알려줄것 같습니다!

ide를 이용할 수도 있겠네요! 또 하나 얻어갑니다!! 감사합니다!

2yunseong · 2023-04-16T07:29:39Z

src/RestaurantList.tsx

+const filterByCategory = (restaurantList, category): Restaurant[] => {
+  if (category === '전체') return restaurantList;
+  return restaurantList.filter(
+    (restaurant) => restaurant.category === category
+  );
+};


이런 계산하는 로직은 따로 분리하는게 나을까요? 저는 이 컴포넌트에서만 사용하는 로직이니까 같은 파일에 넣어두었는데, 케빈의 생각이 궁금합니다.

저도 if (category === '전체') 이런 조건이 있으니까 해당 컴포넌트에서만 사용하는 로직이 분명해서 해당 파일 근처에 두는게 좋을거 같은데, 지금 hook 을 배우고있으니까, hook 을 활용해서 관리해보면 어떨까요?

너무 잘 알고 계시겠지만, 활용할수 있는 힌트는 메인 코멘트에 적어 놓을게요~

한번 도전해보겠습니다~!

JeongBin0227 · 2023-04-16T15:54:28Z

다만 그만큼 componentDidMount, ~DidUpdate 등을 useEffect로 사용하다 보니 리액트 생명주기에 대해 공부를 좀 해봐야겠다 느꼈습니다!

class 의 생명주기를 많이 활용한 코드에서 function 형으로 바꾼 코드를 많이 보시면 도움되실거예요!
지금은 큰 영향 없지만 사이즈가 큰 서비스를 만들때 큰 도움이 되실거예요! (기획을 보면 컴포넌트가 ~~~ 어떨때등 자세히 기획이 나오는 경우가 많아서 생명주기를 자세히 알아야합니다)

JeongBin0227 · 2023-04-16T16:23:56Z

재사용이 가능한 로직을 hook 으로 만들어서 사용하면 좋습니다.
예를 들어 자주 사용하는 api 호출을 예로 들어서 설명하면

같은 api를 사용해서 그리는 컴포넌트들이 있는데, 모든 컴포넌트마다 api 로직을 넣는것이 아니라

useGetApi 라는 파일을 만들고 (custom hook의 이름은 무조건 use로 시작해야합니다.)

export default function useGetApi() {
  const [loading, setLoading] = useState(false);
  const [error, setError] = useState();
  const [data, setData] = useState([]);

  useEffect(() => {
    setLoading(true);
    setError(undefined);
    fetch(~~~)
      .then((res) => res.json())
      .then((data) => {
        setData(data);
      })
      .catch((e) => setError('에러가 발생했습니다.'))
      .finally(() => setLoading(false));
  }, []);
  return [loading, error, data];
}

라는 함수를 만들어, 컴포넌트 마다 const {loading, error, data} = useGetApi() 를 사용해서 뷰와 데이터 패칭을 분리하여 좀더 각각의 관심사에 집중할 수 있습니다

결론은 반복되는 훅 활용 메소드들을 하나로 줄여줌으로써 더 간결하고 보기 좋은 코드를 만들 수 있고, 컴포넌트 로직을 뽑아내서 정의한 후 간편하게 호출할 수 있다는 것이 custom hooks의 장점이 입니다.

JeongBin0227

안녕하세요 윤생!
확실히 코드가 많이 깔끔해지고 읽기 좋아진거 같아요! 🎊

충분히 바로 Approve를 드릴 수 있었지만, 질문해주신 부분이나, 추가적으로 말씀드리고 싶은 내용들을 코멘트로 남겨서 해당 부분 확인해보시면 좋을거 같아서 Request changes 로 드립니다~

코멘트 확인부탁드립니다~ step2 하시느라 고생많으셨습니다 👍 💯

JeongBin0227 · 2023-04-16T15:45:47Z

src/RestaurantDetailDrawer.tsx

+      <button
+        type="button"
+        className="button button--secondary text-caption"
+        onClick={() => onToggleDrawer()}


onToggleDrawer 외에 다른 이벤트가 없다면 굳이 안 감싸줘도 될거 같아요!

JeongBin0227 · 2023-04-16T15:56:19Z

src/App.tsx

+  const onToggleDrawer = (id: number = NO_SELECT_ID): void => {
+    setDrawerSelectId(id);
+    setIsOpenDrawer(!isOpenDrawer);
+  };


이 부분과 const [isOpenDrawer, setIsOpenDrawer] = useState<boolean>(false); 해당 상태는 App 에서가 아닌 RestaurantDetailDrawer 안에서 정의해도 되지않을까요? 최대한 App은 가벼운게 좋습니당!

Drawer를 열고 닫는 부분이 같은 계층에 있는 RestraurantItem과 RestaurantDetailDrawer 둘다에 있어서 다음과 같이 주었습니다!
state 끌어올리기를 참고했어요!!

아하 RestraurantItem, RestaurantDetailDrawer 둘다 필요하군요! 이해했습니다

JeongBin0227 · 2023-04-16T15:57:36Z

src/MainHeader.tsx

-    );
-  }
-}
+const MainHeader: React.FC = () => {


타입으로 React.FC 를 주는건 좋지 않습니다!
이유는 크게는 defaultProps 를 쓰지 못하는 이유가 있는데, 관련해서 읽을만한 것들 첨부해드리겠습니다 :)
https://yceffort.kr/2022/03/dont-use-react-fc
https://velog.io/@yena1025/FunctionComponent-FC-%EC%82%AC%EC%9A%A9-%EC%A4%84%EC%9D%B4%EA%B8%B0-24jhm0wp

헉 읽을 것 까지.. 감사합니다 케빈 🙇

JeongBin0227 · 2023-04-16T16:05:23Z

src/RestaurantItem.tsx

 type RestaurantProps = {
-  readonly restaurant: Omit<Restaurant, 'link'>;
+  restaurant: Omit<Restaurant, 'link'>;
  onToggleDrawer: (id?: number) => void;
 };


그런 이유라면 같은 파일에 둬도 좋을거 같아요!
하지만 저는 타입이 general하게 사용되는 타입이 아니고 해당 컴포넌트의 Props에서만 사용하기 때문이라면 같은 파일보단 같은 파일에 두지 않고, 뷰에 집중하기 위해 ~~~.type.tsx 이렇게 파일을 만들어서 나누어서 사용할 거 같아요! 윤생이 말한 방법은 ide에서 타입을 추론해주기 때문에 마우스 커서만 올려두면 알려줄것 같습니다!

JeongBin0227 · 2023-04-16T16:06:50Z

src/RestaurantList.tsx

+const filterByCategory = (restaurantList, category): Restaurant[] => {
+  if (category === '전체') return restaurantList;
+  return restaurantList.filter(
+    (restaurant) => restaurant.category === category
+  );
+};


저도 if (category === '전체') 이런 조건이 있으니까 해당 컴포넌트에서만 사용하는 로직이 분명해서 해당 파일 근처에 두는게 좋을거 같은데, 지금 hook 을 배우고있으니까, hook 을 활용해서 관리해보면 어떨까요?

JeongBin0227 · 2023-04-16T16:07:21Z

src/RestaurantList.tsx

+const filterByCategory = (restaurantList, category): Restaurant[] => {
+  if (category === '전체') return restaurantList;
+  return restaurantList.filter(
+    (restaurant) => restaurant.category === category
+  );
+};


너무 잘 알고 계시겠지만, 활용할수 있는 힌트는 메인 코멘트에 적어 놓을게요~

JeongBin0227 · 2023-04-16T16:09:58Z

src/util/constant.ts

@@ -44,7 +44,7 @@ export const SORTING_OPTIONS: SelectOption<Sorting>[] = [
    textContent: '이름순',


이부부 놓쳤는데 CATEGORY_OPTIONS 변수에 textContent 가 option 의 글자로 활용되니까 label 은 어떨까요?

오 좋은 네이밍 조언 감사합니다!!

2yunseong

케빈 피드백 반영하고 몇 가지 간단한 질문남겼어요!!

2yunseong · 2023-04-17T07:18:34Z

src/util/type.ts

+  onToggleDrawer: (id?: number) => void;
+};
+
+export type SelectContainerProps = Pick<SelectProps, 'onChangeFilterOptions'>;


props 타입을 유틸리티로 해도 괜찮을까요?

괜찮다고 생각한 이유는 위의 SelectProps에 이미 선언되어있으니, 재사용하고 싶어서 그랬습니다.

그런데 지금 작성하고 보니 또 이러면 Props가 추가 되었을 때 확장이 어렵다는 생각이 드는데.. 또 그러면 Union을 사용해 확장하면 되는거 아닌가 싶네요

케빈의 의견도 궁금합니다!

네 저도 재사용이 가능한 타입은 재사용하는게 좋아보입니다!
만약 추가가 되었을 경우에 Union 을 사용해서 수정하면 될거같아요 👍

2yunseong · 2023-04-17T07:19:56Z

src/hooks/useFilterRestaurantList.ts

+const useFilterRestaurantList = (category, sorting) => {
+  const handleRestaurant = (restaurantList) => {
+    const result = pipe(filterByCategory, filterBySort)(restaurantList, [
+      category,
+      sorting,
+    ]);
+    return result;
+  };
+
+  const filteredRestaurantList = useRestaurantList(
+    'restaurantList',
+    [],
+    handleRestaurant
+  );
+
+  return filteredRestaurantList;
+};


Hook으로 분리한 로직인데, 필터 처리 useRestaurantList에 핸들러를 넘겨주어 필터링을 처리할 수 있도록 하였습니다.

JeongBin0227

안녕하세요 윤생~
미션 수고 많으셨습니다😀 몇가지 코멘트 확인해주세요!
이제 충분히 머지될수 있는 코드라고 생각해서 이만 머지할게요~

처음 다루는 리액트와 커스텀 훅 개념 때문에 많은 러닝 커브가 있었을텐데 그동안 너무 고생많으셨어요 👍

JeongBin0227 · 2023-04-18T03:29:31Z

src/App.tsx

+  const onToggleDrawer = (id: number = NO_SELECT_ID): void => {
+    setDrawerSelectId(id);
+    setIsOpenDrawer(!isOpenDrawer);
+  };


아하 RestraurantItem, RestaurantDetailDrawer 둘다 필요하군요! 이해했습니다

JeongBin0227 · 2023-04-18T03:30:29Z

src/util/type.ts

+  onToggleDrawer: (id?: number) => void;
+};
+
+export type SelectContainerProps = Pick<SelectProps, 'onChangeFilterOptions'>;


네 저도 재사용이 가능한 타입은 재사용하는게 좋아보입니다!
만약 추가가 되었을 경우에 Union 을 사용해서 수정하면 될거같아요 👍

jiwonh423

이미 커스텀 훅을 사용해서 "리액트 컴포넌트 자체는 UI와 이벤트만 알게 하자"를 완벽히 지킨것 같네요 🎉 배우고 갑니다~

jiwonh423 · 2023-04-25T02:51:31Z

src/RestaurantDetailDrawer.tsx

+  onToggleDrawer,
+}: RestaurantDetailDrawerProps) => {
+  const restaurantList = useRestaurantList('restaurantList', []);
+  const restaurant = getRestaurantById(restaurantList, restaurantId);


"리액트 컴포넌트 자체는 UI와 이벤트만 알게 하자"의 모범답안이네요 👍

jiwonh423 · 2023-04-25T02:56:11Z

src/hooks/useLocalStorage.ts

+    let data;
+    try {
+      data = JSON.parse(rawData);
+    } catch (err) {


에러처리까지 꼼꼼하게 잘 구현하셨네요 😄

jiwonh423 · 2023-04-25T03:07:16Z

src/hooks/useRestaurantList.ts

+import useLocalStorage from './useLocalStorage.ts';
+import { identity } from '../util/util.ts';
+
+const useRestaurantList = (key, initValue, callback = identity) => {


callback의 기본 매개변수를 주니 범용성이 더 좋아진것 같네요 👍

2yunseong added 18 commits April 15, 2023 00:04

refactor(Drawer): 함수형 컴포넌트로 변경

c362b95

refactor(Header): 함수형 컴포넌트로 변경

01ac77c

refactor(Select): 함수형 컴포넌트로 변경

b7f2109

refactor(MainHeader): 함수형 컴포넌트로 변경

e8a33ce

refactor(RestaurantDetailDrawer): 함수형 컴포넌트로 변경

3ad22ae

refactor(RestaurantItem): 함수형 컴포넌트로 변경

310aee7

refactor: 메서드 분리

14bd03b

refactor: type 명 구체화

136ee31

refactor(RestaurantList): 함수형 컴포넌트로 변경

86a387b

refactor(SelectContainer): 함수형 컴포넌트로 변경

0aeafd1

refactor(App): 함수형 컴포넌트로 변경

53cbdda

fix: 페이지 title 수정

18be4e1

refactor: 로컬스토리지 로직 Custom Hook 으로 변경

a4a480f

chore: typescript config 설정

1a170e6

style: 코드 포맷팅

d753b1b

refactor: type import 명시

6dae5e2

refactor: 의미에 맞는 변수명 변경

597a866

chore: trivial

02bb074

2yunseong commented Apr 16, 2023

View reviewed changes

JeongBin0227 self-assigned this Apr 16, 2023

JeongBin0227 requested changes Apr 16, 2023

View reviewed changes

2yunseong added 7 commits April 17, 2023 12:05

refactor: 디렉터리 네이밍 변경

fc22a75

refactor: React.FC 선언 제거

f4eb604

style: 코드 포맷팅

139223d

feat: 필터링을 해주는 로직 Hook으로 분리

6c20b21

refactor: type 분리

af8939e

refactor: 프로퍼티 네이밍 변경

aa8a891

feat: 웹 접근성 개선

5429324

feat: default로 전달하는 함수 네이밍

ab48c82

2yunseong commented Apr 17, 2023

View reviewed changes

JeongBin0227 approved these changes Apr 18, 2023

View reviewed changes

JeongBin0227 merged commit d7832f3 into woowacourse:2yunseong Apr 18, 2023

jiwonh423 reviewed Apr 25, 2023

View reviewed changes

		@@ -44,7 +44,7 @@ export const SORTING_OPTIONS: SelectOption<Sorting>[] = [
		textContent: '이름순',

[점심 뭐 먹지 미션 Step 2] 윤생(이윤성) 미션 제출합니다. #64

[점심 뭐 먹지 미션 Step 2] 윤생(이윤성) 미션 제출합니다. #64

Conversation

2yunseong commented Apr 16, 2023

2yunseong left a comment

Choose a reason for hiding this comment

2yunseong Apr 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeongBin0227 commented Apr 16, 2023

JeongBin0227 commented Apr 16, 2023

JeongBin0227 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2yunseong Apr 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2yunseong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeongBin0227 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiwonh423 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2yunseong Apr 16, 2023 •

edited

Loading

2yunseong Apr 17, 2023 •

edited

Loading