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

[FE] 멤버, 스터디 기록 Cache 적용 #686

Merged
merged 23 commits into from
Oct 19, 2023

Conversation

nlom0218
Copy link
Collaborator

@nlom0218 nlom0218 commented Oct 19, 2023

관련 이슈

closed #653

구현 기능 및 변경 사항

1. 달력 스켈레톤 제거 및 CircularProgress 추가

달력 스켈레톤이 계속 보니 어색하더라고! 그래서 이를 제거하고 CircularProgress를 추가했습니다.

2. CacheStorage 생성

브라우저에서 제공하는 CacheStorage를 사용하고자 했는데, 이는 url를 필요로 하는 것을 파악했어요.(이 부분은 추후에 더 알아볼려고 합니다.) 때문에 map객체를 활용하여 CacheStorage 클래스를 만들었고 해당 인스턴스를 export하여 useCatchFetch 훅, usePreFetch 훅 에서 사용하고 있습니다.

3. useCacheFetch 훅

기존에 있는 useMutation훅을 활용하여 useCacheFetch 훅을 만들었어요. 때문에 suspense를 사용하지 못하는 아쉬움이 있습니다. 이를 추후에 개선을 해보고자 해요.

이 훅은 달력조회, 기간조회, 스터디조회, 멤버조회, 멤버의 기록 조회에 사용됩니다.

달력조회와 기간조회의 cache 시간은 1시간이에요. 최소 진행 시간이 60분이기 때문이에요.

스터디조회, 멤버조회, 멤버의 기록 조회는 24시간으로 cache 시간을 정했어요. 이는 한 번 생성되고 나서 앞으로 바뀌지 않을 부분이기 때문이죠. 그래서 추후에 cache 시간을 아예 전달하지 않으면 앞으로 삭제되지 않도록 구현해야 할지 고민하고 적용할려고합니다. 추가로 reset 버튼도 제거했어요. 예전에는 참여자마다 진행도가 달라서 reset 버튼이 필요했지만 현재는 모두 같은 시점이 마무리 되기 때문에 제거했습니다.

4. usePreFetch 훅

해당 훅을 통해 사용자가 앞으로 조회할 데이터를 미리 불러온 후, cache에 저장할 수 있어요. 이를 다음과 같은 곳에 적용했습니다.

  1. 멤버 프로필 fetch 후, 이번 달 스터디 데이터 미리 불러오기
  2. 이전달, 다음달 자신의 스터디 데이터 미리 불러오기
  3. 다음 페이지 스터디 데이터 미리 불러오기

5. 스터디 정보 modal 버튼 제거

기존에 2개였던 modal 닫기 버튼을 하나 제거했습니다.


전체적인 코드 리팩터링은 레벨 5때, 진행할 예정입니다. 조금더 깔끔하게 코드를 가져가고 싶네요 😭

@nlom0218 nlom0218 linked an issue Oct 19, 2023 that may be closed by this pull request
1 task
@nlom0218 nlom0218 requested review from woo-jk and yeopto October 19, 2023 08:29
@nlom0218 nlom0218 self-assigned this Oct 19, 2023
@nlom0218 nlom0218 added FE feature refactor chore 개발 관련 기타 사항(코드 스타일) UI labels Oct 19, 2023
@nlom0218 nlom0218 added this to the 6차 스프린트 milestone Oct 19, 2023
Copy link
Collaborator

@woosung1223 woosung1223 left a comment

Choose a reason for hiding this comment

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

역시는 역시 역시네요

Copy link
Collaborator

@yeopto yeopto left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 Approve 할게요!

Copy link
Collaborator

@woo-jk woo-jk left a comment

Choose a reason for hiding this comment

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

피드백이 생각보다 좀 있네요 ㅎㅎ;;
일단은 배포 해야되니 approve 먼저하고, 데모데이 이후 피드백 천천히 반영해주시면 좋을 것 같아요.

고생하셨습니다~!

@@ -0,0 +1,27 @@
class CacheStorage {
#storage = new Map();
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기 # 대신 private 붙이면 좋을 것 같아요

Comment on lines 16 to 17
if (!key || !this.hasKey(key.join(' '))) return null;
return this.#storage.get(key.join(' ')) as T;
Copy link
Collaborator

Choose a reason for hiding this comment

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

key.join() 연산을 불필요하게 두 번 하는데, 변수 선언해서 쓰면 좋을 것 같아요.
요즘 알고리즘 풀다보니 이런 연산 줄여야 할 것들이 잘 보이네요 ㅋㅋ

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그러게요! 같은 로직이 두 번있네요! 변수로 만들어보겠습니다:)

private delete = (key: string) => this.#storage.delete(key);
}

const cacheStorage = new CacheStorage();
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.

해당 로직을 구현할 때 가장 먼저 떠오른 건 class에요. 전 class가 더 쉽게 와 닿는 느낌이 더 크구요. 이후에 클로저로도 한 번 구현하고 이를 비교해서 반영여뷰를 생각해보겠습니다 :)

onError,
});

const cacheData = cacheStorage.find<T>(cacheKey);
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

@nlom0218 nlom0218 Oct 19, 2023

Choose a reason for hiding this comment

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

cacheFetch 함수에서 값을 가져오도록 했습니다 :)


const useCacheFetch = <T>(
request: () => Promise<T>,
{ errorBoundary = true, cacheKey, cacheTime, onSuccess, onError, isRunLater }: Options<T>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

isRunLater이랑 기존 훅의 enabled랑 비슷한 것 같은데, enable로 통일화 하는건 어떨까요?

Comment on lines 40 to 57
useEffect(() => {
if (!memberInfo || memberInfo.loginType === 'guest') return;

const today = new Date();
const year = today.getFullYear();
const month = today.getMonth() + 1;

const [startDate, endDate] = calendar.getMonthFirstLastDate(year, month).map((date) => {
if (!date) return '';

return format.date(date.date, '-');
});

prefetch(() => requestGetMemberCalendarRecord(memberInfo.memberId, startDate, endDate), {
cacheKey: [startDate, endDate],
cacheTime: 60 * 60 * 1000,
});
}, [memberInfo, prefetch]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이거는 MemberInfo와 관련 없는 로직이라 다른 고차 컴포넌트를 만들어서 사용하는게 더 좋을 것 같아요~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Profile 뱃지에서 preFetch를 할 수 있도록 했습니다!

@nlom0218 nlom0218 merged commit 190e077 into develop Oct 19, 2023
3 checks passed
@nlom0218 nlom0218 deleted the fe/featrue/653-study-record-cache branch October 19, 2023 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore 개발 관련 기타 사항(코드 스타일) FE feature refactor UI
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[FE] 멤버, 스터디 기록 Cache 적용
4 participants