-
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
[4주차 기본/심화/공유 과제] API 통신 #6
base: main
Are you sure you want to change the base?
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.
코드 정말 잘 봤습니다!!
toss에서 사용한 useFunnel
을 참고해서 직접 구현하신 것도 정말 대단하고, 컴포넌트나 style코드를 분리하셔서 사용하신 점이 정말 많은 고민을 하신 것 같아서 보기에도 편했던 것 같습니다!
저는 아직 style 코드를 분리하는 것이 익숙하지 않아서 그대로 한 곳에 쓰고는 하는데 직접 작성해주신 것으로 보니 가독성 측면에서 한번 써 볼만하다고 생각이 듭니다!! 그리고 폴더 구조도 저와는 약간 다른 방법인데 그런 점에서 배울 점도 많았던 것 같아요!!
추가로 저도 잘하지는 않지만 api 호출하고 받는 부분에 대한 코드를 조금 더 고민해보셔도 좋을 것 같아요! 물론 이번 api가 막 엄청 자세히 나눠져 있지 않아서 예외 처리를 하기 조금 힘든 점도 있지만요!!
과제 정말 수고 많으셨습니다 :) 👍
리뷰 컨벤션을 아래와 같이 사용했습니다!!
P1
: 꼭 반영해 주세요 (Request Changes) - 이슈가 발생하거나 취약점이 발견되는 케이스 등
P2
: 반영을 적극적으로 고려해 주시면 좋을 것 같아요 (Comment)
P3
: 이런 방법도 있을 것 같아요~ 등의 사소한 의견입니다 (Chore)
export const api = axios.create({ | ||
baseURL: import.meta.env.VITE_BASE_URL, | ||
headers: { | ||
"Content-type": "application/json", | ||
}, | ||
}); |
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.
p3 : headers
에 Content-type
을 지정하는 것은 주로 post 요청에서 많이 보내게 되는데, axios는 post 요청에서 자동으로 headers
에 "Content-type": "application/json"
를 추가해줍니다! default 값으로 이미 설정이 되어 있어서 이 부분이 없어도 잘 작동하지만 명시적으로 작성을 할 수 있으니 참고하시면 좋을 것 같아요!
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 color = { | ||
white: "white", | ||
lightGray_1: "#B2C6E4", | ||
lightGray_2: "#B2C6E4", | ||
lightGray_3: "#707D90", | ||
gray: "#5A6473", | ||
darkGray_1: "#444B57", | ||
darkGray_2: "#2D323A", | ||
black: "black", | ||
red: "red", | ||
}; |
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.
p2 : 물론 간단한 색상이기도 하고, 큰 프로젝트도 아니라 크게 문제될 것은 없지만 일관성으로 white나 black 등도 hex code
로 작성해주시는 게 좋을 것 같아요! 그리고 더 찾아보니 css에서 단순 colors name
을 사용하는 것보다 hex code
로 사용하는 것의 장점이 더 있는 것 같습니다.
- hex code가 colors name보다 더 정확하다.
- hex code는 어떤 브라우저든 지원을 하지만 colors name은 매우 오래된 브라우저에는 지원을 안 하는 경우도 있다.
- hex code가 더 빠르게 처리된다.
[ 참고 자료 ]
https://www.quora.com/Why-is-using-hex-codes-better-than-using-color-names-in-CSS
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 text = { | ||
large: { | ||
fontSize: "24px", | ||
lineHeight: "24px", | ||
}, | ||
medium: { | ||
fontSize: "20px", | ||
lineHeight: "17px", | ||
}, | ||
small: { | ||
fontSize: "12px", | ||
lineHeight: "14px", | ||
}, | ||
}; |
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.
p2 : fontSize
는 rem
이 아니라 px로 한 이유가 있으신지 궁금합니다!!
그리고 theme
에서 lineHeight
도 지정을 해서 사용할 수도 있군요!! 다음에 디자인 시스템 보고 한번 저도 line-height도 지정해봐야겠어요!
path: "/", | ||
element: <LoginPage></LoginPage>, | ||
}, | ||
{ | ||
path: "/signup", | ||
element: <SignUpPage></SignUpPage>, | ||
}, | ||
{ | ||
path: "/mypage", | ||
element: <MyPage></MyPage>, |
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.
p2 : 컴포넌트에 children
이 없을 경우 self-closing
하면 좋을 것 같습니다!
export const inputStyle = (Theme: ThemeType) => css` | ||
display: flex; | ||
flex-direction: column; | ||
align-items: center; | ||
margin: 1rem; | ||
gap: 0.5rem; | ||
& input { | ||
padding: 0.5rem; | ||
width: 20rem; | ||
border: 1px solid ${Theme.color.lightGray_1}; | ||
border-radius: 5px; | ||
} | ||
& input::placeholder { | ||
color: ${Theme.color.lightGray_3}; | ||
} | ||
`; |
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.
p3 : 확실히 style 코드를 분리하니까 로직 부분과 style코드를 따로 볼 수 있어서 가독성이 좋은 것 같네요! 다른 코드 예시에서도 본 것 같은데 theme을 저렇게 받아왔을 때 ${({ theme }) => theme.fonts.title}
이렇게 props
형태로 받아오는 것과 비교해서 장점이 어떤 게 있는지 궁금합니다!
updateUserData({ | ||
hobby: input.hobby, | ||
password: input.password, | ||
userToken: userToken, | ||
}); |
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.
p3 : userToken
이 key
, value
같으니 한번으로 요약해서 사용해도 좋을 것 같습니다!
updateUserData({
hobby: input.hobby,
password: input.password,
userToken,
});
if (userToken && (input.password || input.hobby)) { | ||
updateUserData({ | ||
hobby: input.hobby, | ||
password: input.password, | ||
userToken: userToken, | ||
}); | ||
alert("수정에 성공했습니다!"); | ||
navigate("/"); | ||
} else { | ||
alert("칸을 입력해주세요."); | ||
} | ||
}; |
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.
p1 : userToken
이 있는 지 확인하고 updateUserData
함수(api 호출 함수)를 호출하는 로직인데, userToken
이 없는 경우에 대한 예외 처리를 서버에서 return 해주니 그걸 이용하는 게 조금 더 맞는 방법이지 않을까 의견을 한 번 내봅니다...!
(제가 틀릴 수 있으니 편하게 말해주세요!!! 저도 배우겠습니다 :) )
그래서 updateUserData
를 호출할 때 userToken
은 확인하지 않아도 될 것 같고, 확인을 해야 한다면 else
문에 alert("칸을 입력해주세요.");
만 쓰는 것이 아니라 예외 처리를 더 추가해야 할 것 같아요! 현재 상황에서 사용자 입장에서 userToken
이 없는 경우인데 칸을 입력해달라고 할 수 있으니 당황할 수 있을 것 같습니다!
const Button = ({ children, disabled, ...props }: ButtonProps) => { | ||
return ( | ||
<button {...props} css={buttonStyle} disabled={disabled}> | ||
{children} | ||
</button> | ||
); | ||
}; |
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.
p3 : 와우!! 이게 pr에 작성해주신 코드네요!! 저는 공통 컴포넌트 구현할 때 props
를 주는 것마다 하나하나 작성을 해줬는데 속성으로 지정할 내용을 그대로 전달해서 바로 { ...props }
로 적용하면 너무 좋을 것 같아요!! 지식 배워갑니다 :)
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.
{... props}
세미나에서도 배웠는데, 이렇게 쓰이니 진짜 공용 컴포넌트 줄때 완전 확장성이 배로 느네요! 진짜 코드 보면서 많이 배웁니다
interface ButtonProps extends ButtonHTMLAttributes<HTMLButtonElement> { | ||
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.
p3 : { ...props }
로 받아오면 이렇게 type을 지정할 수 있군요!! 대박.....!!!!!!
저도 children
관련해서 ReactNode
로 했다가 PropsWithChildren
이라는 타입을 찾아서 관련된 고민을 한 것을 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.
p5: 왕 .. 전 진짜 이번 과제에서 버튼 분리할 생각도 못했는데 진짜 한서님 코드 보고 저 정말 깨달음을 얻었어요!!!
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.
역시 OB의 코드란 이런것이군요..
보면서 깨달음의 연속이었던 코드리뷰였습니다! pr 작성을 보니 정말 많이 고민하신 것 같은데, 그만큼 너무 가독성도 좋고 분리도 잘된 코드가 나온 것 같네요 👍 에러 상수처리부터 공용컴포넌트 핸들링까지 너무 깔끔하고 효율적이었습니다. 좋은 코드 보고 저두 이번 합세에 잘 적용해보겠습니다 🤓
그리구 나중에 타입 분리나, tsx에서 로직을 조금 더 분리해보셔도 너무 좋을 것 같습니다!
코드 너무 잘 봤습니다 :) 4주차 과제도 너무 고생많으셨어요
코리까지 하니 한서님과 함께 합세 같이 할 수 있어서 너무 신나고!! 기대되네요 ㅎㅎ
@@ -0,0 +1 @@ | |||
VITE_BASE_URL = "http://211.188.53.75:8080" |
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.
p3: 옹 .env
가 올라와있는데 혹시 과제 채점때문에 함께 올려주신건가요?! gitignore
하시지 않은 이유가 궁금합시당
*.sln | ||
*.sw? | ||
|
||
.env |
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.
p2: 오홍 위에 엔브가 올라와있다고 코리 남겼는데, .env
가 있는데 적용이 잘 되지 않았던 것 같습니다!
저도 예전에 깃이그노어에서 걸러내려고 했는데 엔브가 올라갔던 경험이 있었어서 그때부터 의식적으로 엔브를 올리지 않기 위해 한 방법이 있는데요! 공유해보겠습니다 :) (전 무려 구글 로그인 구현하려다가 구글키 올려버린.. )
1️⃣ gitignore에 .env 관련 내용 적어주기
.env
.env.local
.env.production
.env.development
2️⃣
git add .
시 항상git status
찍어보고 .env 포함되었는지 확인하기
만약 포함되어있다면git rm --cached .env
해주기
@@ -0,0 +1,13 @@ | |||
<!doctype html> | |||
<html lang="en"> |
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.
p4: HTML의 언어 속성 부분 사소한 부분이지만 저두 맨날 ko로 바꿔주는거 까먹게 되더라구요!
interface ButtonProps extends ButtonHTMLAttributes<HTMLButtonElement> { | ||
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.
p5: 왕 .. 전 진짜 이번 과제에서 버튼 분리할 생각도 못했는데 진짜 한서님 코드 보고 저 정말 깨달음을 얻었어요!!!
const key = `${statusCode}-${errorCode}`; | ||
const errorMessage = LOGIN_ERROR_MESSAGES[key]; | ||
alert(errorMessage); |
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.
p5: 아 res 상태 코드는 왜 필요한지 궁금했는데, 에러코드가 겹칠 수도 있어서 같이 묶어주신거군요!!
코드 보면 볼 수록 한서님이랑 하는 합세가 너무너무 기대됩니다 ... 🤍
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.
맨날 엑시오스 에러 catch 로 에러값만 리턴한 저에게는 아주 멋진 코드입니다!! 👀
hobby: "", | ||
}); | ||
const navigate = useNavigate(); | ||
const userToken = localStorage.getItem("userToken"); |
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.
p3: tsx 파일에서 토큰을 가져오는 것과, axios 쏠때 토큰을 가져오는 것중 어디에서 받아오는게 좋을지 고민을 했었는데요.
저의 경우에는 매개변수로 넘겨주는 것보다, 로직 처리할때 호출되면 get해서 가져오는걸로 코드를 작성했습니다! ui 를 보여주는 곳보다는 api 호출때에 가져오는게 더 좋겠다고 생각해서 그랬는데요
요 코드를 보니 한서님의 생각도 너무너무 궁금합니다!!
const reqBody = { | ||
hobby: hobby, | ||
password: password, | ||
}; |
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.
p5: 던져주기 전에, 깔끔하게 묶어주는 센스 👍
const useGetMyHobby = (userToken: string | null) => { | ||
const [response, setResponse] = useState(""); | ||
|
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.
p4: 혹시 이 로직을 useGetMyHobby 로 한번더 묶어주신 이유가 있을까요?! 그리고 res 를 상태 관리해준 이유도 궁금합니다!! (커스텀 훅으로 구분한 이유) 💭 저는 매번 fetchData 형식에서 그냥 응답값을 return 해주는 방식이었어서 요렇게 관리해주면 어떤 점이 더 좋은지 배우고 싶습니다
const color = { | ||
white: "white", | ||
lightGray_1: "#B2C6E4", | ||
lightGray_2: "#B2C6E4", | ||
lightGray_3: "#707D90", | ||
gray: "#5A6473", | ||
darkGray_1: "#444B57", | ||
darkGray_2: "#2D323A", | ||
black: "black", | ||
red: "red", | ||
}; |
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.
굿~
hobby: "", | ||
}); | ||
const navigate = useNavigate(); | ||
const userToken = localStorage.getItem("userToken"); |
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.
p5: 토큰 처리는 axios instance 만들어서 사용하면 재사용성을 높일 수 있을것 같아요.
아래 예시 코드 첨부드립니다!
const authClient = axios.create(authAxiosConfig);
authClient.interceptors.request.use(
async (config) => {
const accessToken = getAuthHeader();
if (accessToken) {
config.headers.Authorization = `Bearer ${accessToken}`;
} else {
console.error('토큰이 없습니다. 로그인이 필요합니다.');
window.location.replace('/auth/login');
}
return config;
},
(error) => {
console.error(error);
},
);
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.
YB때보다 훨씬 더 성장한 모습 보기 좋네요 한서님 ..
{step === 1 && ( | ||
<Name | ||
handleSaveInputValue={handleSaveInputValue} | ||
onClick={handleMoveToNextStep} | ||
disabled={disabled} | ||
isError={isError} | ||
></Name> | ||
)} | ||
{step === 2 && ( | ||
<Password | ||
handleSaveInputValue={handleSaveInputValue} | ||
disabled={disabled} | ||
onClick={handleMoveToNextStep} | ||
isError={isError} | ||
userInputs={userInputs} | ||
></Password> | ||
)} | ||
{step === 3 && ( | ||
<Hobby | ||
handleSaveInputValue={handleSaveInputValue} | ||
disabled={disabled} | ||
handleClickSignUpBtn={handleClickSignUpBtn} | ||
isError={isError} | ||
></Hobby> | ||
)} |
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.
p5: 요런거 IIFE
즉시 실행 함수로 조금 더 선언적이게 작성해줄 수 있어용
{(() => {
switch (step) {
case 1: return <Name />;
case 2: return <Password />;
case 3: return <Hobby />;
// default: error handling
}
})()}
✨ 구현 기능 명세
💡 기본 과제
취미
,내 정보
메뉴 탭로그아웃
버튼취미
,내 정보
취미 페이지, 내 정보 페이지 출력 (1개의 페이지로 구현해도 되고, url 달라도 됨)🔥 심화 과제
공유과제
제목: 💥 로딩/에러처리 및 데이터 패칭 라이브러리
링크 첨부 : https://wave-web.tistory.com/132
❗️ 내가 새로 알게 된 점
Record<Key, Value>
키가 Key타입이고 값이 Value 타입인 객체 타입을 생성함
❓ 구현 과정에서의 어려웠던/고민했던 부분
1. 폴더구조
2. 공통 컴포넌트 Button 생성 (html 버튼 태그 속성을 받아오도록 인터페이스 확장)
모든 step과 페이지에서 똑같은 버튼이 있고, 다음step으로 이동 또는 input 값 제출 등의 기능이 반복되길래 공통 컴포넌트로 설계했습니다.
버튼 prop의 타입 인터페이스를 지정할 때 ButtonHTMLAttributes(html 버튼 태그의 속성)를 상속받았는데, 버튼 태그가 지원하는 기본 속성(disabled, onClick, type 등)을 ...props로 받아오기 때문에, 해당 공통 컴포넌트를 그냥 버튼 태그 쓰는 것처럼 사용할 수 있게끔하여 확장성을 높여봤습니다.
children prop을 받게 하여, 버튼 속 text들을 유연하게 설정할 수 있도록 했습니다
button 태그의 기본 속성들이 ...props로 한번에 받아와짐
3. 에러처리
다른분들의 방법이 궁금합ㄴ디ㅏ!
🥲 소요 시간
10h
🖼️ 구현 결과물
구현 영상
2024-11-14.8.53.35.mov