[Refactor] ApplyPage 로직 분리 - 1 #441
Conversation
|
eonseok-jeon
changed the title
| const handleShowDialog = () => { | ||
| ref.current?.showModal(); | ||
| }; | ||
|
|
||
| const handleCloseDialog = () => { | ||
| ref.current?.close(); | ||
| }; |
There was a problem hiding this comment.
저는 항상 커스텀 훅에서 반환하는 handler에게는 useCallback을 붙혀주는 편인데, 해당 커스텀 훅을 사용하는 컴포넌트 쪽에서 최적화할 수 있는 여지를 줄 수 있기 때문입니다 !
커스텀 훅에서 반환하는 핸들러를 의존성에 주입하더라도, useCallback을 통해 메모제이션해주었기 때문에 불필요한 렌더링을 방지할 수 있다는 장점이 있을 것 같습니다 !
There was a problem hiding this comment.
오홍 좋은 거 같아요~
근데 useCallback을 이용하는 것도 매번 이전과 달라진 게 있는지 비교를 해줘야 해서 리소스가 발생되는 작업인데 붙여주는게 항상 이득인가요??
There was a problem hiding this comment.
음 ... 사실 저도 항상! 이라는 말에는 자신있게 대답할 수 있지는 못하는 것 같습니다. 언석님 말씀대로 메모이제이션 또한 비용이니까요 !
하지만 리액트 공식문서에서도 useCallback을 커스텀 훅에 적용하여 의존성 비교 시 렌더링 최적화를 권장하는 것처럼, useDialog와 같은 자주 사용되는 커먼 훅에 적용한다면 이점이 더 많을 것 같습니다 !
There was a problem hiding this comment.
제 갠적인 의견을 남겨보자면, , ,
useCallback 이 친구가 효과를 보여줄 수 있는 경우는 복잡한 연산이 반복적으로 일어날때 -> 근데 이런 케이스는 사실 많이 없고, 대부분 주용님이 말씀하신 것처럼 의존성에 들어갈때 라고 생각하는데요
그렇다면 만들어주는 모든 공통 훅에 useCallback을 적용해주기보다, 디폴트로 일반적인 함수로 작성하구
이후 핸들러를 의존성 배열에 주입시키게 될 경우 그때 추가적으로 useCallback으로 감싸주는 작업을 해주는게 좋지 않을까유??
이러면 불필요한 비용이 안생길 것 같아요
특히나 지금 같이 dialog를 껐/켰하는 핸들러들은
사용자 액션으로 인해 발생하는 이벤트시 실행되는 애들이기 때문에 더더욱 useEffect에서 사용될 일 없는, 즉 의존성 배열에 들어갈 일도 (아마도)없을 친구들이라고 생각해요!!
There was a problem hiding this comment.
음 ..
사실 특정 로직을 커스텀 훅으로 분리한다, 메모이제이션을 적용한다, 등등 다양한 부분에서 "일종의 배팅" ? 이 존재한다고 생각해요.
예를 들어서, 우리가 스크롤을 맨 위로 보내는 로직이 중복된다고 생각해서 useScrollToTop이라는 커스텀 훅을 만들었다고 가정해볼게요. 근데 이 커스텀 훅을 하나의 파일로 분리하고 보니 딱 한번만 사용이 되는거에요. 그렇다면 결국 리소스 낭비라고 생각이 들 수도 있어요. 즉 우리가 "중복된 로직을 하나의 훅으로 분리"한 이유는 이게 다른 곳들에서 많이 사용되면서 리소스를 줄여줄 수 있도록 "배팅"을 한다는 말이죠.
이 메모이제이션 또한 비슷하다고 생각합니다. 결국엔 다른 파일에서 최적화할 수 있다는 여지를 제공하면서 일종의 "배팅"을 하는 거라고 생각합니다. 저는 이 "여지"가 중요한 것 같아요. 리액트 공식문서에서도 말하듯이 결국엔 다른 컴포넌트들이 최적화할 수 있는 가능성을 열어주는 것이니까요. 만약 나중에 리렌더링이 되는 이유를 분석하여 결국에는 해당 커스텀 훅을 찾아가서 변경하는 리소스보다 분명 더 낮은 리소스를 투자하여 미래를 대비할 수 있다는 생각입니다.
승희님이 말씀해주신 것처럼, 단순 dialog를 껐다 켜는 핸들러도 마찬가지에요. 단순한 핸들러일수록, 변경되는 일이 없고, 따라서 메모이제이션을 적용해주더라도 최적화 입장에서 이점을 볼 수 있으면 있지 이로 인한 손실이 많다고는 못느끼겠어요. 의존성이 없으면 없을수록 !
There was a problem hiding this comment.
두 분 의견 다 이해했습니다!
사실 제가 useCallback에 대해 완전 깊게 이해하고 있지 못해서 이렇다 저렇다 할 수가 없는 거 같아요
그런 상황에서 바로 코드를 바꾸기 보다는 좀 더 공부해서 이해한 다음 그에 맞춰 코드를 수정하는 게 더 좋을 거 같단 생각이 들었어요!
이 부분에 대해서는 공부 완료한 후 자료랑 다 첨부해서 다시 제 의견 말씀드릴게요!!
There was a problem hiding this comment.
이것 저것 하다 보니 오래 걸렸네요 😓
제 의견 먼저 말씀드리면 useCallback을 사용할 필요는 없어 보입니다!
useCallback는 복잡한 함수의 경우 불필요한 함수 생성과 하위 컴포넌트의 재렌더링을 막기 위해서 사용이 되는데요
- useDialog 함수들이 복잡한 기능을 가지는가? -> x
- useDialog의 handler 함수를 prop으로 가지는 자식 컴포넌트들이 최적화가 필요할 만큼 무거운가? -> x
둘 다 해당하지 않아 사용하지 않는 것으로 결정했어요
실제로도 useCallback을 적용하고 안 하고 차이를 비교했을 때, 적용했을 때가 0.2ms 정도 빨리 렌더링 되는 것으로 확인이 되었는데요
이는 0.0002초로 정말 미비한 차이이므로 useCallback을 사용한다 해도 사실 유의미하지는 않음을 확인할 수 있었어요!
useCallback 적용 전
useCallback 적용 후
useCallback – React
javascript - useCallback vs simple function - Stack Overflow
javascript - What is useCallback in React and when to use it? - Stack Overflow
물론 주용님 말씀대로 위험을 예방하자는 건 너무 좋은 거 같아요!! 저 역시도 확장에 유연한 개발을 지향하거든요 ㅎㅎ 하지만 현재 dialog와 관련해서 추가 기능이 개발될 가능성은 조금 낮을 것으로 예상이 돼요 애초에 지원서 자체에 추가 기능이 더 발생하지 않을 가능성이 높거든요! 그래서 앞으로도 useCallback을 추가함으로써 유의미한 결과를 나타나진 않을 것으로 예상됩니다!
추가로 제 의견을 덧붙이자면
필요없는 코드는 사실 존재하지 않는 게 가장 베스트라고 생각해요
유지보수할 때도,
나중에 저희 후임자가 와서 코드를 읽을 때도 말이죠
실제로 react 공식문서에서도 useCallback을 많이 사용하면 가독성이 떨어질 수 있다고 주의하고 있어요
그래서 추가하지 않기로 결정했습니다!!
이대로 가는 것에 괜찮으시다면 이모지 달아주세요~ 두 분 다 확인되면 머지할게요!
추가로 더 하실 말 있으시면 편하게 comment 남겨주세요 :)
There was a problem hiding this comment.
완전 동의합니다 ~~
구체적인 의견 제시 감사드려요 ㅎㅎ 🚀 🚀 🚀 🚀
| const handleShowDialog = () => { | ||
| ref.current?.showModal(); | ||
| }; | ||
|
|
||
| const handleCloseDialog = () => { | ||
| ref.current?.close(); | ||
| }; |
There was a problem hiding this comment.
음 ... 사실 저도 항상! 이라는 말에는 자신있게 대답할 수 있지는 못하는 것 같습니다. 언석님 말씀대로 메모이제이션 또한 비용이니까요 !
하지만 리액트 공식문서에서도 useCallback을 커스텀 훅에 적용하여 의존성 비교 시 렌더링 최적화를 권장하는 것처럼, useDialog와 같은 자주 사용되는 커먼 훅에 적용한다면 이점이 더 많을 것 같습니다 !
Related Issue : #440
🧑🎤 Summary
apply page 내에서
🧑🎤 Comment
💬 페이지 이탈 시 alert 띄우기 custom hook 분리
관심사 분리하려고 따로 분리해줬습니다 :)
💬 dialog custom hook 분리
dialog 관련 함수들이 모여있는 hook을 생성해줬어요
분리하는 김에 다른 페이지에 있는 dialog들도 다 분리해줬습니다 :)
💬 isReview 전역 변수로 분리
변하지 않는 변수라 component 내에 있을 필요가 없어 보였어요
외부로 빼줬습니다
💬 loading 전역에서 관리
에 Suspense로 fallback을 제공하고 있어서 제거해 줬습니다 :)