Skip to content

Conversation

@minji9029
Copy link
Collaborator

요구사항

기본

기본

  • ‘로고’ 버튼을 클릭하면 ‘/’ 페이지로 이동합니다. (새로고침)
  • 진행 중인 할 일과 완료된 할 일을 나누어 볼 수 있습니다.
  • 상단 입력창에 할 일 텍스트를 입력하고 추가하기 버튼을 클릭하거나 엔터를 치면 할 일을 새로 생성합니다.
  • 진행 중 할 일 항목의 왼쪽 버튼을 클릭하면 체크 표시가 되면서 완료 상태가 됩니다.
  • 완료된 할 일 항목의 왼쪽 버튼을 다시 클릭하면 체크 표시가 사라지면서 진행 중 상태가 됩니다.

주요 변경사항

  • React-Query 5v와 Zustand 동기화하는 방식을 사용해보았습니다.
  • Shadcn을 사용해서 체크박스를 구현하였습니다
  • tailwind 4v 사용하여 구현하였습니다.
  • zod 사용해서 인풋 상태를 관리하였습니다.
  • React-Querydml 의 onMutate사용해서 변경 시 낙관적 업데이트 적용해보았습니다.

스크린샷

image

멘토에게

  • 서버상태와 클라이언트 상태를 분리해보려했는데 잘 된건지 모르겠습니다
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@minji9029 minji9029 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Aug 14, 2025
Copy link
Collaborator

@addiescode-sj addiescode-sj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다!
민지님도 미션과 프로젝트를 거듭하면서 많이 성장한 모습이 보이네요 ㅎㅎ
이대로 열심히 공부해보시고, 제가 드린 피드백도 참고하면서 리팩토링 해보시면 도움 될것 같아요!

주요 리뷰 포인트

  • 관심사 분리 원칙에 따라 내부 로직 이동하기
  • Button 컴포넌트 variant 기반으로 바꿔보기
  • 중복 코드 제거 (컴포넌트 분리 / layout 파일 활용)
  • 입력값 디바운싱 + 유효성 검사 useTransition 적용
  • todo 상태 관리 방식 리팩토링 제안

Comment on lines +14 to +15
const activeTodos = todos.filter((todo) => !todo.isCompleted);
const doneTodos = todos.filter((todo) => todo.isCompleted);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useGetItems hook을 쓰고 계시니, 이왕이면 data fetching과 관련된 내부 로직을 모두 해당 커스텀 훅 안쪽으로 옮기면 좀 더 관심사 분리가 제대로 될 것 같네요 :)

Comment on lines +24 to +28
<div className='flex-1'>
<TodoList todos={activeTodos} />
</div>
<div className='flex-1'>
<DoneList todos={doneTodos} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TodoList, DoneList 컴포넌트가 배열 아이템 갯수가 0일때 렌더링되는 경우를 고려해볼까요? :)

Comment on lines +5 to +11
children: ReactNode;
type?: 'delete' | 'edit' | 'add';
htmlType?: 'button' | 'submit' | 'reset';
disabled?: boolean;
size?: 'sm' | 'lg';
onClick?: MouseEventHandler<HTMLButtonElement>;
className?: string; // 외부에서 className을 추가할 수 있도록 추가
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Button의 역할에 비해 props가 복잡해보이고, 많은 것처럼 느껴지네요.
variant 기반으로 제한된 스타일 규칙만 사용할수있게 개선해보는건 어떨까요?
shadcn이 이 방식으로 만들어져있는데, 아래 정리된 아티클 읽어보시고 컨셉 참고해보셔도 좋을것같아요 :)

참고

Comment on lines +18 to +24
<h1
className='border-none rounded-[24px]
bg-green-700 text-amber-300 font-bold text-5
w-fit px-7 py-2'
>
DONE
</h1>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DoneList, TodoList간에 중복 코드가 있는것같아요.
이런 경우 컴포넌트로 분리해 공용화하거나 페이지 단위 파일에서 컴포넌트 폴더를 만들어 최대한 중복을 줄여보는식으로 구조를 바꾸는건 어떨까요? :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

만약 동적인 기능이나 상태 업데이트가 필요하지않은 상황에서,
페이지 레벨 컴포넌트끼리 공유하는 UI가 있다면 layout 파일을 활용해보셔도 효과적이랍니다 :)

공식 문서 링크: https://nextjs.org/docs/app/getting-started/layouts-and-pages#creating-a-layout

Comment on lines +35 to +39
// 입력값이 변경될 때마다 유효성 검사
useEffect(() => {
const result = todoSchema.safeParse({ content: inputValue });
setIsValid(result.success);
}, [inputValue]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

입력값 디바운싱을 적용해보는건 어떨까요?
구조를 보니 useEffect를 사용해 입력값이 변경될때마다 유효성 검사를 하고,
다시 상태 업데이트를 하고있어 불필요한 리렌더링이 자주 발생하고있어요.

아래 예시 코드처럼 디바운싱을 적용하며, useTransition으로 유효성 검사를 비동기 처리해주는 방식으로 개선해보면 UI blocking이 발생하지않고 좀 더 매끄러운 사용자 경험을 제공할수있을것같아요.

  // states
  const [inputValue, setInputValue] = useState('');
  const [debouncedValue, setDebouncedValue] = useState('');
  const [isPending, startTransition] = useTransition();
  const [validationResult, setValidationResult] = useState(false);

  // 디바운싱 적용 (300ms 후에만 유효성 검사)
  useEffect(() => {
    const timer = setTimeout(() => {
      setDebouncedValue(inputValue);
    }, 300);

    return () => clearTimeout(timer);
  }, [inputValue]);

  // useTransition을 사용한 비동기 유효성 검사
  useEffect(() => {
    if (debouncedValue === inputValue && inputValue.trim() !== '') {
      startTransition(() => {
        const result = todoSchema.safeParse({ content: inputValue }).success;
        setValidationResult(result);
      });
    } else if (inputValue.trim() === '') {
      setValidationResult(false);
    }
  }, [debouncedValue, inputValue, startTransition]);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Todo 상태를 store를 만들어 React Query와 연동할 필요성이 있을까요?
해당 store가 하는 역할이 단순히 서버 상태에 대한 전역적인 접근을 클라이언트 상태로 전환해 수월하게 하기 위해서라면,
오히려 성능을 위해서는 필요한 시점에 필요한 컴포넌트에서 직접적으로 React Query를 사용하는 방법을 추천드립니다.

이유는, 우선 지금과 같이 상태 관리가 중복되면 휴먼 에러에 의해 두 상태가 동기화되지않을 가능성이 높아져, 불필요한 관리포인트가 생기기 마련이기 때문이예요 :)

또 실제 코드를 보면 zustand를 통해 관리하는 클라이언트 상태는 사용되지않아서, 지금과 같이 미리 개발 스펙에 포함할 필요도 없습니다.

따라서, 상태 흐름을 명확하게 만들기위해 불필요한 상태 동기화 로직을 제거하는 방향을 추천드립니다 :)
단일 진실 공급원(Single Source of Truth) 원칙이라는게 있는데, 해당 부분 리팩토링할때 한번 공부해보시면 좋을것같네요! :)

@addiescode-sj addiescode-sj merged commit a76d23d into codeit-bootcamp-frontend:Next-배민지 Sep 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants