-
Notifications
You must be signed in to change notification settings - Fork 0
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
4조 과제 제출(유지석, 소재헌, 윤준수, 우지수, 임예지) #3
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전체적으로 고생하셨고, 타입스크립트 사용하시려고 하신것도 인상깊게 봤습니다. 추가팁은 명시해드렸습니다
슬랙에 공통으로 고지드린 내용은 매번 코멘트는 생략했습니다
|
||
useEffect(() => { | ||
window.onbeforeunload = function pushRefresh() { | ||
window.scrollTo(0, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
신박하긴하네요 beforeunload에 pushRefresh를 넣어준 방식은 근데 return () => {} 형식으로 언마운트 또는 변경 이전에 window.beforeunload = null 해주면 좋겠어요
const App = () => { | ||
const [open, setOpen] = useState(false); | ||
|
||
const handleClickOpen = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React.useCalback 사용해서 최적화 시도해보세요
지금 함수 내용을 보니 마운트지점에 조치해주면될 것 같습니다
@@ -0,0 +1,8 @@ | |||
import axios from "axios"; | |||
|
|||
export const instance = axios.create({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그냥 instance보다 baseInstanc 등의 이름을 명시해보세요 :) 그러면 나중에 추후에 인증서버와 작업하거나 할때 인스턴스를 export하실때 훨씬 확장성에 좋을 것 같아요
그리고 create 내부에 들어가는 것들중 중복되는건 별도 객체로 만든 후 spread Operator와 결합해보세요
setError: React.Dispatch<React.SetStateAction<string>>, | ||
): void; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
보통 fetcher에서는 data 값을 반환해주고, 해당 fetcher를 사용하는쪽에서 주로 행위를 해주는데, 신박하네요
흠... res.data를 빼주고, 차라리 정확하게 response Type을 명시해서 Promise을 명시해주면 어떨지 생각해봅니다.
}, []); | ||
|
||
// // dummy data 로컬에 저장 | ||
localStorage.setItem( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
렌더 시점마다 호출할건가요?
뭔가 useEffect를 넣든 처리가 필요해보입니다.
totalResults: number; | ||
resultsPerPage: number; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
declaration Type을 명시해두고 export를 하는 것은 좋지 않습니다 typeRoots설정해서 exrpot 분리하시고, import없이 사용하세요
url: string | ||
width: number | ||
height: number | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위에서 말씀드린대로, export 제거해주세요
window.scrollTo(0, 0); | ||
}, [pathname]); | ||
return null; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
util내에서 커스텀훅을 쓰면 에러나 경고나올텐데요 사용처에 따라 달라질 수 있으니 이런경우에는 해당 유틸을 커스텀훅으로 만들어서 사용하세요
커스텀훅으로 사용시에 명명은 use로 사용하지 않으면 에러나 경고문구가 출력될겁니다. src/hooks로 담아보세요
} | ||
|
||
return `${Math.floor(betweenTimeDay / 365)}년 전`; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
util명을 함수별로 따시지말고 기능별로 따면 좋을 것 같아요. 단위 관련유틸 시간관련 유틸 이렇게요~
} | ||
} | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런건 videoUtil이 좋겠죠?
어려웠던 점 -> 어려우셨지만, 위 노력을 통해 많은 경험을 얻으셨을 것 같습니다. 💡궁금한 점 혹은 하나의 이펙트안에서 axios.all을 이용해 처리하는게 좋은지 더미데이터를 useState의 기본값에 할당을 해주어서 처음 렌더링 할때 더미데이터 값이 먼저 렌더링 되고 그 다음에 api에서 받아온 값이 렌더링 되는데 고칠 수 있는 방법이 있는지 |
프로젝트 소개
유튜브 API를 활용한 유튜브 클론
배포 사이트
불사조 유튜브
팀원
🔨기술 스택
📆 과제 기간 및 담당 업무
과제 기간: 2023. 1. 16 ~ 2022. 1. 20
🎈기능 구현
🔔어려웠던 점
💡궁금한 점