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: middleware 오동작으로 생긴 배포 이슈 수정 및 리팩토링 (2차) #39

Merged
merged 14 commits into from
Feb 11, 2023

Conversation

young-do
Copy link
Collaborator

@young-do young-do commented Feb 10, 2023

이슈 번호

#20

작업 분류

  • 버그 수정
  • 신규 기능
  • 프로젝트 구조 변경

작업 상세 내용

  • middleware의 오동작으로 생긴 배포 문제를 해결했습니다.
    • 원인은 middleware에서 NextResponse.next가 기대하는 대로 동작하지 않았습니다.
    • config에 edge runtime 선언해도 해결되지 않았습니다.
    • 해결은 middleware 파일을 제거하고 해당 로직을 _app.tsx에 옮겼습니다.
  • 위의 이슈를 해결하면서 _app.tsx에서 사용한 로직과 기존 로직을 하나의 hook으로 옮겼습니다.

기타

  • dynamic route를 쓰는 페이지에 getStaticProps를 쓰면 ssg로 변경되어서, getStaticPaths도 추가하달라고 하네요 ;.;
  • 그래서 fix: middleware 오동작으로 생긴 배포 이슈 수정 및 리팩토링 #38 에서 쓴 방식을 없애고 isProtectedPage 속성을 추가하는 방식으로 변경했습니다.
  • 괜찮은 방식인 것 같은데 다른 분들의 생각이 궁금합니다~!

Copy link
Collaborator

@sangbooom sangbooom left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! ✨
모든 페이지의 컴포넌트 타입을 NextPageWithLayout로 주고 isProtectedPage를 true로 설정하는게 조금 아쉬워서 default값을 true로 하고 로그인이 필요없는 페이지만 false로 할 수 있는 방법은 없을까 고민해봤는데 아직 방법을 찾지 못했네요 😂 좀 더 고민해보겠습니다!

Copy link
Member

@siyeons siyeons left a comment

Choose a reason for hiding this comment

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

LGTM!
저는 괜찮을 것 같습니다!! 미들웨어에서 하는게 좋을것같은데 아쉽네욥 ㅠㅠ 수고하셨습니다!!

@young-do young-do merged commit 29ae9d3 into main Feb 11, 2023
@young-do young-do deleted the fix/deploy-error-by-middleware branch February 11, 2023 04:15
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