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

프로젝트 피드백 - 작성자: 류제천 튜터 #42

Open
rjc1704 opened this issue Jan 13, 2023 · 2 comments
Open

프로젝트 피드백 - 작성자: 류제천 튜터 #42

rjc1704 opened this issue Jan 13, 2023 · 2 comments

Comments

@rjc1704
Copy link

rjc1704 commented Jan 13, 2023

Overview

  1. 전반적으로 프로젝트 문서 구성이 정말 좋습니다.
  2. 프로젝트 컨셉 설명이 너무 좋습니다! 이 앱이 왜 필요한지에 대해 잘 설명해주셨어요. 로고 디자인까지 굿
  3. API 명세도 군더더기 없이 아주 깔끔합니다.
  4. 커밋 컨벤션 잘지켜주셨습니다 굿!
  5. react-query 사용에 대해서 커스텀훅으로 응용한 부분 인상 깊었습니다.
  6. useQuery의 select 옵션처럼 제가 강의에서 다루지 않은 부분도 직접 API문서 보고 필요한 부분 적용한 부분 굿굿!
  7. optimistic update와 같은 리액트쿼리에서 제공하는 고급 기술들까지 API 문서보고 적용하신 부분 굿!!
  8. 트러블슈팅 부분에 템플릿을 적용해서 팀원별로 구성을 통일시킨 부분 굿!!
  9. UI/UX 전반적으로 완성도가 높습니다 굿!
  10. 배포 전에 dev에서 main 브랜치로 머지 해주셨으면 더 좋았겠습니다.
  • 코드리뷰 코멘트는 아래에 남기겠습니다.
@rjc1704
Copy link
Author

rjc1704 commented Jan 13, 2023

image
regex 정의 부분은 utils 로 별도로 빼고 import 받아와서 사용만 하는 방식으로 리팩토링 하시면 더 좋겠습니다.

@rjc1704
Copy link
Author

rjc1704 commented Jan 13, 2023

image
setTimeout 함수는 이벤트리스너 함수와 마찬가지로 별도의 명령을 해주지 않으면 가비지 콜렉팅이 자동으로 이뤄지지 않아 메모리에 잔류하게 됩니다. 따라서 아래와 같은 형식으로 리팩토링 해서 언마운트 시에 setTimeout을 메모리에서 제거하는 방식을 도입해 보시면 좋습니다.

useEffect(()=>{
  const id = setTimtout();
  return () => {
    clearTimeout(id);
  }
}, [])

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

No branches or pull requests

1 participant