[원치준] sprint8#128
Hidden character warning
Conversation
Irelander
left a comment
There was a problem hiding this comment.
치준님, 전체적으로 코드를 살펴봤는데요. Next.js 고급 패턴들을 완벽하게 활용하고 있어서 이미 실무 수준이에요. 다만 변수명, 오타, 텍스트 같은 디테일 부분만 조금 더 꼼꼼하게 체크하면 완벽할 것 같아요. 코드 작성 후 한 번 더 검토하는 습관만 들이면 바로 실무 투입 가능한 수준이에요. 정말 잘하고 있어요!
아래의 부분들만 조금 더 신경쓰시면 좋을꺼 같아요 !
개선하면 좋을 부분들
-
네이밍 체크 - 치준님,,, 생각보다 오타가 많으신거 같더라구요 ?! ㅎㅎㅎ IDE툴들 보면 오타체커 같은게 있어서 실시간으로 오타로 의심되는 부분들 표시되도록 켜둘수도 있어요! 이런거 활용하면 오탈자를 줄일수 있을꺼에요!
-
검색/정렬 처리 위치 - 지금은 클라이언트에서 필터링하는데, 데이터가 많아지면 성능 이슈가 있을 수 있어요. 백엔드에서 처리하도록 API 파라미터로 넘기는 게 좋아요.
-
페이지네이션 - 현재 모든 데이터를 한 번에 가져오는데, 페이지네이션 추가하면 더 좋을 것 같아요.
-
라우팅 - window.location.href 대신 Next.js router 사용하면 전체 새로고침 없이 부드럽게 이동할 수 있어요.
이번 스프린트도 고생 많으셨습니다. :)
| useEffect(() => { | ||
| function onDocClick(e) { | ||
| if (!open) return; | ||
| const t = e.target; | ||
| if ( | ||
| menuRef.current && | ||
| !menuRef.current.contains(t) && | ||
| btnRef.current && | ||
| !btnRef.current.contains(t) | ||
| ) { | ||
| setOpen(false); | ||
| } | ||
| } | ||
| function onKey(e) { | ||
| if (e.key === "Escape") setOpen(false); | ||
| } | ||
| document.addEventListener("mousedown", onDocClick); | ||
| document.addEventListener("keydown", onKey); | ||
| return () => { | ||
| document.removeEventListener("mousedown", onDocClick); | ||
| document.removeEventListener("keydown", onKey); | ||
| }; | ||
| }, [open]); |
There was a problem hiding this comment.
btnRef와 menuRef를 둘 다 체크해서 버튼 클릭은 토글하고, 바깥 클릭만 닫히게 한 게 UX를 고려한 훌륭한 코드예요.
ESC 키 처리 + cleanup 함수도 최고입니다 👏 👏 👏 👏
| method: "DELETE", | ||
| }); | ||
| if (!res.ok) throw new Error("삭제 실패"); | ||
| window.location.href = "/board"; |
There was a problem hiding this comment.
window.location.href는 전체 페이지를 새로고침해요.
Next.js에서는 router.push()나 router.replace()를 쓰는 게 더 좋아요.
| import React, { useEffect, useMemo, useState } from "react"; | ||
|
|
||
| const MarketPage = () => { | ||
| const [articles, setarticles] = useState([]); |
There was a problem hiding this comment.
setarticles (소문자 a)만 JavaScript 네이밍 컨벤션을 따르지 않고 있네요!
| const [search, setSearch] = useState(""); | ||
| const [sortKey, setSortKey] = useState("latest"); | ||
|
|
||
| const BASE_URL = process.env.NEXT_PUBLIC_API_BASE; |
There was a problem hiding this comment.
BASE_URL이 undefined일 수 있어요.
사전에 검사해서 오류를 미리 예측하면 좋을꺼 같아요 ! ㅎㅎ
| const visibleList = useMemo(() => { | ||
| const q = search.trim().toLowerCase(); | ||
| const filtered = q | ||
| ? articles.filter((p) => { | ||
| const title = (p?.name ?? p?.title ?? "").toLowerCase(); | ||
| return title.includes(q); | ||
| }) | ||
| : articles.slice(); | ||
|
|
||
| filtered.sort((a, b) => { | ||
| const ad = new Date(getCreatedAt(a) ?? 0).getTime(); | ||
| const bd = new Date(getCreatedAt(b) ?? 0).getTime(); | ||
| if (sortKey === "latest") return bd - ad; | ||
| return ad - bd; | ||
| }); |
There was a problem hiding this comment.
useMemo 사용은 좋지만, 지금은 괜찮지만 실제 운영환경에서는 검색과 정렬을 프론트에서 하면 안되는점 참고하세요!
만약 프론트에서 처리한다면 다음과 같은 문제점이 생기기 때문이에요.
문제점:
- 모든 데이터를 한 번에 가져와야 함 (메모리 낭비)
- 데이터가 1000개면? 10000개면? (성능 문제)
- 백엔드 DB 인덱스를 활용 못 함
기본 요구사항
공통
자유 게시판 페이지
게시글 등록 & 수정 페이지
게시글 상세 페이지
심화 요구사항
공통
스크린샷