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

fix : useScrapToggle에 낙관적 업데이트를 위해 queryKey를 넘기는 방식으로 수정 #309

Merged
merged 6 commits into from
Nov 25, 2024

Conversation

smosco
Copy link
Collaborator

@smosco smosco commented Nov 25, 2024

📄 Description of the PR

🔧 What has been changed?

  • 기존에는 useScrpToggle에 target을 넘겨서 낙관적 업데이트를 했는데 쿼리키 자체를 받는 식으로 수정했습니다.
  • previewScrapButton 컴포넌트에서 경로로부터 쿼리키를 만드는 generateQueryKey 함수를 구현했습니다.
  • 학습 페이지에서 콘텐츠 목록 불러올 때 로딩 중이면 스피너를 보여줌
  • 포인트 필요한 리스닝 콘텐츠 외부 스크랩 불가능 하도록 수정 ( z-index 수정)

📸 Screenshots / GIFs (if applicable)

⚠️ Precaution & Known issues

✅ Checklist

  • UI 브랜치 같이 확인해서 이슈없는지 확인해보기
  • 함수 이름, 변수 이름만 봐도 어떤 기능을 하는지 파악할 수 있는지 (선언적인 코드인지 확인)

@smosco smosco self-assigned this Nov 25, 2024
@smosco smosco linked an issue Nov 25, 2024 that may be closed by this pull request
3 tasks
Comment on lines -47 to -50
// TODO(@smosco): 외부 스크랩 할 때 setQueryData 또는 invalidate 하기 위해 정확한 queryKey가 필요함
// null 값으로 오면 queryKey가 이상하므로 임시 방편으로 filter를 적용함
// 다만 나중에 sort, direction, categoryId가 들어오는 경우 순서가 유지 되지 않으므로 문제 발생
// 따라서 어디서든지 간에 default 값을 넘길 필요가 있음
Copy link
Collaborator

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.

네넴 이제 queryKey 자체로 변경해서 서치 컴포넌트에서도 할 수 있게 되었습니다~ 아직 추가는 안했지만요!

Comment on lines +92 to 110
queryClient.setQueryData(queryKey, (old: any) => {
if (old?.data?.contents && Array.isArray(old.data.contents)) {
return {
...old,
data: {
...old.data,
contents: old.data.contents.map((content: any) =>
content.contentId === contentId
? { ...content, isScrapped: !isScrapped }
: content,
),
},
};
}
return old;
});
break;

case 'paginatedListeningPreview':
Copy link
Collaborator

Choose a reason for hiding this comment

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

querykey[0]가 listeningPreview, readingPreview라는 타입을 구별하는 것이군요

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 +51 to +77
// 경로별 queryKey 구성
if (pathname === '/') {
// 메인 페이지
return contentType === 'READING'
? ['readingPreview']
: ['listeningPreview'];
}

if (pathname.startsWith('/learn')) {
// 학습 페이지
const type = pathname.includes('/reading')
? 'paginatedReadingPreview'
: pathname.includes('/listening')
? 'paginatedListeningPreview'
: null;

if (!type) {
throw new Error('Invalid path: unable to determine query type');
}

return [type, page, size, sort, direction, categoryId].filter(
(item) => item !== null,
); // 기본값(categoryId)이 null인 경우 제거
}

throw new Error('Invalid path: no matching query key logic');
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 코드는 optimistic update가 굳이 모든 경로에 다 적용될 필요없이 내가 현재 있는 그 화면에만 적용되면 되게 만들어주는 일종의 최적화 코드군요~

Copy link
Collaborator

@godhyzzang godhyzzang left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~

@godhyzzang godhyzzang merged commit 7555f4e into develop Nov 25, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[fix] toggle scrap bug
2 participants