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

[김미소] Week13 #425

Merged
merged 3 commits into from
May 15, 2024
Merged

[김미소] Week13 #425

merged 3 commits into from
May 15, 2024

Conversation

nightowlzz
Copy link
Collaborator

요구사항

기본

  • next.js 로 변경 및 이전

심화

주요 변경사항

스크린샷

image

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@nightowlzz nightowlzz requested a review from kiJu2 May 13, 2024 08:59
@nightowlzz nightowlzz added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels May 13, 2024
@kiJu2
Copy link
Collaborator

kiJu2 commented May 15, 2024

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

@kiJu2
Copy link
Collaborator

kiJu2 commented May 15, 2024

커밋 메시지는 현재 커밋에서 어떠한 기록을 남길지를 말합니다:

Fix: 옮기변서 생긴 에러 수정 및 정리 에서 "어떤 에러를 수정했는가?"에 초점을 맞추어서 커밋을 작성해보면 어떨까요? 😊

다음은 커밋 메시지와 관련된 아티클입니다:

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

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

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

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

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

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

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

tl;dr

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

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

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

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

원문 보기

Comment on lines +10 to +14
export interface IImageSnsArr {
id: string;
src: string;
link: string;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

다음 타입은 배열이 아닌 것으로 보입니다 !

다음과 같이 Arr을 없애보는건 어떨까요?

Suggested change
export interface IImageSnsArr {
id: string;
src: string;
link: string;
}
export interface IImageSns {
id: string;
src: string;
link: string;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

또한, Footer 컴포넌트에서 IImageSns 타입을 반환하고 있습니다.
FooterIImageSns를 사용하고만 있는데 export까지 해주면 Footer.tsx의 역할이 많아질 것으로 우려됩니다 !
ImageSns 컴포넌트를 따로 만들어서 Props를 정의하여 export 하거나, 앱 내에 자주 사용될 것으로 보인다면 타입 파일을 따로 만드시는게 어떨까요?

Comment on lines +15 to +36
const imageSnsArr: IImageSnsArr[] = [
{
id: "Facebook",
src: "/assets/icon/icons_face.svg",
link: "https://www.facebook.com/?locale=ko_KR",
},
{
id: "Twitter",
src: "/assets/icon/icons_twt.svg",
link: "https://twitter.com/?lang=ko",
},
{
id: "YouTube",
src: "/assets/icon/icons_you.svg",
link: "https://www.youtube.com/",
},
{
id: "Instagram",
src: "/assets/icon/icons_ins.svg",
link: "https://www.instagram.com/",
},
];
Copy link
Collaborator

Choose a reason for hiding this comment

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

또한 타입 추론으로도 가능합니다.

만약, IImageSnsArr 타입이 재사용성이 낮다면 타입 추론을 활용해보시는 게 어떨까 싶어요 !

Comment on lines +38 to +39
function Header({userInfo}:IHeaderUserLoginInfoApi) {
const {pathname} = useRouter();
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
function Header({userInfo}:IHeaderUserLoginInfoApi) {
const {pathname} = useRouter();
function Header({ userInfo }: IHeaderUserLoginInfoApi) {
const { pathname } = useRouter();

사소한 줄바꿈, 띄어쓰기 등 코드를 작성하시다보면 자연스럽게 불규칙해지는 경우가 많아요.
이를 돕기 위해서 개발 커뮤니티에서는 개발 경험 향상을 위한 여러가지 도구들이 있는데요.

보편적으로 많이 사용되는 툴은 prettier입니다 !

Prettier: https://prettier.io/

prettier는 vscode plugin에서 install하고 실행할 수 있습니다 ! 🤗

macos 경우 option + shift + f
windows의 경우 alt + shift + f

@@ -0,0 +1,9 @@
import { IImageSnsArr } from "./Footer";

export default function SocialLinkButton({ id, src, link }: IImageSnsArr) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기서 Props타입을 정의해줘도 되겠군요 !

Suggested change
export default function SocialLinkButton({ id, src, link }: IImageSnsArr) {
export default function SocialLinkButton({ id, src, link }: { id: string, src: string, link: string }) {

해당 파일에서만 사용 되는 것으로 보여요. 그렇다면 Footer에서 타입 추론으로 객체를 정의해두고 해당 컴포넌트에 props를 내려줘도 될 것 같아요.

export default function SocialLinkButton({ id, src, link }: IImageSnsArr) {
return (
<a href={link} target="_blank">
<img src={src} alt={id + "이동 버튼"} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

alt는 대체 텍스트예요:

Suggested change
<img src={src} alt={id + "이동 버튼"} />
<img src={src} alt={id + " 아이콘"} />

alt는 스크린 리더 사용자에 대한 보조 텍스트가 될 수 있으므로 "어떠한 이미지 인지"를 작성해주는 것이 좋아요 !

alt의 목적

  • 인터넷 연결이 끊겼을 때 대체되는 이미지
  • 스크린 리더 사용자를 위한 대체 텍스트
  • 이미지를 볼 수 없는 환경에서 이미지를 대체하기 위한 텍스트
    등 목적을 알게 된다면 alt를 어떻게 사용하시면 될지 알 수 있을 것 같아요.

다음은 하버드 에듀케이션에서 제안하는 alt 규칙입니다:

tl;dr

  • Write Good Alt Text
  • Add alt text all non-decorative images.
  • Keep it short and descriptive, like a tweet.
  • Don’t include “image of” or “photo of”.
  • Leave alt text blank if the image is purely decorative
  • It's not necessary to add text in the Title field.

원문 보기

$buttonText: null,
$modalData: null,
});
const [searchContatn, setSearchContent] = useState<any>();
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
const [searchContatn, setSearchContent] = useState<any>();
const [searchContent, setSearchContent] = useState<any>();

이러한 오타나 영어단어 오표기는 vscode에 플러그인 하나만 설치하시면 손쉽게 해결할 수 있습니다 ! 🫠
변수에 영어가 틀리면 cSpell이라고 하는 오류가 출력됩니다. 😊

Code Spell Checker

Comment on lines +27 to +30
} catch (error) {
return {
notFound: true,
}
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
} catch (error) {
return {
notFound: true,
}
} catch (error) {
console.error('ERROR IN SERVER FETCHING DATA: ', error);
return {
notFound: true,
}

Comment on lines +18 to +19
const resTitle = await axios.get(`/folders/${query.id}`);
const resContent = await axios.get(`/links?folderId=${query.id}`);
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
const resTitle = await axios.get(`/folders/${query.id}`);
const resContent = await axios.get(`/links?folderId=${query.id}`);
const [resTitle, resContent] = await Promise.all([
axios.get(`/folders/${query.id}`),
axios.get(`/links?folderId=${query.id}`)
]);

이 전 패치를 기다렸다가 다음 패치를 진행하는 것 보다 한 번에 병렬처리 하는 것이 성능이 더욱 좋습니다 😊

Comment on lines +20 to +26
$title = resTitle.data;
$content = resContent.data;
if(!$title.data[0]) {
return {
notFound: true,
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

그리고 다음과 같이 작성하시면 let으로 스코프를 걱정할 우려가 없어보입니다:

Suggested change
$title = resTitle.data;
$content = resContent.data;
if(!$title.data[0]) {
return {
notFound: true,
}
}
const $title = resTitle.data;
const $content = resContent.data;
if (!$title || !$title.length) {
return {
notFound: true,
}
}
return {
props: {
$title,
$content
}
}

Comment on lines +33 to +38
return {
props:{
$title,
$content
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

제가 드린 코드로 한다면 요기는 지워도 될 것 같군요 !

@kiJu2
Copy link
Collaborator

kiJu2 commented May 15, 2024

수고 많으셨습니다 미소님 😊👍😊👍
코드리뷰 중 궁금하신 것 있으시면 사전 질의를 통해 남겨주세요 !

@kiJu2 kiJu2 merged commit 5aa655c into codeit-bootcamp-frontend:part3-김미소 May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants