Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOTORI] 2주차 첫번째 PR #82

Merged
merged 16 commits into from
May 19, 2022

Conversation

mogooee
Copy link
Collaborator

@mogooee mogooee commented May 18, 2022

🤔 step3 진행상황

🤹‍♀️ 데모페이지

[ 구현 완료 ]

  1. Coin Component 최적화
    • useMemo, useCallback
  2. History Component
    • 지갑에서 금액 투입시 알림 기능
      - useReducer
    • 자판기에서 금액 투입시 알림 기능
    • 상품을 선택했을 때 선택한 상품과 잔돈에 대한 알림 문구를 출력한다.
    • 반환버튼을 눌렀을 때 잔돈 알림을 출력한다.

[ 구현 예정 ]

  • 자판기 기능 구현

[ 고민하는 점 ]

  1. 리렌더링 최적화
    CoinProvider에서 coinselectCoin의 provider를 분리하여 최적화를 해결했습니다.
    하지만 아직 개념에 대한 이해가 부족해서 질문 남기게 되었습니다!

현재 useCoinselectCoinuseCallback을 사용하고 있습니다.
이전에는 함수 자체를 props로 넘겨주는 것이 아니라 또다른 하나의 객체{} 로 만들어 props로 넘겨주었기 때문에, {coin, selectCoin}에 대한 다른 참조값을 계속해서 만들어내는 것이 useCallback을 무용하게 만들었다고 생각하고 있습니다. 하지만 selectCoinCoin 컴포넌트 내부에서 실행되는 함수인데 참조값이 바뀌는 것과 Coin 컴포넌트가 리렌더링 되는 것과는 무관한게 아닌가하는 고민이 생겼습니다. 🥲

 <CoinContext.Provider value={coin, selectCoin}>
      {children}
    </CoinContext.Provider>

 <CoinContext.Provider value={coin}>
      <SetCoinContext.Provider value={selectCoin}>{children}</SetCoinContext.Provider>
    </CoinContext.Provider>

mogooee and others added 16 commits May 10, 2022 15:37
* update: money data

* design: Coin Component

* design: Balance Component

* design: Wallet Component
* update: product data

* design: VendProduct Component

* design: VendController Component

* design: HistoryBox Component

* design: VendingMachine Component

* design: ChangeOutlet Component

* design: InsertCoin Component

* refactor: VendController Component의 하위컴포넌트 분리

* chore: update yarn.lock
* feat: Button Component 구현

* feat: InsertCoin, BrandLabel, Unit에 Button Component 적용

* chore: polished 라이브러리 설치

