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

Part2 장성훈 week12 #406

Conversation

hoonj170214
Copy link

요구사항

기본

  • TypeScript를 활용해 프로젝트의 필요한 곳에 타입을 명시해 주세요.

주요 변경사항

  • 템플릿 코드에 typescript를 설치하고, tsconfig 설정을 했습니다.
  • 모든 코드를 typescript로 바꾸지는 못했습니다.

해결 못한 부분

  • FolderPage.tsx 에서 useGetFolders 훅, useAsync 훅의 타입을 잘 정의해준 것 같은데, | undefined 오류가 납니다. 그래서 FolderPage에서 data 부분에 오류가 납니다.
  • data를 Folders 인터페이스로 타입정의를 했을 때
스크린샷 2024-05-11 16 05 37
  • data를 타입정의하지 않았을 때
스크린샷 2024-05-11 16 05 52

@hoonj170214 hoonj170214 requested a review from kich555 May 11, 2024 07:12
@hoonj170214 hoonj170214 added the 미완성🫠 죄송합니다.. label May 11, 2024
@hoonj170214 hoonj170214 marked this pull request as ready for review May 13, 2024 02:21
Copy link
Collaborator

@kich555 kich555 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!!
전체적으로 너무 잘 해주셨어요!!

앞으로도 이대로만 계속 가주시면 좋을것 같네요 ㅎㅎ

Copy link
Collaborator

Choose a reason for hiding this comment

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

호오 hasOwnProperty 쓴거 좋습니다 ㅎ근데 위 조건일때만 값을 반환하는건 문제가 있을것 같아요
useGetFolder는 항상 loading, error, data상태는 반환해야 useGetFolder를 사용하는 쪽에서 문제가 없을것 같습니다
mapFolderData()를 꼭 사용해야만 한다면

if (data && data.hasOwnProperty('folder')) {
    const folderData = mapFolderData(data);
}

구문은 useGetFolder안이 아니라 useGetFolder 를 선언하는 쪽에서 사용하는게 좋을것 같네요 ㅎ

Component = ()=> {
const [folderData,setFolderData]=useState(null)
const {data,loading,error} = useGetFolder()

if (data && data.hasOwnProperty('folder')) {
    setFolderData(mapFolderData(data))
}

return <div> ...
}

이렇게요!


const cx = classNames.bind(styles);
interface FolderToolBar {
folders: ;
Copy link
Collaborator

Choose a reason for hiding this comment

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

잉 이부분은 eslint가 에러 안띄우나요?

혹시나 프로젝트에 eslint, prettier가 세팅안되어있다면 필수로 세팅해주세요!

interface FolderToolBar {
folders: ;
selectedFolderId: string;
onFolderClick: React.SetStateAction<string>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

그렇게 작성하시는것도 물론 좋지만 그냥 (string)=>void; 해도 되긴 합니다!

title: string;
description: string;
imageSource: string;
}[];
Copy link
Collaborator

Choose a reason for hiding this comment

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

보통 이러면

interface Link = {
    id: number;
    createdAt: string;
    url: string;
    title: string;
    description: string;
    imageSource: string;
  }

links: Link[]

이렇게 나눠주는게 좋아요!

import { mapLinksData } from 'link/util-map/mapLinksData';
import { useAsync } from 'sharing/util';
import { ALL_LINKS_ID } from './constant';
import { Link } from 'sharing/util';
Copy link
Collaborator

Choose a reason for hiding this comment

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

요긴 한칸 띄워두는게 더 좋겠네요

profileImageSource: string;
};
};
links: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 타입을 Link[]로 바꿔주는게 좋을것 같아요

export const useAsync = (
asyncFunction: () => Promise<AxiosResponse<any, any>>
) => {
const [loading, setLoading] = useState(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

const [loading, setLoading] = useState(false);

setData(response.data);
return response;
} catch (error) {
if (error instanceof AxiosError) setError(error.message);
Copy link
Collaborator

Choose a reason for hiding this comment

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

setError에는 error객체 자체를 주는게 더 나아보여요!

const rootElement = document.getElementById(ROOT_ID);
rootElement?.addEventListener('click', callback);
return () => {
rootElement?.removeEventListener('click', callback);
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍👍👍 unMount될 때 등록한 이벤트리스너 회수하는거 좋습니다!

import { useAsync } from 'sharing/util';
import { axiosInstance } from 'sharing/util';

interface UserData {
Copy link
Collaborator

Choose a reason for hiding this comment

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

UserData를 선언만하고 사용하는부분은 안보이네요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

만약에 이유가 있다면 주석을 통해 이유를 남겨주시면 더 좋을것 같아요!!

@kich555 kich555 merged commit 8a7ec11 into codeit-bootcamp-frontend:part2-장성훈 May 13, 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