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

[REFACTOR] Context API를 활용하여 Modal 사용성 개선 #277

Merged
merged 22 commits into from
Sep 26, 2024

Conversation

rbgksqkr
Copy link
Contributor

@rbgksqkr rbgksqkr commented Sep 20, 2024

Issue Number

#272

As-Is

  • 라운드를 넘어갈 때, mutation 에러를 처리할 때 등 사용자에게 알림을 보여줘야할 때 모달을 많이 사용
  • 모달을 사용할 때마다 모달 컴포넌트가 해당 컴포넌트에 위치하는 것에 불편함을 느낌
  • 에러에 따라 다른 메세지를 보여줘야 할 때 대응 어려움

To-Be

  • 모달을 context로 관리하여 Modal을 관리하는 게 컴포넌트의 역할이 아닌 ModalContext의 역할로 위임
  • 함수의 인자로 컴포넌트와 message를 받아 어떤 모달을 어디서든 띄울 수 있도록 구현
  • 테스트 코드를 구현해놓고 리팩토링 후에도 정상적으로 동작하도록 개발

Check List

  • 테스트가 전부 통과되었나요?
  • 모든 commit이 push 되었나요?
  • merge할 branch를 확인했나요?
  • Assignee를 지정했나요?
  • Label을 지정했나요?

Test Screenshot

(Optional) Additional Description

모달을 Context로 관리

기존에 모달을 사용하려면 모달을 컴포넌트 내부에 위치시키고, 하나의 isOpen 상태를 공유해야 했다.

이러한 문제 때문에 컴포넌트의 역할이 아닌데도 해당 컴포넌트가 의존해야하고, 에러 상황에서만 발생하는 모달도 컴포넌트에 있어야 했다.

또한 title과 message도 동적으로 넘겨주기 어려웠다.

이러한 문제를 Modal을 Context로 관리하고, 함수로 인자들을 받을 수 있도록 설정하여 해결할 수 있었다. 이를 통해 컴포넌트는 성공한 케이스만 갖고 있고, 에러 핸들링도 쉬워져 코드의 전체적인 가독성이 높아졌다.

❌ before

const GameResult = () => {
  const { isOpen, show, close } = useModal();

  return (
    <>
      ...
      <FinalButton showModal={show} />
      <AlertModal
        isOpen={isOpen}
        onClose={close}
        title="방 초기화 에러"
        message="방을 초기화하는데 실패했어요. 다시 시도해주세요"
      />
    </>
  );
};

✅ after

import FinalButton from '../common/FinalButton/FinalButton';

const GameResult = () => {
  return (
    <>
      ...
      <FinalButton />
    </>
  );
};

export default GameResult;

🌸 Storybook 배포 주소

https://woowacourse-teams.github.io/2024-ddangkong/storybook/

@rbgksqkr rbgksqkr added ♻️ refactor 리팩토링 ✅ test 테스트 관련 🫧 FE front end labels Sep 20, 2024
@rbgksqkr rbgksqkr added this to the FE Sprint5 milestone Sep 20, 2024
@rbgksqkr rbgksqkr self-assigned this Sep 20, 2024
@rbgksqkr rbgksqkr linked an issue Sep 20, 2024 that may be closed by this pull request
1 task
Copy link
Contributor

@useon useon left a comment

Choose a reason for hiding this comment

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

하이루요 마루 ~ ! ! 해당 작업으로 불 필요한 코드가 엄청 줄었네요 대마루 !!🌟커밋도 잘 나누어져 있어서 보기가 편했답니다 ! Context로 관리를 하게 되는 것 뿐만 아니라 useModal의 유연성과 사용성을 더 높이고 test util 작성 및 테스트 추가까지 꼼꼼 최고 ,, 몇 가지 코멘트 달았으니 편하게 확인해 주시고 답변 주세요오 ~ 수고하셨습니다 🤭👍

@@ -62,4 +66,12 @@ const customRender = (ui: React.ReactNode, options: CustomRenderOptions = {}) =>
});
};

export { wrapper, customRender };
const customRenderWithIsMaster = (Component: React.ReactNode, isMaster: boolean) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

🌸 칭찬 🌸

