강원대 FE_정다연 2주차 STEP1 #45
Conversation
dandamdandam
commented
Jul 5, 2024
•
dandamdandam
commented
- 참고) 2주차 과제 STEP 2,3
- README
prettier --write --config ./.prettierrc './src/**/*.tsx'
header ui가 안보여서 추가로 고쳤습니다..
link(a) 디자인 바꿈
themeKey를 한국어 그대로 사용했더니 특수문자까지 라우팅하는 문제가 생김. 사용자에게 보여주는 표현은 label로 두어 해결
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "singleQuote": true, | |||
| "semi": true, | |||
| "tabWidth": 2, | |||
| "tabWidth": 4, | |||
There was a problem hiding this comment.
4로 바꾸게 된 이유가 있으신가요? 어떤 게 더 옳다거나 하진 않지만 자바스크립트 진영에선 2가 국룰이라서요 ㅋㅋㅋ
There was a problem hiding this comment.
width 4가 개인적으로 보기 편해서 바꿨습니다! JS에서는 2가 국룰인건 처음 안 이야기네요ㅋㅋ 개인프로젝트니 제 편의상으로 맞추고 싶은데 괜찮을까요?
There was a problem hiding this comment.
넵 프리티어 설정이 일반적으로 그렇다는 거지 당연히 정답이 있는 부분은 아닙니다!
| - 같이 편집되는 빈도 수가 높은 것 끼리 묶으려 하니까 컴포넌트를 page에 두어야 할지 component에 두어야 할지 판단이 잘 서지 않습니다. | ||
| - 전역적으로 사용되는 component만 component에 넣으면 되는 걸까요? | ||
| - RankingView같은 경우에는 타입선언 파일마저 한 폴더에 넣었는데, 많이 낮선구조로 보이나요..? | ||
| - 스타일을 지정할 때 flex에 많이 의존하는 편인데, 성능저하가 되거나 코드의 가독성이 떨이지는 등의 문제는 없을까요? |
|
|
||
| ### 궁금한 점 | ||
|
|
||
| - 같이 편집되는 빈도 수가 높은 것 끼리 묶으려 하니까 컴포넌트를 page에 두어야 할지 component에 두어야 할지 판단이 잘 서지 않습니다. |
There was a problem hiding this comment.
먼저 디렉토리 및 파일 구조에 대한 방법론은 워낙 다양해서 무엇이 일반적이라고 말하기가 어렵다는 전제를 깔고 시작하겠습니다.
개인적으로 저는 지금 다연님이 하신 방식(같은 관심사를 가진 파일들을 묶는)을 선호하는 편입니다. 이를 colocation이라 합니다. 참고
한편 컴포넌트를 만들 때 미래는 '적당히' 생각해야 합니다. 지나치게 확장성을 고려하면 오버엔지니어링을 하게 될 위험이 있어요. 제가 '적당히'를 추구하는 방식은 아래 예시와 같은 식입니다.
메인 페이지에서 쓸 랭킹뷰 컴포넌트를 만들어야겠군. 그런데 랭킹뷰가 메인 페이지가 아닌 다른 곳에서도 쓰일 수 있으니까 커먼 폴더에서 관리해야 하나? 음 아직 알 수 없으니 일단 페이지 폴더에서 관리하자. 그러다가 나중에 다른 페이지에서도 쓰이면 그때 옮기지 뭐.
반면 버튼 컴포넌트처럼 누가봐도 여기 저기서 쓰일 컴포넌트라면 처음부터 커먼 폴더에서 관리하는 편이 낫겠죠.
There was a problem hiding this comment.
예시까지 들어서 답변해주셔서 감사합니다! 어떤 느낌인지 조금 더 감을 잡을 수 있게 된 것 같아요
| ); | ||
| }; | ||
|
|
||
| const Footar = styled.footer({ |
|
|
||
| const handleSubmit = (event: React.FormEvent) => { | ||
| event.preventDefault(); | ||
| sessionStorage.setItem('user', username); |
There was a problem hiding this comment.
로그인 상태를 관리하기 위해 세션 스토리지를 사용하기로 결정하셨군요. 어떤 이유로 세션 스토리지가 다른 옵션들에 비해 낫다고 생각하셨나요?
There was a problem hiding this comment.
초반에는 쿠키로 저장해야겠다는 생각을 했습니다. 도메인 단위로 데이터 저장이 되어, 유저가 새 탭으로 브라우징 해도 로그인 정보가 유지되기 때문입니다.
그런데 예시페이지를 뜯어보다가 sessionStorage로 저장하는 것을 발견하고 일단 그대로 따르기로 했습니다...!
| * @param targetType TargetType 필터링 | ||
| * @param rankType RankType 필터링 | ||
| * @returns Ranking이 매겨진 상품 리스트 | ||
| */ |
There was a problem hiding this comment.
jsdoc 활용법을 학습하셨나 보네요. 좋습니다!
다만 코드 그 자체로 충분한 설명이 될 수 있도록 하고, 주석은 예외적인 상황에서만 사용되어야 하는 것을 유념해주세요.
한편 isDetail이라는 이름이 다소 모호하게 느껴집니다. 토글 상태라기보단 디테일 여부로 읽힐 여지가 있는 것 같습니다.
There was a problem hiding this comment.
a3f03c0
해당커밋에서 수정했습니다!
커밋 메시지를 수정하다가 footer typo 수정 커밋과 내용이 섞여버렸는데, 결국 분리하는데 실패해서 이대로..올립니다.,.
| export default ({ isDetail, targetType, rankType }: Props) => { | ||
| const initItems = () => { | ||
| const items = []; | ||
| for (let i = 0; i < 20; i++) { |
There was a problem hiding this comment.
이러한 단순 반복문은 사이드이펙트에 대한 우려 떄문에 현대 프로그래밍에서 지양되는 방법입니다. 사이드 이펙트를 발생시키지 않는 좀 더 나은 배열 생성법은 없을까요?
| import { themeKeys } from '../Theme'; | ||
|
|
||
| export default () => { | ||
| const ThemeBtn = ({ themeKey }: { themeKey: string }) => { |
There was a problem hiding this comment.
Button과 Btn을 혼용해서 쓰는 것보단 하나만 사용하는 것이 통일성과 가독성 측면에서 더 나을 것 같습니다.
There was a problem hiding this comment.
btn -> button으로 수정했습니다!
| import ThemeHeader from './ThemeHeader'; | ||
|
|
||
| export default () => { | ||
| const themeKey = useParams().themeKey ?? ''; |
| if (!themeKeys[themeKey]) { | ||
| navigate('/error/404'); | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
린트 룰은 가급적인 무시하지 말아야 합니다. 만약 무시해야만 내가 의도한 대로 코드가 동작한다면 십중팔구는 구조를 잘못짰을 확률이 높습니다. 왜 추가하게 되셨나요?
There was a problem hiding this comment.
해당 린트 에러는 useEffect의 callback내에서 사용하는 themeKey와 navigate 상태가 dependency array에 추가되지 않아 발생한 에러입니다. dependency array는 그 안에 있는 변수가 변화할 경우 useEffect의 callback을 다시 실행시켜준다고 알고 있습니다.
하지만 themeKey와 navigate의 값은 변화할 가능성이 없습니다(11번~12번 줄에서 값을 지정하고 있기 때문에). 그리고 찾아본 적이 없어 근거는 없지만 왠지 dependency array에 변화할 일이 없을 변수를 추가해두면 그것을 감시하는데 쓰는 메모리는 불필요하고 결국 코스트 낭비가 되는 것이 아닐까..라는 생각에 린트룰을 무시하도록 주석을 추가했습니다.
There was a problem hiding this comment.
그 정도 코스트는 무시하셔도 무방합니다. 그 비용보다는 린트 룰을 무시함으로써 컨벤션을 저해하는 게 더 큰 코스트가 될 가능성이 높아요.
첨언하자면, 리액트를 사용하면서 성능을 '그렇게까지는' 고려하지 않으셔도 괜찮습니다. 대부분은 알아서 똑똑하게 잘해줘요. 특히 뭔가 메모리 낭비에 대한 우려가 기우인 경우가 많습니다. 가장 대표적인 예시로 useMemo 남용이 있습니다. (https://javascript.plainenglish.io/stop-using-usememo-now-e5d07d2bbf70)
There was a problem hiding this comment.
useMemo 사용애 대한 아티클 추천 감사합니다. 고민하고 있던 내용이였는데 큰 도움이 됐습니다. 수정했습니다!
907eb61
