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

[김미소]week15 #484

Merged
merged 5 commits into from
May 29, 2024
Merged

[김미소]week15 #484

merged 5 commits into from
May 29, 2024

Conversation

nightowlzz
Copy link
Collaborator

요구사항

기본

  • page 라우터 app라우터로 변경 수정중

스크린샷

image

멘토에게

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

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

kiJu2 commented May 29, 2024

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

const LINKS = '/users/1/links';

// folder name
export async function getFolderProps() {
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 async function getFolderProps() {
export async function getFolders() {

PropsProperties의 줄임말인데요 ! 폴더의 속성들을 불러오는 것으로 보이지는 않아서..
"폴더들을 불러온다" 라는 함수로 getFolders가 어떤지 제안드리고 싶어요 😊

Comment on lines +9 to +11
const res = await instance.get(FOLDERS);
const { data } = res.data;
return data;
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 res = await instance.get(FOLDERS);
const { data } = res.data;
return data;
const { data } = await instance.get(FOLDERS);
return data;

// folder name
export async function getFolderProps() {
try {
const res = await instance.get(FOLDERS);
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 res = await instance.get(FOLDERS);
const res = await instance.get<GetFoldersResponse>(FOLDERS);

API 도큐먼트를 보고 반환 타입을 지정해주는게 어떨가 싶어서 제안드립니다 =)

Comment on lines +12 to +15
} catch (error) {
console.log('ERROR IN SERVER FETCHING DATA: ', error);
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 되면 해당 함수의 반환 타입은 | undefined가 될 것 같아요 =)

적절한 에러 처리가 되지 않았다면, 어떤 에러인지 명시하는게 어떨까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

(이어서) API 통신부에서 에러가 났을 때 어떻게 대처하면 될까요?

제가 예상하는 문제는 로그인을 하는데 "이미 등록된 아이디임"과 "아이디에 특수문자 들어가면 안됨" 이라는 에러 UI를 출력한다고 생각해봅시다..!

현재 error를 그냥 undefined로 반환해주고 있기에 해당 함수를 사용하는 컴포넌트에서는 분기처리하기 힘들거예요 !

그렇다면 어떻게 할까요?

방법은 다양합니다만, 지금 바로 해볼 수 있는 방법은 throw를 해보는거예요:

Suggested change
} catch (error) {
console.log('ERROR IN SERVER FETCHING DATA: ', error);
return;
}
} catch (error) {
console.error(`ERROR IN SERVER FETCHING DATA: ${error}`); // 통신부에서 처리할 로직 및 로깅
throw(error);
}

위처럼 throw를 해준다면 서버에서 보내준 에러의 메시지를 사용자에게 toast든, 모달이든, 알러트든 보여줄 수 있겠죠?

다음과 같이요 !!:

  // Component
  useEffect(() => {
    try {
      getFolders();
  // 이어서..
    } catch (err) {
      alert(err.message) 
    }
  }, [])

const { data } = res.data;
return data;
} catch (error) {
console.log('ERROR IN SERVER FETCHING DATA: ', error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

console은 생각보다 다양한 메서드가 있어요.

Suggested change
console.log('ERROR IN SERVER FETCHING DATA: ', error);
console.error('ERROR IN SERVER FETCHING DATA: ', error);

console.log 말고도 console.error, console.dir, console.countconsole은 개발자들이 모르는 많은 메써드들이 있어요 !

이번은 실수일 수도 있겠지만 디버깅 할 때 유용하므로 이번 기회에 console에 대해서 mdn 문서를 한 번 보시는 것도 도움이 될 것 같아요. =)
mdn console

Comment on lines +19 to +24
const {
register,
handleSubmit,
formState: { errors },
setError,
} = useForm<IJoinForm>({ mode: 'onBlur' });
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳 ~! 훅 폼을 통해서 비제어로 만들었군요 !

이렇게 되면 성능도 훨씬 좋아지겠어요 👍

const router = useRouter();
const [searchContatn, setSearchContent] = useState<any>();

console.log('menuData', menuData);
Copy link
Collaborator

Choose a reason for hiding this comment

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

console.log를 남겨 두면 추 후 디버깅이 어려울 수 있어요 !

다음 아티클을 읽어보시고 git add -p를 사용해보시겠어요 ? ☺️

git을 좀 더 꼼꼼하게 사용해보자 !

git add {path} -p 옵션

git add . -p를 사용하게 되면 변경사항을 스테이징에 올릴 때 파일 내 코드 단위로 잘라서 올릴 수 있습니다 ! 상당히 유용하므로 히스토리를 신경쓰신다면 꼭 사용해보세요 😊

어떻게 사용하지?

git add . -p

git commit -v

변경 사항을 커밋하기 전에 마지막으로 검토할 수 있습니다 !

git add -p와 git commit -v의 사용

} catch (error) {
console.log('ERROR IN SERVER FETCHING DATA: ', error);
}
revalidatePath('/folder');
Copy link
Collaborator

Choose a reason for hiding this comment

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

오홍 캐시 초기화를 사용하셨군요? 🤔

import { redirect } from 'next/navigation';
import SharedClient from './SharedClient';

export async function getSharedProp(id: string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

여긴 page인데 해당 함수를 API 함수를 export 하고 있는 것으로 보여요.

이렇게 되면 해당 파일에서의 역할이 모호(API와 UI 를 책임짐)해질 수 있다고 생각이 듭니다 !
따로 파일을 분류하시는건 어떻게 생각하세요 ?

Comment on lines +6 to +9
title: {
template: '%s | Linkbrary',
default: 'Linkbrary',
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

(질문)%s가 무엇인가요?

도큐먼트 봐도 잘 나오지 않아서.. 제가 모르는거라 그런데 궁금해서 남겨봅니당 !

@kiJu2
Copy link
Collaborator

kiJu2 commented May 29, 2024

훌륭합니다 ! 정말 많이 변했군요 ㅎㅎㅎ 고생 많이하신게 눈에 보입니다.
미완성이더라도 차곡차곡 자신만의 스프린트 미션을 수행하고 있는 모습이 정말 보기 좋네요 😊😊😊

다음 코드도 정말 궁금합니다 ! 수고많으셨어요 미소님.

@kiJu2 kiJu2 merged commit e9ff857 into codeit-bootcamp-frontend:part3-김미소 May 29, 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