-
Notifications
You must be signed in to change notification settings - Fork 4
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
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.
역시는 역시 역시네요
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.
고생하셨습니다 Approve 할게요!
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.
피드백이 생각보다 좀 있네요 ㅎㅎ;;
일단은 배포 해야되니 approve 먼저하고, 데모데이 이후 피드백 천천히 반영해주시면 좋을 것 같아요.
고생하셨습니다~!
frontend/src/utils/CacheStorage.ts
Outdated
@@ -0,0 +1,27 @@ | |||
class CacheStorage { | |||
#storage = new Map(); |
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.
여기 # 대신 private 붙이면 좋을 것 같아요
frontend/src/utils/CacheStorage.ts
Outdated
if (!key || !this.hasKey(key.join(' '))) return null; | ||
return this.#storage.get(key.join(' ')) as T; |
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.
key.join() 연산을 불필요하게 두 번 하는데, 변수 선언해서 쓰면 좋을 것 같아요.
요즘 알고리즘 풀다보니 이런 연산 줄여야 할 것들이 잘 보이네요 ㅋㅋ
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.
그러게요! 같은 로직이 두 번있네요! 변수로 만들어보겠습니다:)
private delete = (key: string) => this.#storage.delete(key); | ||
} | ||
|
||
const cacheStorage = new CacheStorage(); |
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.
클로저 같은거 활용해서 함수로 해도 될 것 같은데, 싱글톤 클래스를 사용하신 이유가 따로 있을까요?
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.
해당 로직을 구현할 때 가장 먼저 떠오른 건 class에요. 전 class가 더 쉽게 와 닿는 느낌이 더 크구요. 이후에 클로저로도 한 번 구현하고 이를 비교해서 반영여뷰를 생각해보겠습니다 :)
onError, | ||
}); | ||
|
||
const cacheData = cacheStorage.find<T>(cacheKey); |
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.
렌더링 될 때마다 이 연산을 계속 수행할 것 같아요!
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.
cacheFetch
함수에서 값을 가져오도록 했습니다 :)
|
||
const useCacheFetch = <T>( | ||
request: () => Promise<T>, | ||
{ errorBoundary = true, cacheKey, cacheTime, onSuccess, onError, isRunLater }: Options<T>, |
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.
isRunLater이랑 기존 훅의 enabled랑 비슷한 것 같은데, enable로 통일화 하는건 어떨까요?
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]); |
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.
이거는 MemberInfo와 관련 없는 로직이라 다른 고차 컴포넌트를 만들어서 사용하는게 더 좋을 것 같아요~
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.
Profile 뱃지에서 preFetch를 할 수 있도록 했습니다!
관련 이슈
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에 저장할 수 있어요. 이를 다음과 같은 곳에 적용했습니다.
5. 스터디 정보 modal 버튼 제거
기존에 2개였던 modal 닫기 버튼을 하나 제거했습니다.
전체적인 코드 리팩터링은 레벨 5때, 진행할 예정입니다. 조금더 깔끔하게 코드를 가져가고 싶네요 😭