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: 스토리북 에러 대응 #1298

Merged
merged 6 commits into from
Mar 24, 2024
Merged

fix: 스토리북 에러 대응 #1298

merged 6 commits into from
Mar 24, 2024

Conversation

seojisoosoo
Copy link
Member

@seojisoosoo seojisoosoo commented Feb 16, 2024

🤫 쉿, 나한테만 말해줘요. 이슈넘버

🧐 어떤 것을 변경했어요~?

  • 스토리북에서 나는 에러에 대응했어요.

🤔 그렇다면, 어떻게 구현했어요~?

  • 처음 ui를 구현할 때에는 목데이터를 사용해서 스토리북에 문제가 없었어요. 그런데, 카테고리를 api로 통신해서 데이터를 가져오는 로직이 추가되면서 스토리북에서는 401에러가 뜨고 아무 데이터가 뜨지 않았습니다...! 조금 복잡해졌지만...ㅠㅠ 지금은 스토리북을 잘 확인할 수 있어요. 앞으로는 스토리북도 마구 만드는 것이 아니라, ui랑 로직이 분리되게 잘 짜야겠다는 생각을 했습니다...

❤️‍🔥 당신이 생각하는 PR포인트, 내겐 매력포인트.

해결

스크린샷 2024-02-16 오후 6 48 30
  • 이런 에러가 떠서, preview.tsx에서 LazyMotion을 사용하지 않고, m.div를 사용했어요. 이렇게 수정해도 괜찮을지 한 번 확인받고 싶어서, 기존 코드 삭제하지 않고 주석 처리해두었습니다.
  • 위의 에러를 해결하고 나니... 카테고리 쪽에서 401에러가 나면서 모든 드롭다운, 바텀시트가 다 빈칸으로 나왔어요. 목데이터로 넣어주기 위해서, 스토리북 내용이 조금 복잡해졌습니다ㅜㅜ

논의 요망

스크린샷 2024-02-16 오후 11 56 44
  • 여전히 이 에러가 남아있어요.. preview.tsx도 ResponsiveProvider로 감싸져있고, ImageUploadButton/index.stories.tsx에서도 ResponsiveProvider로 감싸져있는데, 왜 이런 에러가 나는 것일까요..? Default부분을 수정해서 잘 뜨면, Uploaded에서 에러나고, Uploaded부분을 수정해서 잘 뜨면, Default에서 에러가 나더라구요,,

📸 스크린샷, 없으면 이것 참,, 섭섭한데요?

Copy link

github-actions bot commented Feb 16, 2024

✨✨ 스토리북으로 확인하기 ✨✨

Copy link

github-actions bot commented Feb 16, 2024

🚀 프리뷰 배포 확인하기 🚀

https://514cf443.sopt-internal-dev.pages.dev

@seojisoosoo seojisoosoo self-assigned this Feb 16, 2024
@seojisoosoo seojisoosoo marked this pull request as ready for review February 16, 2024 15:43
@github-actions github-actions bot requested a review from juno7803 February 16, 2024 15:43
@@ -48,7 +49,9 @@ export const decorators = [
<QueryClientProvider client={queryClient}>
<QueryParamProvider adapter={NextAdapterPages}>
<RecoilRoot>
<LazyMotion strict features={() => import('framer-motion').then((mod) => mod.domAnimation)}>
{/* FIXME: break tree shaking 발생으로 주석 처리 */}
{/* <LazyMotion strict features={() => import('framer-motion').then((mod) => mod.domAnimation)}> */}
Copy link
Member

Choose a reason for hiding this comment

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

Lazy Motion이 문제가 아니라, LazyMotion이 있을 때 m 대신 motion 을 사용한게 문제에요..! (Strict 옵션)

Copy link
Member Author

Choose a reason for hiding this comment

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

호오.., TagSelector에 사용한 바텀시트에 react-modal-sheet 라이브러리를 사용하였는데, 해당 라이브러리 깃허브가서 코드 찾아보니, 여기서 motion을 사용하고 있네요, 방법을 찾아보는 중입니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

해당 이슈를 해결하려고, 바텀시트 레포 찾아가서 이슈도 올려보고, 커스터마이징할 수 있는 방안도 찾아보았는데, 해결하기가 쉽지는 않아보여요...
_app.tsx에서도 strict모드를 사용하고 있지는 않더라구요...! 그래서 4기 FE와 논의 후, strict를 제거하려고 합니다.

Comment on lines 19 to 24
openCategory: () => {
console.log('openCategory');
},
openTag: () => {
console.log('openCategory');
},
Copy link
Member

Choose a reason for hiding this comment

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

p3; 안넣어줘도 되지 않나용?

Copy link
Member Author

Choose a reason for hiding this comment

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

ㅎㅎ맞아용 삭제하도록 하겠숩니다~

@juno7803
Copy link
Collaborator

(뒷북 리뷰 제송합니다 ㅜㅜ..)

그런데, 카테고리를 api로 통신해서 데이터를 가져오는 로직이 추가되면서 스토리북에서는 401에러가 뜨고 아무 데이터가 뜨지 않았습니다...!

@seojisoosoo 어떤 문제인지 완전 공감해요. 그래서 구조 설계를 처음부터 데이터를 받아오는 부분과 해당 데이터를 ui로 그려주는 부분을 잘 나눠주는 것도 하나의 방법이고, 풀구에선 그러려고 했던 것 같아요! (container - presenter 패턴 이라고도 불렀던 것 같아요.)

하지만, 이걸 여러명이서 협업하면서 모두 지키기는 현실적으로 어려운데, ui 테스트를 깨지지 않게 하기 위해서 구조를 바꾸는 리팩토링을 하는건 오히려 너무 위험성이 큰 것 같더라구요. 그래서 차선책으로 지수가 한 것 처럼 mock 데이터를 바라보도록 스토리를 재작성 할 수도 있는데요, 크게 바꾸지 않고 msw로 useQuery의 데이터 호출을 mocking 하는 방식으로도 해결할 수 있어요. (https://storybook.js.org/addons/msw-storybook-addon)

어렵지 않고 기존의 스토리를 크게 바꿔줄 것도 없어서, 스토리가 깨지는 부분들을 빠르고 쉽게 해결하려고 한다면 저는 요 방법도 추천합니다! (해당 스토리에서 바라보는 api의 주소를 적어주고, 해당 주소에 대한 응답만 모킹해서 적어주면 돼요.)

@seojisoosoo seojisoosoo added this pull request to the merge queue Mar 24, 2024
Merged via the queue into main with commit 562a734 Mar 24, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: 스토리북 오류 대응
5 participants