-
Notifications
You must be signed in to change notification settings - Fork 2
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
[#256] Feat: 쿼리 팩토리 적용 및 네트워크 워터폴 최적화 #263
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 개인적으로 get
쿼리에 해당하는 모든 훅을 제거해도 되지 않을까 싶습니다! query factory에 queryKey
와 queryFn
등 쿼리를 핸들링하는 로직이 들어가 있으므로, 이를 사용하는 곳에서 직접 필요한 query Factory의 키를 꺼내 사용하면 되지 않을까 싶어요.
대부분의 훅들이 data 속성을 이름만 변경하여 리턴 시켜주고 있습니다. 이는 레이어가 한층 더 늘어나는 작업으로 보여 복잡해진다는 생각이 들어요! 그래서 저는 커스텀훅을 제거하고, 사용하는 곳에서 query Factory를 호출하여 레이어를 한층 줄이는 것이 좋다고 생각하는 데 용재님 생각은 어떠신가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const AdminImageDetail = ({ imageId }: Props) => {
const { deleteReportedZzal } = useDeleteReportedZzal();
const deleteConfirmOverlay = useOverlay();
// const { reportDetails } = useGetReportsDetails(imageId); 기존 코드
const { data: reportDetails } = useSuspenseQuery(reportQueries.report(imageId)); // 변경된 코드
/* 이하 코드 생략 */
}
위 코드는 관리자 계정에서 신고된 이미지 리스트의 이미지를 자세히볼 때 나타나는 컴포넌트로 useGetReportsDetails
훅을 호출하여 사용하고 있으며, useGetReportsDetails
내에 구현된 로직은 hooks/api/report/queryKeyFactories
에 모두 구현되어 있습니다. hooks/api/report/queryKeyFactories
에 메소드로 구현된 로직들이 저는 하나의 유틸? 훅? 처럼 사용한다고 생각해서 useGetReportsDetails
훅이 따로 존재하지 않아도 생각합니다. 따라서 useGetReportsDetails
훅이 따로 존재하는 것 자체가 하나의 레이어가 더 존재한다고 생각이되고, 재사용성 측면에서도 hooks/api/report/queryKeyFactories
내에 정의한 로직을 재사용하면 되므로 재사용성 측면에서도 문제가 없다고 생각합니다 ㅎㅎ
@@ -40,7 +40,8 @@ const Home = () => { | |||
imageId={imageId} | |||
isLiked={imageLikeYn} | |||
imageIndex={index} | |||
queryKey={["homeZzals", selectedTags]} | |||
type="home" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 지금과 같이 키를 사용 해야하는 상황에서 queryFactories
가 활용해야 한다고 생각합니다! 휴먼 에러 예방 + 조금 더 명확한 키 타입을 위해 hooks/api/zzal/queryKeyFactories
에서 꺼내오는게 어떨까요?
지금까지의 구현 상황을 보니 여기서 키를 queryFactories
에서 꺼내오면 연쇄적으로 타입부터 시작해서 바뀌어야 할 부분들이 있을 것 같아서 용재님의 의견이 궁금합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위 내용에 대한 예시를 말씀드리면, hooks/api/zzal/queryKeyFactories
에 쿼리 함수
와 쿼리키
과 관리되고 있습니다.
해당 파일에는 myLikedZzals
, homeZzals
, myUploadedZzals
키 관련 메소드가 정의가 되어있고, 해당 파일과 같은 상황에서는 homeZzals: () => [...zzalQueries.all(), "home"],
을 직접 호출하여 type={zzalQueries.homeZzals()}
와 같이 작성한다면 개발자로 하여금 직접 문자열을 입력하는 것보다 humman error를 줄일 수 있지 않을까 생각합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM🚀
+ close 이슈번호가 빠졌네요!
[#256] Feat: 쿼리 팩토리 적용 및 네트워크 워터폴 최적화
📝 작업 내용
PR [#254] Modify: query factory 수정 에서 수정된 쿼리 팩토리를 프로젝트 전역에 적용하는 작업을 진행했습니다
server 및 client에서 발생하는 네트워크 워터폴을 최적화 하는 작업을 진행했습니다
📷 스크린샷 (선택)
💬 리뷰 요구사항(선택)
클라이언트 사이드에서 발생하는 주요 네트워크 워터폴은
Home.tsx
,MyLikedZzals.tsx
,MyUploadedZzals.tsx
으로 판단됩니다. 모두 다수의 data fetching을useSuspenseQuery
또는useSuspenseInfiniteQuery
를 사용하여 진행됩니다. 컴포넌트를 suspend하는 함수를 사용하기 때문에 각 data fetch는 직렬적으로 진행됩니다.useSuspenseQueries를 사용하는 방법도 있지만 3 경우 모두 infiniteQuery가 포함되어 데이터 작업이 복잡해져서 보류했습니다.
tag관련 data fetch를
useQuery
를 사용해서 하는 방법을 고민해 보고 있습니다. 대신, 해당 api의 data에 의존적인useEffect
문이 있어 의도치 않은 side effect가 발생할 우려가 있어 보류했습니다.📍 기타 (선택)
close #256