Conversation
브랜치를 잘못 생성한 상태로 진행해서 다시 브랜치를 생성함 아래는 이전 커밋의 내용만을 가져온 것임 feat: Nav 기능 구현 완료 styles: Nav.module.css 스타일 작성 완료 style: global.css 작성 chore: Home CSS 파일 경로 수정 chore: 기본적인 파일 구조 세팅 chore: sprint8 디렉토리 생성 & next.js 설치 & 필요 없는 파일 정리
-main-color로 지정돼 있지 않은 모든 색 변수로 지정 -현재 경로에 따라 header 글씨 색 바뀌지 않는 것 수정
Jin-coding-333
added
미완성🫠
There was a problem hiding this comment.
대체적으로 잘 작성해주신것 같습니다. 프론트엔드에서의 최적화는 항상 고민해봐야할 과제이며 컴포넌트를 어떻게 분리할지, 어떻게 관리할지에 대한 고민도 항상 따라오는 문제입니다.
next라는 새로운 스펙을 학습하시면서 고민해봐야 할 것은 next가 우리에게 어떤 이로움을 제공하고, 이러한 프레임워크를 사용하려면 어떤것들을 지켜야하는지에 대한 고민이 필요합니다.
때문에 라이브러리(react)를 사용하는것은 크게 제약이 없지만 프레임워크(next)를 사용하는것에는 제약이 있는만큼 공식문서를 (pageRouter 기준으로)다시한번 숙지하시고 사용해주시면 좋을 것 같습니다.
| import styles from "@/styles/pages/404.module.css"; | ||
| import Link from "next/link"; | ||
|
|
||
| function NotFound() { |
There was a problem hiding this comment.
404페이지를 따로 구성해주신거 좋습니다. 유효하지 않은 router를 제어하기위한 방식으로 사용되고 있고, 실제로도 이렇게 사용하고 있습니다.
다만 인지하고 계실수 있지만 next pageRouter기준으로 404페이지는 SSG형태로 구성됩니다. 즉 Static페에지로 빌드타임에 결정되고, 그내용을 유저에게 노출하는것 입니다. 원래 SSG로 제공하려면 getStaticProps함수를 사용해야하지만 404는 예외적으로 강제 static으로 사용된다는 점 입니다.
추후에 404페이지 이지만 동적으로 컨트롤 해야한다(유저도메인에 대한 국제화, 테마 같은 값으로 제어)면 대안이 필요할 수 있습니다
There was a problem hiding this comment.
404페이지는 강제로 static이 되는지 몰랐네요. 인지하겠습니다.
sprint8/pages/ArticleDetail/[id].jsx
Outdated
| import Comments from "@/components/ArticleDetail/Comments"; | ||
| import NoneComments from "@/components/ArticleDetail/NoneComments"; | ||
|
|
||
| function ArticleDetail() { |
There was a problem hiding this comment.
페이지라우터에 조건에맞게 라우팅을 구성해주신 것 좋습니다.
다만 우리가 웹서비스에서 url을 제공할때는 통용되는 규칙이 존재합니다.
아래 링크의 내용중 위반된 내용을 정리하자면 많은 규칙중 url Path는 소문자로 작성해야된다는 점이 있습니다.
추가자료
There was a problem hiding this comment.
page 디렉토리 안의 파일 명을 모두 소문자와 하이픈(-)으로 수정 했습니다.
sprint8/pages/ArticleDetail/[id].jsx
Outdated
|
|
||
| articleLoadHandler(); | ||
| commentLoadHandler(); | ||
| }, [articleId, commentPost]); |
There was a problem hiding this comment.
컴포넌트를 구성해주시면서, 서버로의 API요청과 UI(JSX)를 구분해주신것 아주 좋습니다. 더 나아가서 이러한 내용들을 customHooks를 이용해서 파일자체를 분리시켜 관리하는법도 존재합니다.
분리된 파일에서 business, ui요소를 각각처리(관심사분리)하고, UI에서는 명시적인 함수를 통해 business를 제어할 수 있습니다.
현재는 파일크기가 크지않아 와닿지 않을 수 있지만 '컴포넌트를 가능한 작게유지하는것'에 대해서 고민이 필요할 수 있습니다.
There was a problem hiding this comment.
뭔가 깔끔하게 된 것 같지는 않지만 커스텀 hook을 만들어 관심사를 분리했습니다.
sprint8/pages/ArticleDetail/[id].jsx
Outdated
| commentLoadHandler(); | ||
| }, [articleId, commentPost]); | ||
|
|
||
| if (!article) return <p>Loading article...</p>; |
There was a problem hiding this comment.
로딩처리를 위해 conditional 렌더링을 해주신것도 좋습니다!
다만 로딩이란 의미를 확인해보시면, API를 호출하고 네트워크를 통해 도착하지않은 Promise에 대해서 로딩처리를 해주셔야 한다는 점입니다. article은 조회했을때 0개 즉 서버요청에대한 응답값이 없을수도 있습니다. 이는 로딩상태가 아닌 no contents 상황일 수 있다는 점입니다.
async를 동작시키고 응답이오기전까지 를 로딩으로 생각해보신다면 로딩을 처리하는 더 좋은 방법이 존재할 것같아요
There was a problem hiding this comment.
네 로딩 중일 때와 에러가 났을 때, 응답 값이 없을 때 3가지 경우로 수정했습니다.
sprint8/pages/ArticleDetail/[id].jsx
Outdated
| return ( | ||
| <div className={styles.ArticleDetailBox}> | ||
| <ArticleInfo article={article} /> | ||
| <CommentPost articleId={articleId} commentPost={setCommentPost} /> |
There was a problem hiding this comment.
컴포넌트를 작게 파일단위로 분리해주신것도 아주 좋습니다.
위 리뷰와 이어서 피드백을 드린다면, 분리된 컴포넌트에서 여전히, article, commentPost에 대한 상태가 공통으로 사용해야하는 것 이기때문에 가장 상위에서 state를 관리해주신걸로 확인이 됩니다. 이는 리액트에서 자주 사용하게되는 presentational-container패턴으로 설명할 수 있는데요, 이는 명백한 단점이 존재하긴 합니다.
때문에 위에서 말씀드린것 처럼 (API호출을 분리한)business로직을 분리하는것을 설명드릴 수 있는데요, 분리된 business로직에서는 각각의 컴포넌트에서 재사용할 수 있습니다.
이렇게된다면 또다시 문제가 발생할 수 있는게, 컴포넌트 마다 API호출을 해야한다는 점이 있을수 있어요. 분리된 3개의 컴포넌트에서 서버로부터 받아와야 할 값이 필요하다면 3번 요청해야한다는 점 입니다. 이때바로 지금 학습하시는 react-query의 대안이 필요하게 될 것입니다.
이처럼 기술이 나오게되는 배경과 흐름에대해서 이해하시면 기술, 스펙을 습득하시는데 좀더 도움이 될 수 있다고 생각이됩니다.
There was a problem hiding this comment.
음.. 현재는 [id].js에서 article이든 comment이든 데이터를 불러와서 하위 컴포넌트로 내려주고 있는데 그렇게 하는 것보다 하위 컴포넌트 내에서 따로 따로 데이터를 불러오는 게 좋고 그랬을 때 나올 수 있는 문제 점을 react-query가 보완해준다는 말씀이시죠?
깊이가 부족해서 아직 완전히 이해가 되진 않지만 react-query를 앞으로 써보면서 이해가 될 것 같습니다.
일단 알고 넘어가겠습니다.
근데 presentational-container패턴의 단점에 대해서는 아직 잘 몰라서 언젠가 설명을 해주셔도 좋을 것 같네요.
sprint8/pages/ArticleDetail/[id].jsx
Outdated
| <button className={styles.toCommunityFeedButton}> | ||
| 목록으로 돌아가기 | ||
| <div className={styles.toCommunityFeedButtonImage}> | ||
| <Image |
There was a problem hiding this comment.
next/image를 사용해주신것도 좋습니다!
nextjs가 이미지 컴포넌트를 따로 제공하는이유, 최적화하는 방식과 종류를 나중에 설명할 수 있으면 좋을것 같네요.
sprint8/pages/ArticlePost.jsx
Outdated
| } | ||
| }; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
이러한 종류의 UI는 form형태로 구성해주시는것이 좋습니다. HTML의 form은 기본기능만으로도 제공되는게 많은데요, 그이점을 충분히 사용할 수 있기 때문입니다.
또한 현재 작성해주신 컴포넌트기준으로 상태를 관리하는것을 본다면, 하나의 input에서 이벤트가 발생할 때마다 나머지 input도 모두 리-렌더링이 되는걸 확인할 수 있습니다. 이런부분이 리액트에서 최적화를 고민하게 되는 부분이기도 합니다. 내가 원하는 곳만 리-렌더링 시키는 방식을 말이죠.
'컴포넌트를 가능한 작게 유지' 하는것이 상태를 원하는곳만 사용하도록 세팅하는 것 이고, 그렇기때문에 form으로 작성될때의 이점이 있을 수 있다는점 다시한번 설명드립니다.
There was a problem hiding this comment.
네 일단 인지하겠습니다!
sprint9 미션을 하면서 한번 적용해봐야 겠네요.
| const [textareaValue, setTextareaValue] = useState(""); | ||
|
|
||
| const handlePostComment = async (e) => { | ||
| e.preventDefault(); |
There was a problem hiding this comment.
e.preventDefault()는 javascript의 이벤트의 기본동작을 막거나 이벤트의 전파를 막는데 사용됩니다. 때문에 사용한 이유가 명확해야하는데요, 코멘트 확인하시면 해당 로직을 작성한 이유를 답변해주시면 좋을것 같습니다.
There was a problem hiding this comment.
강의 보고 적용하기 바빠서요.. 보고 적용한다고 했는데 다시 확인해보니 form 태그의 기본 동작인 쿼리 스트링과 함께 이동하는 것을 막기 위해 썼던 코드네요..ㅎ 필요 없는 코드라 관련된 모든 코드를 수정했습니다!
sprint8/components/ui/Footer.jsx
Outdated
| @@ -0,0 +1,59 @@ | |||
| import Link from "next/link"; | |||
There was a problem hiding this comment.
header와 footer를 컴포넌트형태로 분리해주신것 좋습니다. 다만 넣어주신 디렉토리 이름이 ui인데 좀더 명확한 이름(layouts?)으로 사용되는것이 좋을 수 있습니다.
또한 이러한 정적 컴포넌트들은 next의 SSR SSG형태로 제공하는것을 고민해보시는것이 필요할 수 있습니다.
There was a problem hiding this comment.
디렉토리 이름은 layouts으로 바꿨습니다. 더 명확한 이름이 있는지 몰랐네요!
Header와 Footer와 같은 지극히 정적인 컴포넌트에 대해 왜 SSR이나 SSG형태를 고민해야 할까요?
기본적으로 Header와 Footer는 SSG로 생성되고 데이터를 fetch해야 하지도 않아서 괜찮은 거 아닌가요??
어떤 이유로 어떤 차이가 있어 SSR 또는 SSG 형태로 제공해야 하는지 궁금합니다.
Footer와 Header가 있는 디렉토리는 layout으로 하는 게 낫다
Jin-coding-333
added
최종 제출
📋 스프린트 미션 요구사항
배포 사이트(배진한_Sprint8)
기본 요구사항
공통
자유 게시판 페이지
게시글 등록 & 수정 페이지
게시글 상세 페이지
심화 요구사항
공통
멘토에게
스크린샷
404페이지
게시글 목록 페이지
게시글 상세 페이지(댓글0)
게시글 상세 페이지(댓글X)
홈 페이지
