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

⚙️ MSW setting #11

Merged
merged 13 commits into from
Oct 26, 2024
Merged

⚙️ MSW setting #11

merged 13 commits into from
Oct 26, 2024

Conversation

Legitgoons
Copy link
Member

@Legitgoons Legitgoons commented Oct 25, 2024

CheckList

  • gap을 이용하여 자식 요소간의 간격을 제어하였나요? (iOS 14 미지원으로 gap -> space between 적용)

작업 이유

  • msw를 사용한 server mocking

작업 사항

1️⃣ msw 설정

  • maintainer가 올린 예시를 참조해 저희 프로젝트 상황에 맞게 적용했습니다.
    • server side일 경우
    //app/Layout.tsx
    if (process.env.NEXT_RUNTIME === ‘nodejs’)
    //...
    • client side일 경우
    //src/app/mocks/MswProvider.tsx
      typeof window !== ‘undefined’
        ? import('./browser').then(async ({ worker }) => {
            await worker.start({
    //...

2️⃣ src/app/provider -> query로 수정

  • 기존에는 provider파일들을 보관하고자 만들었으나 다음 부분들이 msw의 특성과 맞지 않는다고 생각해 query 폴더로 수정했습니다.
    • 다른 목적으로 사용하는 도구들을 하나의 Provider로 모으는 것
    • msw, react-query등의 도구를 export하는 것이 provider인데, 이것들을 따로 보관하는 것
    • fsd 공식문서에서 같은 단계 내의 다른 폴더를 import하는 것을 지양하라고 작성되어 있는 것

리뷰어가 중점적으로 확인해야 하는 부분

1. 수환

  • msw가 정상적으로 동작하는지
  • 하단의 이슈들을 인지하였는지

2. 병준

  • msw가 정상적으로 동작하는지
  • 하단의 이슈들을 인지하였는지

발견한 이슈

  • 현재 next.js에서 msw가 hmr을 지원하지 않습니다.
    • msw maintainer와 vercel이 해당 이슈를 해결 중이며, hmr을 지원할 수 있는 방법을 확인하는대로 업데이트 하겠습니다.
  • next.js에서 fetch를 사용하면 기본적으로 캐싱을 지원하는데, 이로 인해서 에러가 발생하는 이슈가 있었습니다.
    • 우선 임시로 axios를 사용해서 처리해놓았습니다.
    • 하지만 기본적으로 지원되는 캐싱을 포기하기는 아쉬워 다음의 방법들을 생각중입니다. 이에 대해서 의견 제시해주시면 감사드리겠습니다!
      • msw로 요청을 보낼때는 cache를 꺼두고, 추후 프로덕션 환경으로 넘어갈때 다시 cache를 사용하는 방법
      • 프로덕션용 api 공통함수와 cache를 끈 msw용 api 공통 함수를 별개로 구현해두고 적절하게 사용하는 방법

@Legitgoons Legitgoons self-assigned this Oct 25, 2024
Copy link

github-actions bot commented Oct 25, 2024

🔍 Visual review for your branch is published 🔍

Here are the links to:

Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-11.d1rrzjx2upcmxq.amplifyapp.com

@Legitgoons Legitgoons marked this pull request as ready for review October 25, 2024 17:35
Copy link
Collaborator

@BangDori BangDori left a comment

Choose a reason for hiding this comment

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

현재 next.js에서 msw가 hmr을 지원하지 않습니다.

위 문장의 의미가 API mock handler 함수를 수정해도 그 변경 사항이 즉시 반영되지 않는다는 것을 말씀하시는게 맞을까요?

next.js에서 fetch를 사용하면 기본적으로 캐싱을 지원하는데, 이로 인해서 에러가 발생하는 이슈가 있었습니다.

nextJS fetch Troubleshooting을 확인해보면 nextJS에서는 기본적으로 모든 fetch 요청을 hmr 캐시에 저장해두지만, hmr이 지원되지 않는 상황이라면 mock api에 대한 캐시 기능을 적용하기엔 어렵다는 생각이 듭니다. 그래서 우선 1번 방법을 적용하고 실제 API를 연동하고 캐시 기능이 적용된 상황을 확인해봐야할 것 같습니다!

app/page.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@suhwan2004 suhwan2004 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 실제 pr 브랜치 내에서 정상동작 확인했고, 짧게 질문 하나 남겨두었습니다!

공유 주신 이슈 관련된 생각은 다음과 같습니다.

현재 next.js에서 msw가 hmr을 지원하지 않습니다.
=> 개발 편의성이 불편하지만, 별다른 해결방법이 없다면 어쩔 수 없을 것 같네요...공유 감사합니다!

next.js에서 fetch를 사용하면 기본적으로 캐싱을 지원하는데, 이로 인해서 에러가 발생하는 이슈가 있었습니다...
=> 저는 2번이 괜찮다고 생각한게, 1번으로 갈 시에 msw 말고 따로 사용치않는 axios 도입 및 api 코드를 두 번 작성되야 되는것이 애매해서 2번을 희망합니다!

src/app/mocks/MswProvider.tsx Outdated Show resolved Hide resolved
@Legitgoons Legitgoons merged commit f5ccb3e into main Oct 26, 2024
3 checks passed
@Legitgoons Legitgoons deleted the feat/msw-setting-real branch October 29, 2024 11:54
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