-
Notifications
You must be signed in to change notification settings - Fork 2
[ Refactor ] home 화면 QA #483
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
base: main
Are you sure you want to change the base?
Conversation
🚀 CI Check Results
|
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.
전반적으로 잘 고쳐진 것 같네요.
댓글 입력 컴포넌트 스타일 조정
이 부분에서 질문 주신 내 정보 가져오기는 그렇게 하시는 게 맞습니다.
NextAuth 및 로그인 server action으로 next 서버에 로그인 정보를 저장하고 클라이언트에도 그 정보를 줘서 context api(useSession)에 저장해요. 클라 측에서 직접 알고헙 서버에 api 요청할 때나 사용자 정보를 얻기 위해 매 번 next 서버에서 로그인 정보를 받아오는 비효율적인 방식보다 useSession으로 저장해 둔 데이터를 사용하는 게 낫기 때문이에요. 참고로 알고헙 jwt가 갱신 되거나 사용자 정보가 바뀌면 useSession 메서드 중 update({ ... })를 통해 데이터를 갱신하도록 해 놨습니다.
댓글 최신 3개 보이게
댓글을 두 뷰 모두 과거순으로 보여주고, 댓글을 최신순으로 보여줄 뷰는 없을거라 판단해 라고 PR에 적으셨는데 아마 과거순과 최신순을 반대로 쓰신 거겠죠?
코드는 잘 수정하신 것 같아 따로 의견은 없습니다.
그 외
revalidation은 판단하신 대로 삭제하는 게 맞네요. 페이지가 아니라 스터디 목록과 추천 스터디를 가져오는 fetch에 부분적으로 넣어야겠어요.
page에 revalidate를 넣는 full route cache와 달리, fetch의 option으로 next: { revalidate... }를 넣는 data cache는 dynamic 페이지 여부와는 관계없이 fetch 결과 자체를 캐싱하는 것이므로 의도에 맞는 사용이라 생각하기 때문이에요. 처음 의도는 변경 주기가 긴 스터디 리스트와 추천 스터디 데이터는 캐싱해서 쓰는 게 좋겠어서 사용했던 건데 덕분에 그동안 잘못 쓰고 있었다는 걸 알았네요. 이 부분의 수정은 따로 티켓을 만들어서 제가 처리할게요.
✅ Done Task
☀️ New-insight
매우 엉망으로 구현을 해서 고칠부분이 많았네요..
빠르게 다 고쳐서 정상적인 뷰로 구현해두겠습니다!
💎 PR Point
댓글 입력 컴포넌트 스타일 조정
간격이 안맞고, 풀이를 올린 사람의 프로필이 댓글 입력 창에 뜨고있어 useSession 훅 활용해 수정해주었습니다.
근데 내 정보를 이렇게 가져오는게 맞는지? 다른 방법이 있으면 알려주시면 감사할거 같아요.
댓글 최신 3개 보이게
댓글이 최신 3개가 아닌, 첫댓글 부터 3개가 보이게 로직이 구현되어있어 이를 최신 3개로 수정해주었습니다.
수정하고 보니 댓글 더보기의 위치가 밑이 아니라 위에 있어야 더 자연스러울거 같단 생각도 들어 디자이너분과 상의해볼게요.
근데 이상하게 댓글 더보기 모달을 들어갔다 나오면 댓글의 순서가 바뀌는 버그가 있었습니다.
원인을 추적해보니 모달 내에서 comments.sort() 로직이 있어 쿼리 캐시 자체를 바꾸어 버리고 있었습니다.
(주용아... 덕분아 1시간 날렸다..)
이를 예방하고 관심사를 명확하게 하기 위해 queryObject에서 select 옵션에서
toReverse메서드 사용해 원본배열 안건드리게 헤주었어요.댓글을 두 뷰 모두 과거순으로 보여주고, 댓글을 최신순으로 보여줄 뷰는 없을거라 판단해 queryObject 자체에 넣어주었습니다.
[user] 페이지에
revalidation이 걸려있어서 확인해보니 어차피 dynamic 페이지라 의미가 없는 코드라 생각해 삭제해주었어요.혹시 꼭 필요한 코드면 알려주세요!
피드 순서와 뜨는 기준도 손봐야 하는데 이건 서버분과 의논해서 추후 다른 pr에 올릴게요.
📸 Screenshot