와 선생님 이거 넘 좋은데요 ~ ? ? ? 🤭🤭🤭 테스트는 아니었지만 memberId 임의로 안 넣고 안 된다며 울부짖던 저의 모습이 주마등처럼 지나가네요 .. ^^ .. 어차피 recoil을 사용하는 경우 전부 해당 값이 필요하니까 유틸로 빼서 더 편리하게 사용할 수 있어 정말 좋네요 !!!!

import { customRenderWithIsMaster } from '@/utils/test-utils';

describe('Header 테스트', () => {
it('방 설정 버튼을 클릭했을 때, 방 설정 모달이 뜬다.', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

🌸 칭찬 🌸

모달 개선 작업을 하면서 모달이 뜨는 부분에 대한 헤더 테스트도 추가했군요 !!!!?? 참으로 꼼꼼한 청년 🤭👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

리팩토링했다가 안되면 안되니까 테스트 코드 작성 후 리팩토링 진행하였습니다:)

if (dispatch === null) {
throw new Error('ModalDispatchContext가 존재하지 않습니다.');
}
return dispatch;
};
export default useModal;
Copy link
Contributor

Choose a reason for hiding this comment

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

🙏 제안 🙏

useModal이 Modal 컴포넌트 폴더 내에 말고 hooks 폴더에 위치하여 있는 점이 살짝 고민되네요 ! 저는 여러 곳에서 사용되는 훅이 아니라 hooks 폴더 보다는 Modal 폴더 내에 위치하는 것이 더 찾기 편하다 생각하는데 마루 생각은 어떠신가요 ? 🙌

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오호 useModal이니까 Modal 자체의 동작이라고 생각할 수도 있겠네용
저는 해당 커스텀훅을 사용하는 입장에서 바라봤습니다! 여러 곳에서 사용되는 훅이 아니라고 하셨는데, 현재 코드에서는 여러 곳에서 사용되고 있다고 생각합니다!

만약 Modal 컴포넌트나 Provider 내부적으로만 동작한다면 Modal 근처에 위치시키는 게 좋다고 생각하지만, 현재 코드에서는 useToast와 마찬가지로 사용처가 범용적이라 생각해서 hooks 디렉토리에 위치시켰습니다.
useModalEscClose, useDisableBackgroundScroll 은 모달 컴포넌트를 사용하는 곳에선 모두 동작하지만, useModal이 Modal 컴포넌트 내에서 동작하는 훅이 아니라고 생각하였습니다!

썬데이의 생각은 어떤가요 !!!

Copy link
Contributor

Choose a reason for hiding this comment

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

오 ... 너무나도 설득력 있는 주장이군요 !!! 마루가 왜 거기에 위치시켰는지 단번에 이해했습니다 ^ .^ 저도 좋네요 !

onClose={close}
message={isMaster ? '방 생성에 실패했습니다.' : '해당 방에 참여할 수 없습니다.'}
title={isMaster ? '방 생성 실패' : '방 참가 실패'}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

🌸 칭찬 🌸

정말 많은 코드가 이런 식으로 줄일 수 있게 되어 좋아요 🤭👍👍👍 !!!

const AlertModal = ({ isOpen, onClose, onConfirm, message, title }: AlertModalProps) => {
const handleClick = () => {
onConfirm && onConfirm();
onClose();
Copy link
Contributor

Choose a reason for hiding this comment

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

🌸 칭찬 🌸

오 알람 모달과 안내 모달을 하나로 합치면서 확인 함수가 있는 경우에만 onConfirm 동작을 하고 모달이 닫히도록 유연하게 수정이 되었네요 !! 좋은데 ........ ???????? 천재만재.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

굿굿 만약 확인버튼과 닫기버튼이 별도의 UI로 존재한다면 수정해야할 것 같긴 하네용

Copy link
Contributor

Choose a reason for hiding this comment

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

좋아요 해당 UI가 생기게 된다면 그때 고민해 봅시다 ^^

<br />
</Fragment>
))}
{message &&
Copy link
Contributor

Choose a reason for hiding this comment

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

🌸 칭찬 🌸

혹시 메세지가 없는 경우가 있을 수도 있으니 꼼꼼한 처리 ........... 감동.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

message가 없는 모달도 있어서 optional로 타입을 지정했더니 꼼꼼한 사람이 되었네요 ㅎㅎ

}));
};

