스프린트 미션 9제출 - Next 박창기 #80
Hidden character warning
Conversation
…, RouteGuard에서 보호 경로 및 공개 경로 수정
|
리뷰어를 저로 설정 해 주세요~ |
devbini
left a comment
devbini
left a comment
There was a problem hiding this comment.
👍 수고하셨습니다!
코드 설계에 더 신경 쓴 게 느껴져서 인상 깊었어요. 특히 fetchClient처럼 캡슐화한 점도 좋았는데, 그 캡슐 내에서도 역할에 따라 분리하고, 마치 인터셉터마냥 구성한 설계는 사실 실무에서 써도 큰 지장이 없을 정도로 좋아보입니다.
다만, axios 패싱ㅠㅠ과 React Query를 사용하지 않은 점은 조금 아쉽네요.
설치만 해 두시고 사용을 안 한 것 같은데, 직접 꼭 구현 해 보시면 좋을 것 같습니다 :D
기능 구현도 좋지만, 결국 프론트는 사용자와 닿아있으니,, 사용자가 어떤 짓을 해도 웹이 터지지 않으려면 어떻게 해야 할까~를 고민해 보시면 더할 나위 없을 것 같아요.
너무너무 고생하셨습니다~~!
| const path = pathname.split("?")[0]; | ||
|
|
||
| //정확한 경로 매칭 or 하위 경로 매칭 | ||
| const isProtectedRoute = protectedPaths.some( |
There was a problem hiding this comment.
🚩 Red Flag
이거 동작하나요?
const protectedPaths = [
"/articles/write",
"/items/[id]",
"/articles/[id]",
"/items/write",
];
선언할 때, [id] 이 문자열은 빼 줘야 동작할 것 같습니다.
현재 상태로는, 정말 말 그대로 경로가 /items/[id]/인 경로만 보호될 것으로 보입니다.
만약 아니라면 알려주세요, 우선 지금 제 생각에선 그러네요,..?
| </ul> | ||
| )} | ||
|
|
||
| <Pagination page={page} totalPages={totalPages} onChange={setPage} /> |
There was a problem hiding this comment.
🚩 Red Flag
알고 계시겠지만.. totalPages가 5로 고정이 되어있어서, 상품이 엄청 늘어나도 무조건 5페이지 까지만 볼 수 있는 것 같아요.
API 연동 과정에서 데이터의 양을 함께 반환받도록 수정하면, 조금 더 유연하게 페이지를 구현할 수 있지 않을까요? 😁
| <section className=" mt-6 mb-10"> | ||
| <div className="flex gap-6"> | ||
| <Image | ||
| src={images?.[0]} |
There was a problem hiding this comment.
🚩 Red Flag
만약 images가 undefined거나 null이라면 페이지 전체가 깨져버릴 것 같아요 지금 상태는.
가능하면 조건부 렌더링을 한다거나, 기본 이미지를 추가하는 등 렌더링에 있어서 예외처리가 반드시 필요할 듯 합니다!
| // src/lib/fetchClient.js | ||
|
|
||
| /** | ||
| * 기본 fetch 클라이언트 - 인증이 필요 없는 일반 요청용 |
There was a problem hiding this comment.
👍 Good Point
개인적으로는 사실 axios를 좋아합니다. 😂
그럼에도, 이 파일 내부 자체가 너무 일관성있게 잘 짜여져 있어서 언급 해 봐요.
단일 책임 원칙에 맞춰서 각 함수 구분을 너무 잘 했고, 주석은 물론, 특히 토큰쪽 유지보수를 생각한 설계도 좋습니다.
| isInitialized: false, | ||
| }); | ||
|
|
||
| export const useAuth = () => useContext(AuthContext); |
There was a problem hiding this comment.
👍 Good Point
이렇게 쪼개서, 유지보수하기 좋게 코드를 만드는 것도 큰 능력이라 생각해요.
그 부분에 있어서 이런 캡슐화 전략은 적극 지지합니다. 너무 나노하게 되면 안되겠지만.. 지금처럼 함수나 주어진 역할, 전역 관리를 위해 훅으로 사용하는 건 React가 좋아할 법한 개발 방법론이라 생각해요.
| const [inputValue, setInputValue] = useState(""); | ||
| const [select, setSelect] = useState("최신순"); | ||
|
|
||
| const handleSearch = (e) => { |
There was a problem hiding this comment.
☕ Thinking...
예전에 말씀 드렸던 디바운싱 기억 나시나요?
서버 개발자가 갑자기 문 부수고 찾아와서 뭐라 할 수도 있을 것 같아요. 너무 무서운 상황...
입력값 검증에 있어서는 디바운싱 기법을 꼭 제대로 적용 해 봤으면 좋겠습니다!
| return; | ||
| } | ||
|
|
||
| setComments((prev) => [...prev, { text: cleanContent, date: new Date() }]); |
There was a problem hiding this comment.
☕ Thinking..
의도하신 것 같긴 해요. 요구사항도 아니었을 것 같긴 한데
완성도를 높이는 측면에서 봤을 때- 새로고침하면 댓글이 사라지는 상황이라.. DB에 댓글을 관리하는 부분만 추가해서 연동 해 보면 어떨까 하는 욕심이 있습니다.
| <input | ||
| type={showEye ? "text" : "password"} | ||
| value={password} | ||
| onChange={handlePassword} |
There was a problem hiding this comment.
🚩 Red Flag
아래 handlePasswordConfirm과 동기화 오류가 생길 것 같아요.
코드를 쭉 보니까, handlePassword 함수는 오로지 비밀번호가 정상적인 녀석인지 확인하고,
handlePasswordConfirm 함수는 입력한 비밀번호가 확인용 비밀번호와 맞는지 확인하는 함수 같더라구요.
그런데 만약 아래와 같은 시나리오라면 어떻게 될까요?
1. 비밀번호[1] / 확인용 비밀번호[1] -> 통과
2. 이후 비밀번호[1]을 비밀번호[2]로 변경
3. handlePassword는 동작해서 정상으로 통과됨
4. 하지만 handlePasswordConfirm은 동작하지 않음
UX상으로 둘이 값이 다른데 같은 비밀번호가 입력 되어있다는 화면을 보게될 것 같답니다.
| price, | ||
| }) { | ||
| console.log("상품 ID:", id, "이미지 데이터:", images); | ||
| const [currentImg, setCurrentImg] = useState(images?.[0] || defaultImg); |
There was a problem hiding this comment.
☕ Thinking...
이 경우... 페이지네이션을 통해 상품 목록이 싹 바뀌었을 때 컴포넌트를 새로 불러오는 게 아니라면, 이미지가 변하지 않을 수 있어요.
이런 경우엔 조금 번거롭더라도, useEffect()로 images의 변경여부를 검사하고 set 해 주는 로직을 추가 해 두는 것도 안전하지 않을까 생각이 드네요.
2 participants
스프린트 미션 9 시안 확인
아래 링크를 통해 강의 학습 후 만들어봐야 할 실습 과제 디자인을 확인해 보세요
기본 요구사항
공통
로그인/회원가입 페이지
로그인 페이지
회원가입 페이지
로그인, 회원가입 페이지 공통
GNB
상품 상세 페이지
심화 요구사항
로그인 및 회원가입 페이지 공통
유저 기능
React-Query로 마이그레이션
fetch 혹은 axios로 구현된 기존의 API 요청 코드를 React-Qeury로 마이그레이션 합니다.
로딩 및 에러 핸들링
로딩 인디케이터와 에러 메시지를 구현합니다.
상품 목록 및 상품 상세 데이터를 Prefetching 합니다
상품 데이터 캐싱 및 업데이트