[정남영] 스프린트미션9 [merge안해서 미션8이랑 겹쳐진상태]#71
Hidden character warning
[정남영] 스프린트미션9 [merge안해서 미션8이랑 겹쳐진상태]#71rl0425 merged 15 commits intocodeit-sprint-fullstack:next-정남영from
Conversation
rl0425
left a comment
There was a problem hiding this comment.
고생많으셨습니다.
전체적으로 사용하지 않는 div 태그, Fragment와
디버깅 코드는 제거하시고 올리시면 좋을 것 같습니다.
또한, 변수명 설정에 있어서 카멜케이스를 사용하시지 않는 경우와 실제 데이터와 변수명의 이름이 서로 불일치하는 경우가 있어보여, 이 부분을 수정하시면 더 좋은 코드가 될 것 같습니다.
수고하셨습니다!
| import axios from "axios"; | ||
| import { postBoard } from "@/pages/api/product"; | ||
|
|
||
| export default function usePostBoard() { |
There was a problem hiding this comment.
현재 코드는 커스텀 훅이라기보다는 단순한 래핑 함수인 것 같습니다. 실제로 postBoard()가 필요하면 usePostBoard()를 사용하는 것과 postBoard()를 그대로 불러오는 것이 차이가 없습니다. 따라서, 현재로서는 커스텀 훅의 의미가 거의 없기 때문에 로딩/에러 상태를 추가하시는게 좋을 것 같습니다.
export default function usePostBoard() {
const [loading, setLoading] = useState(false);
const [error, setError] = useState(null);
const submitPost = async (title, content) => {
setLoading(true);
setError(null);
try {
const res = await postBoard(title, content);
return res;
} catch (err) {
setError(err);
throw err;
} finally {
setLoading(false);
}
};
return { submitPost, loading, error };
}
Util/useArticlesList.js
Outdated
| @@ -0,0 +1,12 @@ | |||
| import { getAricleList } from "@/pages/api/product"; | |||
There was a problem hiding this comment.
오타가 나신 것 같습니다.
getAricleList -> getArticleList
| import { useEffect, useState } from "react"; | ||
|
|
||
| export default function useArticleList() { | ||
| const [articleList, setarticleList] = useState([]); |
There was a problem hiding this comment.
카멜케이스를 사용하시는 거면
setarticleList -> setArticleList 로 바꾸시는게 좋을 것 같습니다.
Util/useArticlesList.js
Outdated
| const [articleList, setarticleList] = useState([]); | ||
|
|
||
| useEffect(() => { | ||
| getAricleList().then((data) => setarticleList(data)); |
There was a problem hiding this comment.
전반적으로 커스텀 훅이 단순히 Api를 불러오기만 하는 코드이기 때문에, 위에서 말씀드린 것 처럼 로딩과 에러 처리를 전반적으로 처리해주셔야 할 것 같습니다.
component/board/board.js
Outdated
| import style from "@/styles/component.module.css"; | ||
| export default function Board() { | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
불필요한 Fragment는 제거해주시면 될 것 같습니다.
| </div> | ||
| </div> | ||
| ))} | ||
| </> |
There was a problem hiding this comment.
여기도 사용하지 않는 Fragment는 제거해주시면 될 것 같습니다.
| {/* 베스트이미지 */} | ||
| <Image | ||
| src={ic_medal} | ||
| style={{ width: 30, height: 30 }} |
There was a problem hiding this comment.
<Image> 태그는 크기와 넓이를 정하실 때, style 대신 속성을 사용하시면 됩니다.
<Image
src={ic_medal}
width={30}
height={30}
alt="메달이미지"
/>
component/board/boardItem.js
Outdated
| <div className={style.BoardItemContainer}> | ||
| <div className={style.boardItemBetween}> | ||
| <div> | ||
| <p>{content}</p> |
There was a problem hiding this comment.
content가 없을 경우의 예외처리도 해주시면 좋을 것 같습니다.
| const router = useRouter(); | ||
|
|
||
| const handleClick = () => { | ||
| setmodal(true); |
There was a problem hiding this comment.
이렇게 작성하지 마시고,
const handleClick = () => {
setModal(prev => !prev);
};이렇게 바꾸셔야 할 것 같습니다.
pages/detail/[id].js
Outdated
| </div> | ||
| <div className={style.detailCommentList}> | ||
| {/* 댓글리스트 */} | ||
| {commentList.map((comment) => { |
There was a problem hiding this comment.
여기서 사용하시는 comment라는 변수명과 실제 위에 useState로 사용하시는 comment 변수명이 일치하기 때문에, 하나는 바꿔주셔야 할 것 같습니다. 저는
const [comment, setComment] = useState("");
이걸
const [commentText, setCommentText] = useState("");이렇게 바꾸시는 걸 추천드립니다.
rl0425
left a comment
There was a problem hiding this comment.
고생하셨습니다.
너무 잘해주셨는데, 몇 가지만 말씀을 드리자면
-
관심사를 훅으로 분리하시는 건 너무 좋은 패턴이지만, 훅 내부에 로딩이나 에러 처리 없습니다. 아직 미완성이시겠지만, 비동기 패칭을 위한 훅이라면 내부적으로 로딩과 에러는 제일 먼저 설계하시는걸 추천드립니다.
-
훅들간의 코드 통일성이 없습니다. 어떤 훅은 useEffect 안에서의 함수 선언과 실행을 하지만, 다른 훅은 외부의 펑션을 useEffect에서 가져다 쓰고 있습니다.
전체적으로 코드를 통일성있게 작성하시면 좋은 공부가 되실 것 같습니다.
사실 컴포넌트에서 함수나 변수는 거의 동일하게 설계되고 작성되기 때문에, 본인만의 훅 관리나 패턴을 익히셔서 함수나 변수의 선언과 사용에 있어서 통일감을 주시면 더 좋은 코드가 될 것 같습니다.
| import { useRouter } from "next/router"; | ||
| import { createContext, useState, useContext, useEffect } from "react"; | ||
|
|
||
| const AuthContext = createContext(null); |
There was a problem hiding this comment.
아직 타입스크립트를 안사용하시지만, 컨택스트를 사용하실 때는 기본값을 제공하시는게 좋습니다.
const AuthContext = createContext({
user: null,
accessToken: null,
refreshToken: null,
login: () => {},
logout: () => {},
});| logout(); | ||
| router.push("/login"); | ||
| } | ||
| }, [accessToken, refreshToken]); |
There was a problem hiding this comment.
현재 if (storedAccessToken && storedRefreshToken) 을 타게되면, 엑세스 토큰이 변경이 되고, 의존성에 accessToken, refreshToken를 둘다 넣어주셨으니 useEffect가 다시 실행이 됩니다. 그럴 경우 조건문만 계속 맞는다면, 무한 루프가 돌 수 있기 때문에, 조건문이 다시 돌때는 false로 처리가 되서 문제가 없는 상황이여도 무조건 고치셔야 합니다.
전체적인 코드 흐름을 알 수 없어서 현재 추천드릴 수 있는건, 의존성 배열에 두 아이템을 빼셔야 할 것 같습니다.
| const [comments, setComments] = useState([]); | ||
|
|
||
| useEffect(() => { | ||
| fetchCommentList().then((data) => setComments(data)); |
There was a problem hiding this comment.
데이터 패칭 시 try..catch 문을 반 필수적으로 작성하셔서 예외/에러처리를 하셔야 합니다.
| fetchCommentList().then((data) => setComments(data)); | ||
| }, []); | ||
|
|
||
| return { comments, setComments }; |
There was a problem hiding this comment.
훅으로 다른곳에서 커멘트를 불러오는 함수라면, isLoading, error 데이터와 로직도 넣으셔서 패칭 중간에는 isLoading을 true로 반환하시고, 패칭이 완료될 경우 false로 변경하셔서 export 해주셔야할 것 같습니다.
| const [commentList, setCommentList] = useState([]); | ||
|
|
||
| const fetchComments = async () => { | ||
| console.log("id값넘어감???????????", articleId); |
There was a problem hiding this comment.
실무에서도 콘솔로그를 실수로 넣는 경우가 많습니다!
| export default function BoardItem({ userInfo, item = {} }) { | ||
| const { title, createdAt, likeCount, writer, image } = item; | ||
| const user = userInfo || defaultUserInfo; | ||
| const nickName = writer.nickname; |
There was a problem hiding this comment.
writer가 없을경우의 에러처리가 필요합니다.
const nickName = writer?.nickname || "익명";| export default function UserBoard() { | ||
| const router = useRouter(); | ||
| const [searchKeyword, setSearchKeyWord] = useState(""); | ||
| const [orderBy, setOrderBy] = useState(); |
There was a problem hiding this comment.
orderBy는 기본값을 설정해주시는게 좋을 것 같습니다.
const [orderBy, setOrderBy] = useState("recent");| </select> | ||
| </div> | ||
| <div className={style.userBoardByBoardItem}> | ||
| {searchKeyword === "" ? ( |
There was a problem hiding this comment.
현재 삼항연산자가 엄청 중첩되있는데, 제가 알기로는 요즘 개발 추세 자체가 삼항연산자 사용을 기피하고 있습니다. 대표적인 예시가 지금과 같은 상태일텐데, 처음 코드를 보는 사람이라면, 숙련된 개발자라도 어느 조건에 어느 부분이 렌더링되는지 한눈에 파악이 어렵습니다.
따라서, 삼항연산자를 중복해서 사용하시거나, 조건문이 너무 까다로운 경우에는 사용을 자제하시고, && 이나, 변수에 선언하시고, 변수를 불러오는 방식, 컴포넌트를 쪼개서 임포트해오는 방식 등을 고려해보시면 좋을 것 같습니다.
| <p>정말로 상품을 삭제하시겠어요?</p> | ||
| <div className={style.DeleteModalButtonBox}> | ||
| <button className={style.DeleteModalButton1} onClick={onClose}> | ||
| 취소 |
There was a problem hiding this comment.
네와 반대되는 버튼이기 때문에, '아니오'나 '아니요'가 더 적합하지 않을까요?
취소라는 문구를 사용하고 싶으시면 '네' 대신, '확인'이 나을 것 같습니다.
✅ 기본 요구사항
🔗 공통
🔐 로그인/회원가입 페이지
로그인 페이지
회원가입 페이지
로그인/회원가입 페이지 공통
📌 GNB (상단 내비게이션 바)
🛒 상품 상세 페이지
🔥 심화 요구사항
🔐 로그인 및 회원가입 페이지 공통
로그인, 회원가입 기능에
react-hook-form을 활용해 주세요.반응형 디자인 적용 (viewport 기준):
※ 375px 미만 사이즈는 고려하지 않음
🙋♂️ 유저 기능
axios interceptors를 활용해 주세요. (axios를 사용하지 않는다면 유사한 기능 활용)🔁 React Query 마이그레이션
React Query로 마이그레이션🔄 로딩 및 에러 핸들링
🧠 상품 데이터 캐싱 및 업데이트
멘토님께