const dispatch = useMemo(() => ({ show, close }), []);
Copy link
Contributor

@useon useon Sep 23, 2024

Choose a reason for hiding this comment

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

💭 질문 💭

해당 부분에 useMemo를 사용하는 이유가 무엇인지 궁금해요 ! 불필요한 리렌더링을 방지하고자 작성하신 걸까요? useMemo를 사용하기 때문에 처음 생성한 dispatch 객체에 있는 show와 close 함수는 최초 생성된 함수이고 show와 close 함수는 각각 useCallback을 사용하지 않아 매번 새로운 함수를 참조할텐데 여기서 발생하는 문제는 없을까요? 제가 useMemo와 useCallback을 많이 사용해 보지 않아서 .. 마루의 맛도리 설명을 기다려요 .. 🫡🙌

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Context API memoization (useMemo vs useCallback)

불필요한 리렌더링을 방지하고자 작성하신 걸까요?

넵 맞습니다! velopert 님이 작성한 블로그글을 참고하여 "그럴 것 같다" 라고만 생각했는데, 코멘트가 들어온김에 직접 측정하여 useMemo를 적용하지 않은 경우와 적용한 경우를 비교해봤습니다!

useMemo를 사용하기 때문에 처음 생성한 dispatch 객체에 있는 show와 close 함수는 최초 생성된 함수이고 show와 close 함수는 각각 useCallback을 사용하지 않아 매번 새로운 함수를 참조할텐데 여기서 발생하는 문제는 없을까요?

심신미약 상태라 문장을 제대로 이해하지는 못했지만 메모이제이션이 제대로 되지 않을 것을 우려한 것 같아요 ㅎㅎ
useMemo는 첫번째 인자로 들어오는 함수의 반환값을 리렌더링 간에 캐싱하는 훅, useCallback은 첫번째 인자로 들어오는 함수 자체를 리렌더링 간에 캐싱하는 훅입니다!

useCallback은 테스트해보니 큰 효과가 없었어요! 궁금해서 이유를 생각해보니 setState가 발생하면 리렌더링이 발생하여 컴포넌트 내의 코드가 다시 실행될텐데, dispatch가 매번 새로운 객체를 만드니까 참조하는 컴포넌트도 리렌더링이 일어나는 것으로 보입니다! context로 내려주는 dispatch는 useMemo로 반환값인 { show, close } 를 캐싱한 것이고, 의존성 배열이 빈 배열이므로 그대로 반환하게 되는 것이죠!

성능 실험

useMemo를 적용하지 않은 경우 modal을 열거나 닫을 때 이를 참조하는 컴포넌트들이 리렌더링됩니다.
왼쪽 이미지를 보시면 RoomSettingHeader와 ReadyMembersContainer에서 show 함수를 참조하고 있기 때문에 리렌더링이 발생하였습니다.
이와 달리 useMemo를 적용한 오른쪽 이미지는 Provider만 리렌더링된 것을 확인할 수 있습니다.

useMemo X / useMemo O

useMemo 적용x useMemo 적용o

useCallback

useCallback

다른 사람들이 안 알려주는 리액트에서 Context API 잘 쓰는 방법 - velopert

Copy link
Contributor

Choose a reason for hiding this comment

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

이렇게 성능 실험한 내용을 보여주니 useMemo를 왜 사용했는지 확실히 와 닿는 것 같아요 !!!!! useCallback에 대한 코멘트도 남겼는데 직접 적용해 보시고 알려주셔서 정말 따봉입니다요 🤭👍👍👍

Copy link
Contributor

@novice0840 novice0840 left a comment

Choose a reason for hiding this comment

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

Modal이 깔끔하게 분리가 잘 되었네요!!
반복되는 코드들이 있어 코드 양에 비해 리뷰할게 적었던 것 같아요. 크게 신경쓰이는 부분은 없었어서 이번에는 바로 approve 하겠습니다

