sparcs-kaist / biseo

Realtime voting tool for meetings @ KAIST
https://biseo.sparcs.org
MIT License
6 stars 0 forks source link

Optimize memoizeable values and callback functions #362

Open JeukHwang opened 1 year ago

JeukHwang commented 1 year ago

제안 내용 *

현 상황

여러 가지 구현이 혼재되어있음.

  1. useMemo, useCallback을 이용한 방법 https://github.com/sparcs-kaist/biseo/blob/cdd9f387d022189736e2dada3587c4936e50fd95/client/src/components/molecules/ChatInput.tsx#L36-L51 https://github.com/sparcs-kaist/biseo/blob/cdd9f387d022189736e2dada3587c4936e50fd95/client/src/components/molecules/ChatInput.tsx#L27-L33
  2. 그저 함수를 이용한 방법 https://github.com/sparcs-kaist/biseo/blob/cdd9f387d022189736e2dada3587c4936e50fd95/client/src/components/organisms/CreateTemplateModal.tsx#L81-L87 https://github.com/sparcs-kaist/biseo/blob/cdd9f387d022189736e2dada3587c4936e50fd95/client/src/components/organisms/CreateTemplateModal.tsx#L30-L35

대안

1번 구현으로 통일하길 원함.

장점

2번 방법의 단점: 컴포넌트가 리렌더링 될 때마다 값 및 함수가 다시 계산됨 1번 방법의 장점: dependency array의 값 중 일부가 바뀔 때에만 다시 계산됨(memoization)

관련 Task *

JeukHwang commented 1 year ago

@SnowSuno dependency array를 [input.value, validated]가 아니라 [input.value]만 작성할 수도 있나요? React가 알아서 validatedinput.value에 의존함을 알아내서 허락해주는... 그런 상황도 있나요?

SnowSuno commented 1 year ago

useMemo, useCallback을 사용해서 값, 함수를 memoizing하는 것은 '코딩 컨벤션'이라고 보기에는 조금 어렵기 때문에 리팩토링이 아닌 성능 최적화라고 하는 것이 맞는 것 같습니다.

동일하게, 이 훅들은 어떠한 코딩 컨벤션이 아닌 특정 목적에 따라서 쓰이는 함수들이기 때문에 통일한다기보단, 최적화가 필요한 부분에 적용해야 하는 함수들인 것 같습니다.

SnowSuno commented 1 year ago

@SnowSuno dependency array를 [input.value, validated]가 아니라 [input.value]만 작성할 수도 있나요? React가 알아서 validatedinput.value에 의존함을 알아내서 허락해주는... 그런 상황도 있나요?

Dependency array에는 어떤 값을 넣는다고 해서 에러가 나지는 않습니다. 다만 그 값들이 바뀔 때에 다시 계산하라고 알려주는 것입니다.

위 상황에선 input.value가 바뀌면 validated가 항상 바뀌기 때문에 말씀하신 것처럼 [input.value] 만들 deps array로 주어도 똑같이 동작합니다. 다만 이를 명시적으로 작성해주지 않는 경우에는 코드를 유심히 읽어 보아야만 그 사실을 알 수 있고, react에서 dependency array는 예상치 못한 버그를 많이 발생시키는 원인들 중 하나이기 때문에 dependency array에는 그 훅에서 사용하는 모든 dependency들을 명시해 주는 것이 best practice입니다.

실제로 eslint에서 이를 강제하는 react-hooks/exhaustive-deps 규칙은 eslint-plugin-react의 recommended 프리셋에서 활성화되어 있는 규칙이고, 이에 따라 마찬가지로 비서에서도 활성화되어있기 때문에 [input.value] 만 쓰면 린터가 화낼거에요

SnowSuno commented 1 year ago

당장 urgent한 이슈는 아닌 성능 최적화 이슈라서 전동대회 마일스톤에선 뺐어용