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.

Sign up for GitHub

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

Jump to bottom

feat(web): Tanstack Query 세팅 및 API 공통 메서드 작성 #57

Merged
merged 4 commits into from
Jul 14, 2024
Merged

feat(web): Tanstack Query 세팅 및 API 공통 메서드 작성 #57

merged 4 commits into from
Jul 14, 2024

Conversation

Andrevile
Copy link
Member

🎉 변경 사항

🔗 링크

🙏 여기는 꼭 봐주세요!

@Andrevile Andrevile self-assigned this Jul 12, 2024
Doeunnkimm
Doeunnkimm reviewed Jul 12, 2024
View reviewed changes
Copy link
Member

@Doeunnkimm Doeunnkimm left a comment

Choose a reason for hiding this comment

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

고생하셔씁니다 🚀

추후에 query 파일 관리 방법에 대해서도 논의해봐야겟군요!

apps/web/app/layout.tsx Outdated Show resolved Hide resolved
apps/web/app/layout.tsx Outdated Show resolved Hide resolved
apps/web/app/page.tsx Outdated Show resolved Hide resolved
apps/web/app/page.tsx Outdated Show resolved Hide resolved
apps/web/next.config.mjs Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
packages/web-domains/src/common/apis/base.api.ts Outdated Show resolved Hide resolved
packages/web-domains/src/common/apis/check.api.ts Outdated Show resolved Hide resolved
packages/web-domains/src/common/contexts/QueryClientProvider.tsx Outdated Show resolved Hide resolved
semnil5202
semnil5202 reviewed Jul 13, 2024
View reviewed changes
Copy link
Collaborator

@semnil5202 semnil5202 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~~ 👍👍👍 Http Class 부분은 제가 했던 방식과 사뭇 달라서 한 번 확인 후 재 리뷰하겠습니다~~

apps/web/app/page.tsx Outdated Show resolved Hide resolved
packages/web-domains/src/common/apis/check.api.ts Outdated Show resolved Hide resolved
packages/core/eslint-config/base.js Show resolved Hide resolved
...es/web-domains/src/first-domain/features/services/useFirstFeatureOfFirstDomainTestService.ts Outdated Show resolved Hide resolved
semnil5202
semnil5202 reviewed Jul 13, 2024
View reviewed changes
Copy link
Collaborator

@semnil5202 semnil5202 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 에서는 악시오스 인스턴스가 page 별로 필요한가여?? 제가 이 부분을 잘 몰라서여.

클라이언트에서 악시오스 인스턴스 사용할 땐 싱글톤 느낌으로 인스턴스 하나만 뺐는데, 넥제 SSR 에선 무언가 다른듯 하네요.

@Andrevile
Copy link
Member Author

@semnil5202
세민님 말대로 하나만 빼는게 맞아요. 애초에 axios가 싱글톤 패턴으로 해놓은게 맞아서.
코드 작성할 당시 저것도 고민하긴 했었습니다.
리뷰를 보면 http 를 상속하는걸 제거할 예정이니 http 클래스의 메서드들을 static으로 변경예정이라
세민님 케이스대로 할 예정이에요

@Doeunnkimm Doeunnkimm changed the title feat: Tanstack Query 세팅 및 API 공통 메서드 작성 feat(web): Tanstack Query 세팅 및 API 공통 메서드 작성 Jul 13, 2024
@Andrevile Andrevile merged commit defe94d into main Jul 14, 2024
2 checks passed
@Andrevile Andrevile deleted the feat/SAMBAD-67 branch July 14, 2024 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Doeunnkimm Doeunnkimm Doeunnkimm approved these changes

@LeeJeongHooo LeeJeongHooo Awaiting requested review from LeeJeongHooo LeeJeongHooo is a code owner

@sambad-adventure sambad-adventure Awaiting requested review from sambad-adventure sambad-adventure is a code owner

@semnil5202 semnil5202 Awaiting requested review from semnil5202 semnil5202 is a code owner

Assignees

@Andrevile Andrevile

Labels
feat
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Andrevile @semnil5202 @Doeunnkimm @sambad-adventure