Comment on lines +46 to +47
onError: (error: CustomError) => {
showModal(AlertModal, { title: '방 초기화 에러', message: error.message });
Copy link
Contributor

Choose a reason for hiding this comment

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

🌸 칭찬 🌸

이번 식으로 구현이 되는 군요. 전역 변수에 컴포넌트 자체를 담는 건 처음보네요

Comment on lines 13 to 35
it('시작 버튼을 클릭했을 때, 게임 시작 API에서 에러가 발생하면 알림 모달이 뜬다.', async () => {
const user = userEvent.setup();
server.use(
http.patch(MOCK_API_URL.startGame, async () => {
return HttpResponse.json(
{
errorCode: 'NOT_READY_ROOM',
message: ERROR_MESSAGE.NOT_READY_ROOM,
},
{ status: 400 },
);
}),
);

customRenderWithIsMaster(<StartButton />, true);

const startButton = await screen.findByRole('button', { name: '시작' });
await user.click(startButton);

await waitFor(() => {
const closeIcon = screen.getByAltText('닫기 버튼');
expect(closeIcon).toBeInTheDocument();
});
Copy link
Contributor

Choose a reason for hiding this comment

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

🌸 칭찬 🌸

꼼꼼한 테스트 좋네요 ㅎ ㅎ

Comment on lines -9 to +10
message: string;
onConfirm?: () => void;
message?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

💭 질문 💭

메시지를 필수에서 옵션으로 바꿨는데 "메세지 없는 모달"이 있을 수 있을거라 생각해서 바꾼건가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RoomSettingModal은 message가 없더라구요 그래서 유연하게 받기 위해 옵셔널로 설정했습니다!

Comment on lines +32 to +35
await waitFor(() => {
const closeIcon = screen.getByAltText('닫기 버튼');
expect(closeIcon).toBeInTheDocument();
});
Copy link
Contributor

Choose a reason for hiding this comment

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

🙏 제안 🙏

Modal이 있는 여부를 확인하는 로직이 중복되는 것 같아서 이 부분을 검색해 봤더니 네 곳에서 사용되고 있더라고요.
공통 함수로 묶는건 어떠한가요?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오호 중복해서 사용되고 있긴 하네용 다만 개인적으로 중복을 줄이는 것도 좋지만 가독성과 직관적인 것도 중요하다고 생각합니다! 그래서 expect문을 중복을 줄이기 위해 추상화하는 게 테스트 코드의 유지보수를 용이하게 만들지는 고민해봐야겠네용
함수명을 잘 지어도 그 안에 어떤 expect문이 있는지는 한번더 확인해야 알 수 있다고 생각합니다. 추후에 프로덕션 코드가 변경되었을 때 찾기 어려울 거라고 생각이 들었어요! 어떻게 생각하시나요??

it('방장은 활성화 되어 있는 "다음" 버튼이 화면에 보인다.', async () => {
renderWithRecoilState(true);
customRenderWithIsMaster(<NextRoundButton />, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

🙏 제안 🙏

customRenderWithIsMaster라는 이름을 봤을 때 "이미 이 함수는 isMaster가 true로 설정되어 있겠구나" 라고 생각할 수 있을 것 같은데
두 번째 인자를 true를 기본값으로 넣어서 "customRenderWithIsMaster(<NextRoundButton )" 라고 해도 충분히 master가 true라고 유추가 가능할 것 같아요.
아니면 함수를 2개로 분리해서 "customRenderWithIsMaster" 이랑 "customRenderWithIsNotMaster"으로 나눠도 괜찮을 것 같은데 어떻게 생각하시나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오호 그렇게 생각할 수도 있겠군용
저는 jest 함수를 참고해서 작성했어요! with 라는 전치사가 파라미터를 의미한다고 생각했고요!
그래서 "isMaster라는 인자로 customRender를 호출했다" 가 함수의 의미가 되는 것이고, 자연스럽게 isMaster의 값을 인자로 받게 되었습니다. 예시로는 customRenderWithUserId 라고 한다면, userId를 인자로 받는다고 생각할 것 같아요! 그리고 개인적인 생각으로는 함수를 2개로 나누는건 불필요할 것 같아요:) 어떻게 생각하시나요??

https://jestjs.io/docs/expect#tohavebeencalledwitharg1-arg2-

onError: () => {
showModal();
onError: (error: CustomError) => {
showModal(AlertModal, { title: '방 참여 에러', message: error.message });
},
});
Copy link
Contributor

Choose a reason for hiding this comment

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

💭 질문 💭

위쪽 useMutation는 error는 타입이 없고 아래쪽에는 타입이 있는 것 같은데 의도된 부분인가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

엇 의도한 건 아닙니다 ㅎㅎ 실수네요! 저는 mutaiton 타입을 따로 작성하지 않았었는데, 이 함수만 명시되어 있더라구요
근데 API 반환타입이랑 mutation, query 반환 타입을 한번 다 통일해야할 것 같긴 합니다
CustomError 타입을 사용할 거면 다 명시하는 것도 좋아보이네용
타입을 명시하는 것을 선호하는 편인가요??

Comment on lines +35 to +51
const show = (Component: React.FC<ModalState> | null, props?: ModalProps) => {
setModal({
Component,
title: props?.title,
message: props?.message,
onConfirm: props?.onConfirm,
isOpen: true,
});
};

const close = () => {
setModal((prev) => ({
...prev,
Component: null,
isOpen: false,
}));
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🌸 칭찬 🌸

이런 식으로 하면 컴포넌트 파일의 JSX 부분에 불필효하게 Modal JSX 없어도 되겠군요

Copy link
Contributor Author

@rbgksqkr rbgksqkr left a comment

Choose a reason for hiding this comment

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

코멘트 완료했습니다:) 답변 부탁드려용

const AlertModal = ({ isOpen, onClose, onConfirm, message, title }: AlertModalProps) => {
const handleClick = () => {
onConfirm && onConfirm();
onClose();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

굿굿 만약 확인버튼과 닫기버튼이 별도의 UI로 존재한다면 수정해야할 것 같긴 하네용

<br />
</Fragment>
))}
{message &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

message가 없는 모달도 있어서 optional로 타입을 지정했더니 꼼꼼한 사람이 되었네요 ㅎㅎ

import { customRenderWithIsMaster } from '@/utils/test-utils';

describe('Header 테스트', () => {
it('방 설정 버튼을 클릭했을 때, 방 설정 모달이 뜬다.', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

리팩토링했다가 안되면 안되니까 테스트 코드 작성 후 리팩토링 진행하였습니다:)

if (dispatch === null) {
throw new Error('ModalDispatchContext가 존재하지 않습니다.');
}
return dispatch;
};
export default useModal;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

오호 useModal이니까 Modal 자체의 동작이라고 생각할 수도 있겠네용
저는 해당 커스텀훅을 사용하는 입장에서 바라봤습니다! 여러 곳에서 사용되는 훅이 아니라고 하셨는데, 현재 코드에서는 여러 곳에서 사용되고 있다고 생각합니다!

만약 Modal 컴포넌트나 Provider 내부적으로만 동작한다면 Modal 근처에 위치시키는 게 좋다고 생각하지만, 현재 코드에서는 useToast와 마찬가지로 사용처가 범용적이라 생각해서 hooks 디렉토리에 위치시켰습니다.
useModalEscClose, useDisableBackgroundScroll 은 모달 컴포넌트를 사용하는 곳에선 모두 동작하지만, useModal이 Modal 컴포넌트 내에서 동작하는 훅이 아니라고 생각하였습니다!

썬데이의 생각은 어떤가요 !!!

Comment on lines -9 to +10
message: string;
onConfirm?: () => void;
message?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

RoomSettingModal은 message가 없더라구요 그래서 유연하게 받기 위해 옵셔널로 설정했습니다!

it('방장은 활성화 되어 있는 "다음" 버튼이 화면에 보인다.', async () => {
renderWithRecoilState(true);
customRenderWithIsMaster(<NextRoundButton />, true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

오호 그렇게 생각할 수도 있겠군용
저는 jest 함수를 참고해서 작성했어요! with 라는 전치사가 파라미터를 의미한다고 생각했고요!
그래서 "isMaster라는 인자로 customRender를 호출했다" 가 함수의 의미가 되는 것이고, 자연스럽게 isMaster의 값을 인자로 받게 되었습니다. 예시로는 customRenderWithUserId 라고 한다면, userId를 인자로 받는다고 생각할 것 같아요! 그리고 개인적인 생각으로는 함수를 2개로 나누는건 불필요할 것 같아요:) 어떻게 생각하시나요??

https://jestjs.io/docs/expect#tohavebeencalledwitharg1-arg2-

onError: () => {
showModal();
onError: (error: CustomError) => {
showModal(AlertModal, { title: '방 참여 에러', message: error.message });
},
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

엇 의도한 건 아닙니다 ㅎㅎ 실수네요! 저는 mutaiton 타입을 따로 작성하지 않았었는데, 이 함수만 명시되어 있더라구요
근데 API 반환타입이랑 mutation, query 반환 타입을 한번 다 통일해야할 것 같긴 합니다
CustomError 타입을 사용할 거면 다 명시하는 것도 좋아보이네용
타입을 명시하는 것을 선호하는 편인가요??

}));
};

const dispatch = useMemo(() => ({ show, close }), []);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Context API memoization (useMemo vs useCallback)

불필요한 리렌더링을 방지하고자 작성하신 걸까요?

넵 맞습니다! velopert 님이 작성한 블로그글을 참고하여 "그럴 것 같다" 라고만 생각했는데, 코멘트가 들어온김에 직접 측정하여 useMemo를 적용하지 않은 경우와 적용한 경우를 비교해봤습니다!

useMemo를 사용하기 때문에 처음 생성한 dispatch 객체에 있는 show와 close 함수는 최초 생성된 함수이고 show와 close 함수는 각각 useCallback을 사용하지 않아 매번 새로운 함수를 참조할텐데 여기서 발생하는 문제는 없을까요?

심신미약 상태라 문장을 제대로 이해하지는 못했지만 메모이제이션이 제대로 되지 않을 것을 우려한 것 같아요 ㅎㅎ
useMemo는 첫번째 인자로 들어오는 함수의 반환값을 리렌더링 간에 캐싱하는 훅, useCallback은 첫번째 인자로 들어오는 함수 자체를 리렌더링 간에 캐싱하는 훅입니다!

useCallback은 테스트해보니 큰 효과가 없었어요! 궁금해서 이유를 생각해보니 setState가 발생하면 리렌더링이 발생하여 컴포넌트 내의 코드가 다시 실행될텐데, dispatch가 매번 새로운 객체를 만드니까 참조하는 컴포넌트도 리렌더링이 일어나는 것으로 보입니다! context로 내려주는 dispatch는 useMemo로 반환값인 { show, close } 를 캐싱한 것이고, 의존성 배열이 빈 배열이므로 그대로 반환하게 되는 것이죠!

성능 실험

useMemo를 적용하지 않은 경우 modal을 열거나 닫을 때 이를 참조하는 컴포넌트들이 리렌더링됩니다.
왼쪽 이미지를 보시면 RoomSettingHeader와 ReadyMembersContainer에서 show 함수를 참조하고 있기 때문에 리렌더링이 발생하였습니다.
이와 달리 useMemo를 적용한 오른쪽 이미지는 Provider만 리렌더링된 것을 확인할 수 있습니다.

useMemo X / useMemo O

useMemo 적용x useMemo 적용o

useCallback

useCallback

다른 사람들이 안 알려주는 리액트에서 Context API 잘 쓰는 방법 - velopert

Copy link
Contributor

@useon useon left a comment

Choose a reason for hiding this comment

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

이미지와 읽어보면 좋을 아티클까지 첨부하신 정성스런 답변 최고입니다요 .. 🤭 코멘트 확인 후 어프루브 드립니다 !!! 고생많았숨니다 ^___^ 이거 머지되면 제 파트 에러 처리하려 합니다 두근세근..

@rbgksqkr
Copy link
Contributor Author

[ 추후 개선 사항 ]

  • mutation 또는 query에 타입 모두 명시할 것인지?
    • 할거면 다 명시하고, 안하면 다 빼는 게 맞을 것 같아요!

@rbgksqkr rbgksqkr merged commit 502d6dc into develop Sep 26, 2024
2 checks passed
@rbgksqkr rbgksqkr changed the title [REFACTOR] 모달을 Context로 관리하여 사용성 개선 [REFACTOR] Modal을 Context API를 활용하여 사용성 개선 Oct 29, 2024
@rbgksqkr rbgksqkr changed the title [REFACTOR] Modal을 Context API를 활용하여 사용성 개선 [REFACTOR] Context API를 활용하여 Modal 사용성 개선 Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🫧 FE front end ♻️ refactor 리팩토링 ✅ test 테스트 관련
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[REFACTOR] 모달을 Context로 관리하여 사용성 개선
3 participants