6주차 개발 상황 코드리뷰 #68
6주차 개발 상황 코드리뷰 #68
Conversation
Chore: Weekly 브랜치와 머지
회원가입 페이지 UI 수정
시니또 평가 페이지 UI 구현
5주차 코드리뷰 피드백 반영
�Devlop 브랜치 pull , 필요없는 파일 제거
카카오 로그인 기능 구현
전체적인 구조 변경
| @@ -0,0 +1,3 @@ | |||
| export const BASE_URI = import.meta.env.VITE_APP_BASE_URL; | |||
|
|
|||
| export const KAKAO_AUTH_URL: string = `${BASE_URI}/api/auth/oauth/kakao`; | |||
There was a problem hiding this comment.
FE에서 사용하는 환경 변수는
변수를 감추기 위한 용도가 아니라 URI를 환경에 따라 다르게 표기하기 위함입니다 ㅎㅎ
덕분에 저는... 로그인이 불가능하네요..
| title = '요청 가이드라인'; | ||
| break; | ||
| case RouterPath.SERVICE_HISTORY: | ||
| title = '서비스 이용 내역'; |
There was a problem hiding this comment.
shared가 app을 의존하는 모습은 좋지 못한 것 같아요 ㅠㅠ
반대 방향으로 의존이 되어야 하는게 아닐까요?
Layout의 title은 pages가 만들어서 넘겨주도록 하면 좋겠습니다.
| const data = DUMMY_DATA.slice(0, visibleCount); | ||
|
|
||
| return { data, enabled, toggle }; | ||
| }; |
There was a problem hiding this comment.
여기도 동일합니다. components가 pages를 가져다 사용하는게 아니라,
pages에서 components를 가져다 사용하는 모습이여야 해요.
데이터를 내부에서 불러오는게 아니라 바깥에서 불러올 수 있도록 해보세요!
| }; | ||
|
|
||
| const set = (value: StorageKey[T]) => { | ||
| if (value == undefined || value == null) { |
There was a problem hiding this comment.
null == undefined; // true
null === undefined; // false
이렇게 연산이 된답니다 ㅎㅎ
그래서 더 정확하게 체크하고 싶으면
if (value === undefined || value === null) {이렇게 표현해야 된답니다!
| accessToken?: string; | ||
| }; | ||
|
|
||
| const initStorage = <T extends keyof StorageKey>(key: T, storage: Storage) => { |
There was a problem hiding this comment.
const initStorage = <T extends keyof StorageKey>(key: T, storage = window.localStorage) => {이렇게 기본값을 넣어주면 어떨까요?
| handleClick={handleClick} | ||
| /> | ||
| </TypeWrapper> | ||
| </Wrapper> |
| handleClick: (id: string) => void; | ||
| }; | ||
|
|
||
| export const RegisterType = ({ userType, handleClick }: Props) => { |
There was a problem hiding this comment.
handleClick이 아니라 onClick으로 받아와야 자연스러울 것 같아요!
| } | ||
| return response.data; // 200 | ||
| } catch (err) { | ||
| throw new Error('회원가입 실패'); |
There was a problem hiding this comment.
catch는 제거해주세요..
오히려 오류 정보가 다 손실되고 있네요 ㅠ
| // 공통 타입 정의 (onSuccess 내부에서 분기) | ||
| export type SignupApiResponse = SignupResponse | SignupErrorResponse; | ||
|
|
||
| export const registerUser = async ({ |
There was a problem hiding this comment.
useRegister와 userRegister의 거리가 너무 멀리 있네요 ㅎㅎ
같은 폴더에 위치시키면 어떨까요?
어딘가에서는 api와 hook이 한 묶음으로 쓰이고
여기는 이산가족처럼 멀리 떨어져있어요.
일관적인 모습으로 구조화가 되면 좋겠습니다.
|
|
||
| fetchInstance.interceptors.request.use( | ||
| (config: InternalAxiosRequestConfig) => { | ||
| const accessToken = localStorage.getItem('accessToken'); |
말씀해주신 것 처럼 백엔드 주소를 환경변수로 관리하는건... 지금 아무 의미도 없답니다 ㅎㅎ https://fe-developers.kakaoent.com/2022/220310-kakaopage-bff/ 프론트엔드(클라이언트)에서는 BFF만 호출하고 클라이언트 -> BFF -> Backend 이런 흐름으로 네트워크 요청을 주고받는다고 보시면 좋을 것 같아요. 지금은 저같이 프로젝트에 대한 컨텍스트가 없는 사람이 리뷰할때 "이거 어떻게 실행해?" 라는 의문만 생긴답니다.. ㅎㅎ
link 태그..는 아니고 정확히는 컴포넌트를 말씀하시는 것 같네요. 사용자가 직접 이동하게 만들 것이냐, 함수를 통해서 이동하게 만들 것이냐의 차이입니다.
지금 고쳐진 부분도 보이고 아닌 부분도 보이네요! 점진적으로 변경되리라 기대하고 있겠습니다 ㅎㅎ 이 외에도 코멘트에 남겨놓긴 했지만, 각각의 폴더에 대해 의존 관계를 명확하게 하면 좋을 것 같아요 app 이렇게 있을 때 shared는 app과 pages의 존재를 몰라야합니다. 이게 없어도 잘 돌아가야 이상적인 모습일 것 같아요 그런데 지금은 이 관계가 뒤죽박죽입니다. 가령, 이 코드를 아예 패키지로 분리한다고 생각해보세요. @team8/app 이 상태에서 app과 pages가 상호의존 하고 있으면 패키지 설치 자체가 불가능해질 수 있답니다. |
#️⃣ 연관된 이슈
📝 작업 내용
스크린샷 (선택)
💬 리뷰 요구사항(선택)
⏰ 현재 버그
✏ Git Close
안녕하세요 멘토님! 이번주 코드리뷰 PR도 많이 늦어버렸네요.. ㅜㅜ 다음주부터는 꼭 기간맞출수 있도록 더 신경쓰겠습니다 🥲
이번주차부터는 기능 위주로 개발을 진행했고, 저희팀 벡엔드에서 개발이 어느정도 끝나가기 때문에 서버와 통신할 수 있는 api 통신 코드도 작성해보면서 정상적으로 동작하는지 확인해보면서 개발을 진행하고 있습니다!
개발을 진행하면서 몇가지 궁금한 점이 생겨 질문 남겨봅니다...!
백엔드 주소 노출 관련
저희가 개발을 진행하면서 백엔드 주소를 파일 내에 작성해두고 그대로 커밋을 날리게 되면서 코드상에 백엔드 주소가 노출되었는데, 이 경우 커밋을 취소하고 다시 커밋을 작성하는 것이 맞는지 궁금합니다.
제가 알기로는 모든 ajax 통신은 통신 행위가 이루어질때 백엔드 주소가 브라우저의 네트워크탭에서 노출되는걸로 알고 있습니다. 그렇다면
.env에 환경변수로써 백엔드 주소를 등록하고, gitignore에 등록하여 github에 올리지 않는 이 방법이 어떤 의미가 있는지 궁금합니다!평소 개발을 진행하면서는 당연히 백엔드 주소는 환경변수로 등록하여 가려놓는게 맞다고 생각했는데, 네이버에서도 네트워크탭에 백엔드 주소로 추정(??) 되는 것들이 노출이 되는 것 같아서 조금 의문점이 들어서 이렇게 질문 남깁니다!
useNavigate()와<Link>태그는 어떤 차이점이 있나요?react-router-dom 라이브러리를 활용해서 페이지를 이동하는 수단이 이렇게 useNavigate와 link태그로 두가지가 있는걸로 알고 있습니다.
<a>태그와 비교했을때 이 두가지 수단이 다른점은 "페이지 전체를 이동하는 것과 컴포넌트 단위로 변경하는 방법" 이라는 것은 알고 있었고, 여러가지 블로그를 찾아보니 useNavigate와 link 태그는 함수로 작성하여 추가적인 기능을 넣을 때 (handler)는 useNavigate, 단순히 컴포넌트를 변경하는 행위만 할떄는 link 태그를 사용하며 단순하게 생각하면 link 태그를 사용하면 코드양 자체가 줄어든다는 점이 장점 같은데, 정확하게 어떤 차이점이 있는지 잘 모르겠습니다..추가적으로 멘토님께서 5주차때 말씀하셨던
index.tsx보다 더 직관적인 파일명을 사용하자는 부분은 기능개발을 진행하면서 수정하고 있고, 폴더 depth 또한 계속해서 고민하고 있습니다. 사실 최근에 멘토님 깃허브를 둘러보다가 너무 마음에 드는 구조를 발견해서 전부 갈아엎고 싶은 마음이 들었지만.. 개발이 어느정도 진행되었기에 혼자 구조를 자주 변경하면 혼란스러울 수 있을것 같아서, 저번 멘토링때 말씀해주셨던 처럼 'index.ts' 파일에 export 기능 (이 부분을 '진입점을 만든다.' 라는 표현이 맞나요?) 을 활용하는 방법을 사용해보고 있습니다!사실 이외에도 개발적인 부분에서 물어보고 싶은점이 이것저것 많은데,, 조금 더 찾고 공부해보는 시간을 가져보고, 그 이후에도 잘 모르겠다면 멘토링 시간을 활용해서 질문해보도록 하겠습니다! 이번 6주차 코드리뷰도 잘부탁드리겠습니다! ㅎㅎ