* refactor: Nav를 NavLink 적용하여 리팩토링
* [DOTORI] 1주차 첫번째 PR (#25)

* chore: 프로젝트 초기 셋팅 (#2)

* feat:VendingMachine, Wallet 라우팅 (#5)

* Wallet Component - Markup (#6)

* update: money data

* design: Coin Component

* design: Balance Component

* design: Wallet Component

* VendingMachine Component - Markup (#9)

* update: product data

* design: VendProduct Component

* design: VendController Component

* design: HistoryBox Component

* design: VendingMachine Component

* design: ChangeOutlet Component

* design: InsertCoin Component

* refactor: VendController Component의 하위컴포넌트 분리

* chore: update yarn.lock

* Common Components 분리(1) (#10)

* feat: Button Component 구현

* feat: InsertCoin, BrandLabel, Unit에 Button Component 적용

* chore: polished 라이브러리 설치

* refactor: Nav를 NavLink 적용하여 리팩토링

* chore: gh-pages 라이브러리 설치

* feat: useDisplay 커스텀 훅 생성

* feat: AppLayout, ToggleDisplay Component 구현

* feat: DisplayProvider 구현

displayMode를 전역 상태로 관리한다.

* design: title font 변경, change outlet title 대문자로 변경
* refactor: components 디렉토리 구조 변경

* feat: common components - Nav 구현

* refactor: 라우터 모듈 분리

* refactor: 사용하지 않는 styled-components 제거, div->span, strong 태그 변경

* refactor: CoinContainer, VendProductContainer Component 분리
* feat: 금액 버튼 클릭시 해당 금액 개수 감소 렌더링

useCoin 커스텀 훅 사용

* feat: 금액 버튼 클릭시 잔고 금액 감소 렌더링

useBalance 커스텀 훅 사용

* feat: CoinProvider 생성

outlet 상위 컴포넌트에 생성하여 라우팅에 따른 상태 초기화 방지

* feat: InsertCoinProvider 구현

지갑에서 선택한 금액을 자판기에 투입된 금액으로 렌더링

* rename: components 디렉토리 구조 수정
coin state로 연산할 수 있는 balance state를 제거
* refactor: setCoin 로직 변경

배열의 카피본에서 같은 값을 갖는 인덱스 요소의 객체를 변경 -> map 사용

* refactor: grid repeat 함수 적용

* refactor: Coin Component useMemo 적용, setProvider 분리

Coin Component 클릭시 금액의 변동사항이 있는 컴포넌트만 리렌더링 된다.
@mogooee mogooee requested a review from kowoohyuk May 18, 2022 09:45
@mogooee mogooee self-assigned this May 18, 2022
@kowoohyuk
Copy link

도토리 안녕하세요!
개인적인 사정으로 저녁쯤 리뷰 드릴 수 있을 것 같아요.. 🙏
늦어져서 죄송합니다! 😢

Copy link

@kowoohyuk kowoohyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

도토리 안녕하세요!
json입니다!

먼저 리뷰가 늦어진점 다시 한 번 사과드립니다 🙇
저번에 말씀하신 것처럼 리렌더링 관련해서 많은 고민이 있으신 것 같아요!

말씀하신 것처럼 기존에는 object({{ coin, selectCoin }})형태로 사용했기 때문에 리렌더링이 발생합니다.
또한, 현재의 구조에서는 select에 해당하는 Context가 State에 해당하는 Context의 하위 컴포넌트이며,
selectCoin을 통해 coin의 상태가 변경되기 때문에 리렌더링이 발생하게 됩니다.
충분한 답변이 되었을까요...?!

개발하시느라 수고많으셨습니다!! 😄

Comment on lines +29 to +31
const areEqual = (prevProps, nextProps) => {
return prevProps.count === nextProps.count;
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

중괄호와 리턴을 추가한 이유가 궁금합니다!
컨벤션을 바꾸신 걸까요?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 아무 생각없이 썼네요..! 수정했습니다 🥲

@@ -4,7 +4,7 @@ import { VendProduct } from "./VendProduct";

const StyledVendProductContainer = styled.ul`
display: grid;
grid-template-columns: 25% 25% 25% 25%;
grid-template-columns: repeat(4, 1fr);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -0,0 +1,47 @@
import { useCallback } from "react";

const { createContext, useReducer } = require("react");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

일관성을 위해 import로 변경하면 어떨까요?? ✋

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

자동완성 기능을 사용해서 몰랐는데 확인을 꼭 해야겠네요..! 감사합니다 🙏

Comment on lines +8 to +21
histories.history = histories.history.concat({
...history,
id: histories.history.length + 1,
comment: `${history.coin}원이 투입되었습니다.`,
});
return histories;
case "CHANGE_COIN":
histories.history = histories.concat({
...history,
commet: `${history.change}이 반환되었습니다.`,
});
return histories;
case "PURCHASE_PRODUCT":
histories.history = histories.concat({ ...history, commet: `${history.product}를 구매했습니다.` });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

공통되는 부분을 함수로 분리해도 괜찮을 것 같습니다!

},
[coin]
);
const selectCoin = useCallback((unit) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@kowoohyuk kowoohyuk merged commit 75ed2d7 into codesquad-members-2022:dotori May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants