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

[오동혁] Week19 #494

Merged

Conversation

ohdong9795
Copy link
Collaborator

@ohdong9795 ohdong9795 commented Jun 30, 2024

요구사항

기본

  • 변경된 api 사용
  • 모달에 필요한 api 요청을 만들어 기능을 완성해 주세요.
  • api 요청에 TanStack React Query를 활용해 주세요.

심화

주요 변경사항

  • pagerouter -> approuter
  • styled-components -> tailwind
  • api 커스텀 훅 -> react-query

스크린샷

멘토에게

  • vercel 배포: https://5-weekly-mission-umber.vercel.app/
  • 하루면 가능할 줄 알았는데 approuter, tailwind로 바꾸고 SSR, react-query 넣으려 했더니 시간이 부족해서 SSR 버리고 모달은 api 요청 없이 껍데기입니다.
    이렇게 될 줄 알았으면 갈아엎지 말고 그냥 기능이라도 전부 완성하는게 좋았을 것 같은데 미완성으로라도 리뷰 받겠습니다.

@ohdong9795 ohdong9795 requested a review from kiJu2 June 30, 2024 12:34
@kiJu2
Copy link
Collaborator

kiJu2 commented Jul 1, 2024

수고 하셨습니다 ! 스프리트 미션 하시느라 정말 수고 많으셨어요.
학습에 도움 되실 수 있게 꼼꼼히 리뷰 하도록 해보겠습니다.

@kiJu2
Copy link
Collaborator

kiJu2 commented Jul 1, 2024

하루면 가능할 줄 알았는데 approuter, tailwind로 바꾸고 SSR, react-query 넣으려 했더니 시간이 부족해서 SSR 버리고 모달은 api 요청 없이 껍데기입니다.

오옷 모두 제가 좋아하는 스펙이네요. 기대가 되는군요 🥺🥺

이렇게 될 줄 알았으면 갈아엎지 말고 그냥 기능이라도 전부 완성하는게 좋았을 것 같은데 미완성으로라도 리뷰 받겠습니다.

완성이 중요하지 않습니다 !! ㅎㅎㅎ 지금까지 하신거 꼼꼼히 살펴보고 동혁님께 도움이 될 게 있는지 봐볼게요 !

@kiJu2
Copy link
Collaborator

kiJu2 commented Jul 1, 2024

commit 단위를 더욱 자주, 작게 해보시는건 어떠실까요?

git을 다룰 때 commit은 "언제 해야 하는가"를 생각해보신 적 있으신가요?
흔히 하는 말이 있습니다:

커밋은 합칠 수 있지만 나눌 수 없습니다.

그럼 커밋을 언제 해야 할까요?

저는 다음과 같은 룰을 지키며 커밋을 하는걸 권장 드립니다:

  1. 커밋을 하는 단위는 커밋 메시지 한 줄로 설명할 수 있는 행동
  2. 하나의 목표 혹은 액션이 달성될 때

관련하여 읽으시면 좋은 아티클을 추천드릴게요:

tl;dr

관련 변경 사항 커밋
커밋은 관련 변경 사항에 대한 래퍼여야 합니다. 예를 들어 두 개의 다른 버그를 수정하면 두 개의 별도 커밋이 생성되어야 합니다. 작은 커밋을 통해 다른 개발자가 변경 사항을 더 쉽게 이해하고 문제가 발생한 경우 롤백할 수 있습니다. 준비 영역과 같은 도구와 파일의 일부만 준비하는 기능을 통해 Git을 사용하면 매우 세부적인 커밋을 쉽게 만들 수 있습니다.

자주 커밋
커밋은 커밋을 작게 유지하고 관련 변경 사항만 커밋하는 데 도움이 되는 경우가 많습니다. 또한 이를 통해 코드를 다른 사람들과 더 자주 공유할 수 있습니다. 이렇게 하면 모든 사람이 정기적으로 변경 사항을 통합하고 병합 충돌을 방지하는 것이 더 쉬워집니다. 대조적으로, 대규모 커밋을 갖고 이를 드물게 공유하면 충돌을 해결하기가 어렵습니다.

미완성 작업을 커밋하지 마십시오
논리적 구성 요소가 완료된 경우에만 코드를 커밋해야 합니다. 자주 커밋할 수 있도록 기능 구현을 빠르게 완료할 수 있는 논리적 청크로 분할합니다. 깨끗한 작업 복사본이 필요하기 때문에(브랜치 확인, 변경 사항 가져오기 등) 커밋하고 싶은 유혹이 든다면 Git의 «Stash» 기능을 대신 사용하는 것이 좋습니다.

커밋하기 전에 코드를 테스트하세요
완료되었다고 생각하는 일을 저지르고 싶은 유혹에 저항하세요. 철저하게 테스트하여 실제로 완료되었는지, 부작용이 없는지(알 수 있는 한) 확인하세요. 로컬 저장소에 설익은 것을 커밋하려면 자신만 용서하면 되지만, 코드를 다른 사람과 푸시/공유하는 경우에는 코드를 테스트하는 것이 훨씬 더 중요합니다.

원문 보기

또한 깃 커밋 메시지 컨벤션도 함께 읽어보세요:

tl;dr:

커밋 메시지 형식

type: Subject

body

footer

기본적으로 3가지 영역(제목, 본문, 꼬리말)으로 나누어졌다.

메시지 type은 아래와 같이 분류된다. 아래와 같이 소문자로 작성한다.

feat : 새로운 기능 추가
fix : 버그 수정
docs : 문서 내용 변경
style : 포맷팅, 세미콜론 누락, 코드 변경이 없는 경우 등
refactor : 코드 리팩토링
test : 테스트 코드 작성
chore : 빌드 수정, 패키지 매니저 설정, 운영 코드 변경이 없는 경우 등

원문보기

@@ -1 +1,2 @@
NEXT_PUBLIC_BASE_URL = https://bootcamp-api.codeit.kr/api/linkbrary/v1
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳 ~! 환경 변수를 설정하셨군요 👍

훌륭합니다 ! 환경에 따라 변경되는 수는 환경변수에 적용하는게 좋지요 !

다음은 제가 작성한 아티클인데 아실 수도 있으나 혹시나 도움되실까 첨부드립니다 😊:

왜 환경 변수에 저장해야 하나요?

🔐 보안

지금처럼 카카오 API 개인 키를 소스코드에 그대로 노출되면 git이 추적하게 되고 github와 같은 오픈된 공간에 비밀 키 값이 노출되므로 보안에 위협이 될 수 있습니다.

🤸 용이성

개발(dev), 테스트(test), 실제 사용(prod) 등 다양한 환경에서 앱을 운영하게 되는 경우, 각 환경에 따라 다른 base URL을 사용해야 할 수 있습니다. 만약 코드 내에 하드코딩되어 있다면, 각 환경에 맞춰 앱을 배포할 때마다 코드를 변경해야 하며, 이는 매우 번거로운 작업이 됩니다. 하지만, 환경 변수를 .env.production, .env.development, .env.test와 같이 설정해두었다면, 코드에서는 단지 다음과 같이 적용하기만 하면 됩니다.

const apiUrl = `${process.env.REACT_APP_BASE_URL}/api`;

이러한 방식으로 환경 변수를 사용하면, 배포 환경에 따라 쉽게 URL을 변경할 수 있으며, 코드의 가독성과 유지보수성도 개선됩니다.

실제 코드 응용과 관련해서는 다음 한글 아티클을 참고해보세요 !
=> 보러가기

Comment on lines +4 to +11
export interface authRequest {
email: string;
password: string;
}

export interface authResponse {
accessToken: string;
refreshToken: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

인터페이스나 타입 등은 보편적으로 파스칼 케이스를 사용합니다 😊:

Suggested change
export interface authRequest {
email: string;
password: string;
}
export interface authResponse {
accessToken: string;
refreshToken: string;
export interface AuthRequest {
email: string;
password: string;
}
export interface AuthResponse {
accessToken: string;
refreshToken: string;

Comment on lines +16 to +18
const result = await instance.post('/auth/sign-in', { email, password });

return result.data as authResponse;
Copy link
Collaborator

Choose a reason for hiding this comment

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

as보다는 제네릭을 사용해보는건 어떨까요?:

Suggested change
const result = await instance.post('/auth/sign-in', { email, password });
return result.data as authResponse;
const result = await instance.post<authResponse>('/auth/sign-in', { email, password });
return result.data;

타입 스크립트에서는 흔히 2가지 금기 사항이 있습니다. 💀

  1. any를 사용하지 말 것.
  2. 타입 어설션(as)을 사용하지 말 것.


return response.data as FolderData[];
} catch (error) {
if (axios.isAxiosError(error)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

오우 ~ 굳굳. 타입 가드를 적절히 사용하셨군요??

Comment on lines +14 to +28
export async function getLink() {
try {
return (await instance.get('/links')).data as Link[];
} catch (error) {
throw error;
}
}

export async function getLinkById({ folderId }: { folderId: number }) {
try {
return (await instance.get(`/folders/${folderId}/links`)).data as Link[];
} catch (error) {
throw error;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도 제네릭을 사용해볼 수 있겠네요 ㅎㅎㅎ

Suggested change
export async function getLink() {
try {
return (await instance.get('/links')).data as Link[];
} catch (error) {
throw error;
}
}
export async function getLinkById({ folderId }: { folderId: number }) {
try {
return (await instance.get(`/folders/${folderId}/links`)).data as Link[];
} catch (error) {
throw error;
}
}
export async function getLink() {
try {
return (await instance.get<Link[]>('/links')).data;
} catch (error) {
throw error;
}
}
export async function getLinkById({ folderId }: { folderId: number }) {
try {
return (await instance.get<Link[]>(`/folders/${folderId}/links`)).data;
} catch (error) {
throw error;
}
}

Comment on lines +19 to +37
const [{ data: userData }, { data: folderData }, { data: linkData }] = useQueries({
queries: [
{
queryKey: ['user'],
queryFn: getUser,
staleTime: 1000 * 60 * 60,
},
{
queryKey: ['folder', folderId],
queryFn: () => getFolderById({ folderId: folderIdNumber }),
staleTime: 1000 * 60 * 60,
},
{
queryKey: ['folderLinks', folderId],
queryFn: () => getLinkById({ folderId: folderIdNumber }),
staleTime: 1000 * 60 * 60,
},
],
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳 ! 리액트 쿼리 잘 사용하고 계시는군요 !

추가로 queryKey는 따로 파일을 만들어서 관리하는 것도 용이하답니다 !
다음과 같이요 😊:

const articleKeys = {
  all: ['articles'] as const,
  list: (page: number) => [...articleKeys.all, 'list', page] as const,
  favoriteList: (page: number) => [...articleKeys.all, 'favorite', page] as const,
  detail: (id: number) => [...articleKeys.all, 'detail', id] as const,
}

const {data} = useQuery(articleKeys.list(page), ()=> getArticle(page));

Comment on lines +63 to +80
<label className='w-full text-sm mb-3'>이메일</label>
<div className='relative w-full mb-6'>
<input
className={`w-full h-[60px] rounded-lg border ${
errors.email ? 'border-red-500' : 'border-[#ccd5e3]'
} text-base px-4 focus:outline-none focus:border-[#6d6afe] hover:border-[#6d6afe]`}
placeholder='이메일을 입력해 주세요.'
{...register('email', {
required: '이메일을 입력해 주세요.',
pattern: {
value: /^[a-zA-Z0-9+-\_.]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$/,
message: '올바른 이메일 주소가 아닙니다.',
},
})}
/>
{errors.email && (
<span className='absolute bottom-[-20px] left-0 text-red-500 text-sm'>{errors.email.message}</span>
)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

라벨을 연결해볼까요?:

Suggested change
<label className='w-full text-sm mb-3'>이메일</label>
<div className='relative w-full mb-6'>
<input
className={`w-full h-[60px] rounded-lg border ${
errors.email ? 'border-red-500' : 'border-[#ccd5e3]'
} text-base px-4 focus:outline-none focus:border-[#6d6afe] hover:border-[#6d6afe]`}
placeholder='이메일을 입력해 주세요.'
{...register('email', {
required: '이메일을 입력해 주세요.',
pattern: {
value: /^[a-zA-Z0-9+-\_.]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$/,
message: '올바른 이메일 주소가 아닙니다.',
},
})}
/>
{errors.email && (
<span className='absolute bottom-[-20px] left-0 text-red-500 text-sm'>{errors.email.message}</span>
)}
<label className='w-full text-sm mb-3' id="email">이메일</label>
<div className='relative w-full mb-6'>
<input
id="email"
className={`w-full h-[60px] rounded-lg border ${
errors.email ? 'border-red-500' : 'border-[#ccd5e3]'
} text-base px-4 focus:outline-none focus:border-[#6d6afe] hover:border-[#6d6afe]`}
placeholder='이메일을 입력해 주세요.'
{...register('email', {
required: '이메일을 입력해 주세요.',
pattern: {
value: /^[a-zA-Z0-9+-\_.]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$/,
message: '올바른 이메일 주소가 아닙니다.',
},
})}
/>
{errors.email && (
<span className='absolute bottom-[-20px] left-0 text-red-500 text-sm'>{errors.email.message}</span>
)}

<label><input> (en-US) 요소와 연결하면 몇 가지 이점이 있습니다:

  • label 텍스트는 텍스트 입력과 시각적으로 관련이 있을뿐만 아니라 프로그래밍적으로도 관련이 있습니다. 예를 들어, 화면리더기(screenreader) 는 폼 입력(form input)에서 label 을 읽어서 보조기술(assistive technology) 사용자가 입력해야하는 텍스트가 무엇인지 더 쉽게 이해할 수 있게 합니다.
  • 관련 label 을 클릭해서 input 자체에 초점을 맞추거나 활성화를 시킬 수 있습니다. (활성되어서)늘어난 누를 수 있는 영역(hit area)은 터치스크린 사용자를 포함해 입력하려하는 모든 사람에게 이점을 줍니다.

출처: MDN

src={pwCheckType === 'password' ? eyeOff : eyeOn}
width={16}
height={16}
alt=''
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳 ! 장식용 alt의 경우 빈 값으로 스킵할 수 있지요 😊:

장식 이미지는 페이지 콘텐츠에 정보를 추가하지 않습니다. 예를 들어, 이미지에서 제공하는 정보는 인접한 텍스트를 사용하여 이미 제공될 수도 있고, 웹 사이트를 시각적으로 더욱 매력적으로 만들기 위해 이미지가 포함될 수도 있습니다.

이러한 경우 스크린 리더와 같은 보조 기술에서 무시할 수 있도록 null(빈) alt텍스트를 제공해야 합니다( ). alt=""이러한 유형의 이미지에 대한 텍스트 값은 화면 판독기 출력에 청각적 혼란을 추가하거나 주제가 인접한 텍스트의 주제와 다른 경우 사용자의 주의를 산만하게 할 수 있습니다. 속성 을 생략하는 alt것도 옵션이 아닙니다. 속성이 제공되지 않으면 일부 화면 판독기가 이미지의 파일 이름을 대신 알려주기 때문입니다.

Decorative Images

}
);

Modal.displayName = 'Modal';
Copy link
Collaborator

Choose a reason for hiding this comment

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

오호 꼼꼼하시군요 👍

Comment on lines +38 to +64
};

return (
isOpen &&
createPortal(
<dialog ref={dialogRef} className='w-full h-full z-[999]' open>
<div className='fixed inset-0 bg-black bg-opacity-40'>
<div
className='fixed top-1/2 left-1/2 transform -translate-x-1/2 -translate-y-1/2 bg-white rounded-lg'
style={{ width, height, padding }}
>
<button
onClick={handleModalClose}
aria-label='CloseButton'
className='absolute top-4 right-4 bg-none border-none cursor-pointer'
>
<Image src={close} width={24} height={24} alt='Modal close button' />
</button>
<div className='flex justify-center w-full text-xl font-bold'>{title}</div>
{children}
</div>
</div>
</dialog>,
document.getElementById('modal-root')!
)
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Guard Clause 패턴을 사용해볼까요?:

Suggested change
};
return (
isOpen &&
createPortal(
<dialog ref={dialogRef} className='w-full h-full z-[999]' open>
<div className='fixed inset-0 bg-black bg-opacity-40'>
<div
className='fixed top-1/2 left-1/2 transform -translate-x-1/2 -translate-y-1/2 bg-white rounded-lg'
style={{ width, height, padding }}
>
<button
onClick={handleModalClose}
aria-label='CloseButton'
className='absolute top-4 right-4 bg-none border-none cursor-pointer'
>
<Image src={close} width={24} height={24} alt='Modal close button' />
</button>
<div className='flex justify-center w-full text-xl font-bold'>{title}</div>
{children}
</div>
</div>
</dialog>,
document.getElementById('modal-root')!
)
);
}
};
if (!isOpen) return;
return createPortal(
<dialog ref={dialogRef} className='w-full h-full z-[999]' open>
<div className='fixed inset-0 bg-black bg-opacity-40'>
<div
className='fixed top-1/2 left-1/2 transform -translate-x-1/2 -translate-y-1/2 bg-white rounded-lg'
style={{ width, height, padding }}
>
<button
onClick={handleModalClose}
aria-label='CloseButton'
className='absolute top-4 right-4 bg-none border-none cursor-pointer'
>
<Image src={close} width={24} height={24} alt='Modal close button' />
</button>
<div className='flex justify-center w-full text-xl font-bold'>{title}</div>
{children}
</div>
</div>
</dialog>,
document.getElementById('modal-root')!
)
}

Guard Clause을 사용하게 되면 조건부를 조기에 처리할 수 있게 되어 핵심 로직을 구분할 수 있다는 장점이 있습니다. 따라서 가독성을 향상시킬 수 있어요.

또한, 미리 조건에 따라서 데이터의 범위를 좁힐 수 있기에 핵심 로직에서는 보장받은 데이터를 편리하게 사용할 수 있다는 장점도 있어요 😊

@kiJu2
Copy link
Collaborator

kiJu2 commented Jul 1, 2024

수고하셨습니다 동혁님 !
제가 좋아하는 기술들로 구성하셔서 즐겁게 리뷰하였습니다 😊😊😊
리액트 쿼리도 처음이실텐데 능숙하게 적용하셨군요 👍
리뷰 중 궁금하신거 있으시면 사전 질의를 통해서 남겨주시거나 멘토링 미팅 때 질문주세요 ㅎㅎㅎ

@kiJu2 kiJu2 merged commit 3830cea into codeit-bootcamp-frontend:part3-오동혁 Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants