-
Notifications
You must be signed in to change notification settings - Fork 37
[최성락] Sprint9 #295
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
[최성락] Sprint9 #295
The head ref may contain hidden characters: "Next-\uCD5C\uC131\uB77D-sprint9"
Conversation
Lanace
left a comment
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.
작업하느라 고생하셨어요ㅎㅎ
잘 하시길래 조금 꼼꼼하게 본거긴 해요!
지금 하신것도 충분히 잘하신거라...ㅋㅋ
오늘 멘토링때 조금 더 보강해서 설명드릴게여!
| .nickname { | ||
| font-size: 14px; | ||
| line-height: 24px; | ||
| color: #4b5536; | ||
| } | ||
|
|
||
| .date { | ||
| font-size: 14px; | ||
| line-height: 24px; | ||
| color: #9ca3af; | ||
| } |
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 const formatDate = (dateString: string): string => { | ||
| const date = new Date(dateString); | ||
| return date | ||
| .toLocaleDateString("ko-KR", { | ||
| year: "numeric", | ||
| month: "2-digit", | ||
| day: "2-digit", | ||
| }) | ||
| .replace(/\./g, ". "); | ||
| }; |
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.
물론 이렇게 해도 되긴 한데, 실무에서는 moment 나 dayjs 같은걸 좀더 많이 쓰긴 해요!
const value = dayjs(dateString);
return value.format("YYYY.MM.DD");
이런 느낌으로 쓸 수 있거든요ㅎㅎ
물론 지금 작성하신 코드도 맞아요..!
| import { formatDate } from "@/utils/formattedDate"; | ||
|
|
||
| export function ArticleItemCard({ title, content, image, likeCount, writer, updatedAt }: Article) { | ||
| const formattedDate = formatDate(updatedAt); |
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.
Format유틸로 잘 분리하셨네요ㅎㅎ!
굳굳
이런식으로 분리할 수 있으면 분리시켜주는게 좋고, 유틸성은 따로 관리하는게 좋아요!
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.
조금더 개선한다면... useMemo를 쓰는것도 좋을것같네요!
const formattedDate = useMemo<string>(() => {
return formatDate(updatedAt);
}, [updatedAt]);| import Image from "next/image"; | ||
| import { formatDate } from "@/utils/formattedDate"; | ||
|
|
||
| export function ArticleItemCard({ title, content, image, likeCount, writer, updatedAt }: Article) { |
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.
이렇게 하면 ArticleItemCard 를 사용하는쪽이 이렇게 되어야해요
<ArticleItemCard title={item.title} content={item.content} ... />물론 이렇게 할수도 있긴 하겠죠
<ArticleItemCard {...item} />근데 아에 타입을 따로 정의하면 좀더 사용하는사람에게 명시적으로 전달할 수 있어요
interface ArticleItemCardProps {
article: Article
}
...
<ArticleItemCard article={item} />|
|
||
| interface ArticleListProps { | ||
| initialArticles: FetchArticlesResponse; | ||
| initialOrder: "recent" | "like"; |
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.
좋네요ㅎㅎ! 딱 이거 2개만 올 수 있도록 처리하면 실수로 잘못된 값이 들어오는걸 막을 수 있을거에요!
| const [hasMore, setHasMore] = useState(true); | ||
|
|
||
| // IntersectionObserver | ||
| const observerTarget = useRef<HTMLDivElement | null>(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.
잘 하신것같아요!
observer가 동작하는 방식은 멘토링떄 같이 설명드릴게요.
그리구 성능에 그렇게 무리가 되진 않아요ㅎㅎ!
| const observer = new IntersectionObserver(onIntersect, { threshold: 0.1 }); | ||
| observer.observe(observerTarget.current); | ||
|
|
||
| return () => observer.disconnect(); |
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.
disconnect 시켜주는 부분까지 꼼꼼하시네요!
꼭 이벤트를 걸어줬다면 이벤트를 해지하는것도 쌍으로 처리해주세요~ㅋㅋ
useEffect에서요
| <div className={style.all_section}> | ||
| {articles.map((article) => ( | ||
| <ArticleItemCard key={article.id} {...article} /> | ||
| ))} | ||
| </div> |
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.
데이터가 없을떄도 같이 처리해주면 좋을것같아요ㅎㅎ
| <div className={style.post_header}> | ||
| <h3>게시글</h3> | ||
| <button onClick={handleClick}>글쓰기</button> | ||
| </div> |
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.
페이지 이동하는건 가급적이면 router를 쓰는것보단 Link를 쓰는게 좋아요ㅠ
미리 이동하기 전에 prefetching 을 하거든요ㅎㅎ
성능상 좋아요
| }: { | ||
| page?: number; | ||
| pageSize?: number; | ||
| orderBy?: "like" | "recent"; | ||
| keyword?: string; | ||
| }): Promise<FetchArticlesResponse> { |
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.
이것도 별도의 Request Type을 만들어주면 사용하기 편하실꺼에요ㅎㅎ!
SSR과 CSR일떄 장단점을 생각해보시면 좋을것같아요ㅎㅎ! 아마 CSR을 하고 있는 곳들이 조금 더 많을것같아요. |
지금 분리한 정도도 나름 잘 정리가 되어있다고 생각해요! 정답이 있는건 아닌데, 나눴을때 균질하게 나뉘는지, 정리가 잘 되는지 정도를 보고 판단해보시면 좋을것같아요! |
이부분은 딱히 성능상 문제가 있진 않을텐데, 라이브러리에 다른것들은 뭐가 있는지 보시는것도 좋을것같아요ㅎㅎ! |
2가지가 있을텐데, 하나는 브라우저에서 직접 서버 API를 호출할 수 있구요, 근데 저는 가급적이면 그떄마다 request / response 타입을 정의해서 쓰는걸 좀 더 선호하긴 해요ㅎㅎ! |

요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게