-
Notifications
You must be signed in to change notification settings - Fork 1
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
[TASK-43] setting: MSW 설정 추가 및 axios client 추가 #8
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mocks
폴더 내 파일들은 리뷰 패스했습니다!
달리 제가 알아야 하는 것이 있거나 리뷰가 필요한 부분이 있으시면 얘기해주셔요 :)
public/mockServiceWorker.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
예시로 올려주신 거라면 그래도 PR 올리셔도 좋을 것 같아요! API 작업할 때 참고하겠습니다 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MSWProvider 만 이번에 추가되긴했는데 이후 oauth 관련해서 또 provider가 추가되는 것들이 있었습니다.
provider를 원래 컴포넌트에서 관리할까 하다가 생각보다 종류가 다양해지는 느낌이라 provider 폴더를 따로 두었습니다..!
NextUI도 provider의 성격이 강하면 provider로 분리해도 좋다고 생각합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
설명 감사해요 :)
음.. 그럼 이번에 올린 commit 저 상태 그대로인데, 따로 빼야 할지 좀 더 고민해보고 빼야 할 것 같으면 다음 PR에 반영하겠습니다!
src/app/providers/MSWProvider.tsx
Outdated
|
||
import { ReactNode, useEffect, useState } from 'react'; | ||
|
||
export default function MSWProvider({ children }: { children: ReactNode }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
크게 중요한 건 아니지만 rafce
로 함수 코드 스니펫을 통일하기로 했었는데, 이 코드처럼 진행하는 게 편하실까요? 🤔 궁금해서 여쭤봅니당
통일하는 거라면 ReactQueryProvider.tsx
도 반영해서 수정해야 하고, 다른 것도 rafce
에 맞게 적용돼 있는지 확인봐야 할 것 같아요. <메인 페이지>도 rafce
로 작성 안 돼있어서 다음 PR 올릴 때 반영하려고 수정해두었습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 저희 통일하기로 한 부분이 있으니 그렇게 변경하겠습니다! 우선 MSWProvider에 대해서 만 수정하겠습니다!
@@ -2,6 +2,7 @@ import type { Metadata } from 'next'; | |||
import React from 'react'; | |||
import '../styles/globals.css'; | |||
import ReactQueryProvider from './ReactQueryProvider'; | |||
import MSWProvider from './providers/MSWProvider'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import
띄어쓰기 반영 안 되는 게 아직도 맘 아프네요,,, import
구문 종류에 맞춰 띄어쓰기 한 번 부탁드립니당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제 vscode 로컬 옵션이랑 충돌이 뭔가 있는건지... import 정렬은 되는 것 같은데... 제 설정값 한번 확인하겠습니다.!
src/app/layout.tsx
Outdated
@@ -16,7 +17,9 @@ export default function RootLayout({ | |||
return ( | |||
<html lang="ko"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
헉 .prettierrc
에 "jsxSingleQuote": true,
반영이 안 된 것 같습니다.
많은 파일의 충돌을 피하기 위해 반영해서 PR 부탁드려요..!
import { ReactNode, useEffect, useState } from 'react'; | ||
|
||
export default function MSWProvider({ children }: { children: ReactNode }) { | ||
const [mswReady, setMswReady] = useState(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그냥 넘어갈 수 있는 부분이긴 한데, 요기 한 칸 띄어쓰기 ㅎ.. 부탁드립니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const [ mswReady, setMswReady ] = useState(false);
이렇게 일까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
엇 그게 뭔가요? 코드 그대로 복사 붙여넣기 해주신 걸로 보여요
저는 useState 코드와 useEffect 코드 사이의 한 줄 띄어쓰기를 의미한 거였어요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아ㅋㅋㅋㅋ 저는 대괄호랑 mswReady 사이에도 한칸띄워야하는지 여쭤봤습니다 ㅎㅎ 말씀하신 부분은 빈줄 추가해두었습니다!
init(); | ||
} | ||
}, [mswReady]); | ||
return <>{children}</>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그냥 넘어갈 수 있는 부분이긴 한데, 요기 한 칸 띄어쓰기 ㅎ.. 부탁드립니다 22,,
import axios from 'axios'; | ||
|
||
const defaultClient = axios.create({ | ||
baseURL: process.env.NEXT_PUBLIC_API_BASE_URL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NEXT_PUBLIC_API_BASE_URL
은 추후에 저에게도 따로 공유 부탁드려요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이부분은 아래 노션 문서에서 공유드리겠습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
답글 추가했습니다 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
설명 감사해요 :)
음.. 그럼 이번에 올린 commit 저 상태 그대로인데, 따로 빼야 할지 좀 더 고민해보고 빼야 할 것 같으면 다음 PR에 반영하겠습니다!
import { ReactNode, useEffect, useState } from 'react'; | ||
|
||
export default function MSWProvider({ children }: { children: ReactNode }) { | ||
const [mswReady, setMswReady] = useState(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
엇 그게 뭔가요? 코드 그대로 복사 붙여넣기 해주신 걸로 보여요
저는 useState 코드와 useEffect 코드 사이의 한 줄 띄어쓰기를 의미한 거였어요 :)
📝 작업 내용
📸 스크린샷
💬 리뷰 요구사항