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

refactor(my-page): my-page suspense 적용 및 root page server component로 변경 #392

Draft
wants to merge 8 commits into
base: release/routing
Choose a base branch
from

Conversation

chaaerim
Copy link
Member

💡 왜 PR을 올렸나요?

  • toast와 userInfo에 parallel route를 적용하여 my-page의 root page를 server component로 변경했습니다.
  • react query v5의 useSuspenseQuery를 이용해서 userInfo에 suspense를 적용했습니다.
  • ssr 환경 대응을 위해 util/http 내부에서 window 사용시 window가 undefined인 경우 얼리 리턴 해주었습니다.

💁 무엇이 어떻게 바뀌나요?

  • 디자인 레퍼런스:
    화면 기록 2024-01-19 오후 5 07 50

💬 리뷰어분들께

  • 카테고리 bottomsheet 부분 무한 에러 나는 곳 주석처리하고 작업을 해서 아마 ci 통과를 못할 것 같은데 일단 코드 먼저 보시면 좋을 것 같아 Pr 올립니다~~ 리뷰 먼저 남겨주시면 같이 보고 수정해둘게요 !!!
  • 위키도 곧 업로드 해두겠습니다 ㅎ.ㅎ

@LineGu LineGu enabled auto-merge (squash) January 19, 2024 08:31
Comment on lines +95 to +97
if (typeof window === 'undefined') {
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

redirectToLoginPage함수가 혹시나 서버 컴포넌트에서 호출될까 염려하여 얼리리턴 해준건가요?
ssr대응이라는게 어떤 문제가 있어서 추가하게 되었는지 궁금합니다..!

Copy link
Collaborator

Choose a reason for hiding this comment

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

아 실행 시켜보니 window를 참조하는 과정에서 에러가 발생하는군요

Copy link
Member Author

Choose a reason for hiding this comment

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

@dengoyoon 네네 맞아요!! 서버에서 렌더 될 때에는 window 참조를 못해서 서버 렌더되는 경우 아예 리턴 시켜서 에러 발생을 막기 위한 코드였습니다

@codecov-commenter
Copy link

Codecov Report

Attention: 172 lines in your changes are missing coverage. Please review.

Comparison is base (b21ab65) 0.08% compared to head (a6e239b) 0.07%.

Files Patch % Lines
src/app/(BackHeader)/my-page/@userInfo/page.tsx 0.00% 53 Missing ⚠️
src/app/(BackHeader)/my-page/@toast/page.tsx 0.00% 33 Missing ⚠️
...er)/my-page/_components/GoogleFormButton/index.tsx 0.00% 23 Missing ⚠️
...my-page/_components/SkeletonUserInfo.tsx/index.tsx 0.00% 23 Missing ⚠️
src/app/(BackHeader)/my-page/layout.tsx 0.00% 22 Missing ⚠️
...er)/my-page/_lib/hooks/useSuspenseUserInfoQuery.ts 0.00% 10 Missing ⚠️
src/app/(BackHeader)/my-page/page.tsx 0.00% 3 Missing ⚠️
src/common/utils/http.ts 0.00% 3 Missing ⚠️
src/app/(AppBarHeader)/layout.tsx 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                 @@
##           release/routing    #392      +/-   ##
==================================================
- Coverage             0.08%   0.07%   -0.01%     
==================================================
  Files                  171     176       +5     
  Lines                 6166    6254      +88     
  Branches               171     176       +5     
==================================================
  Hits                     5       5              
- Misses                6161    6249      +88     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@minsgy minsgy disabled auto-merge January 20, 2024 15:01
Copy link
Collaborator

@dengoyoon dengoyoon left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 !

@minsgy minsgy marked this pull request as draft February 20, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants