Skip to content

Conversation

@summerDev96
Copy link
Collaborator

@summerDev96 summerDev96 commented Aug 10, 2025

요구사항

기본

목록 조회

  • ‘로고’ 버튼을 클릭하면 ‘/’ 페이지로 이동합니다. (새로고침)
  • 진행 중인 할 일과 완료된 할 일을 나누어 볼 수 있습니다.

할 일 추가

  • 상단 입력창에 할 일 텍스트를 입력하고 추가하기 버튼을 클릭하거나 엔터를 치면 할 일을 새로 생성합니다.

할 일 완료

  • 진행 중 할 일 항목의 왼쪽 버튼을 클릭하면 체크 표시가 되면서 완료 상태가 됩니다.
  • 완료된 할 일 항목의 왼쪽 버튼을 다시 클릭하면 체크 표시가 사라지면서 진행 중 상태가 됩니다.

주요 변경사항

  • tanstack-query로 api 호출 및 투두 리스트 추가/변경 시 낙관적 업데이트 적용하였습니다.
  • tailwind css로 스타일 관리하였습니다.
  • react-hook-form + zod로 폼 상태를 관리하였습니다.
  • 초기 데이터 패칭 상태에서 로딩 적용

배포 사이트 링크

https://summerdev-sprint-mission.vercel.app/

스크린샷

screencapture-16-sprint-mission-vercel-app-2025-08-10-21_52_00

멘토에게

  • app 라우터를 처음 써보는데, 전체적인 구조에 문제가 없을까요?
  • HydrationBoundary를 페이지 내에 추가하였는데, layout.tsx에 공통으로 추가하는 것이 좋을까요?
  • 낙관적 업데이트 적용 시 뭔가 버벅이는 부분이 있는데, 어떻게 해결할 수 있을까요?
  • 셀프 코드 리뷰로 해당 부분에 질문 추가하겠습니다

@summerDev96 summerDev96 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Aug 10, 2025
src/app/page.tsx Outdated
Comment on lines 12 to 16
<HydrationBoundary state={dehydrate(queryClient)}>
<div className='flex justify-center pt-6 bg-gray-50 min-h-[calc(100vh-3.75rem)]'>
<TodoContent />
</div>
</HydrationBoundary>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

HydrationBoundary를 페이지 내에 추가하였는데, layout.tsx에 공통으로 추가하는 것이 좋을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

프로젝트 요구사항에 맞춰 선택하시면 좋을것같아요.
각 페이지마다 필요한 데이터만 prefetch를 적용하고싶다면, 지금과 같은 방식을 (페이지 단위로 적용) 유지하시는게 좋습니다.

그런데 이 점때문에 관리 포인트가 추가되거나 코드 중복이 늘어날것같다면 wrapper 용도의 공용 컴포넌트를 만들고 각 컴포넌트에서 필요할 경우에만 선언적인 방식으로 사용되도록 개선해주시는것도 좋겠네요 :)

예시)

...
  return (
    <HydrationWrapper prefetchQueries={[{ queryKey: ['todos'], queryFn: getItemList }]}>
      <div className='flex justify-center pt-6 bg-gray-50 min-h-[calc(100vh-3.75rem)]'>
        <TodoContent />
      </div>
    </HydrationWrapper>
  );

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵! 공통 컴포넌트로 만들어서 사용하겠습니다.

Comment on lines 66 to 89
onMutate: async (updatedTodo: Item) => {
await queryClient.cancelQueries({ queryKey: TODO_QUERY_KEY, exact: true });
const previousTodos = queryClient.getQueryData<Item[]>(TODO_QUERY_KEY);

queryClient.setQueryData<Item[]>(TODO_QUERY_KEY, (todoList) => {
if (!todoList) return [];
return todoList.map((todo) => (todo.id === updatedTodo.id ? updatedTodo : todo));
});

return { previousTodos };
},
onSuccess: (response: ItemDetail) => {
const responseItem = {
id: response.id,
name: response.name,
isCompleted: response.isCompleted,
};

queryClient.setQueryData<Item[]>(TODO_QUERY_KEY, (todoList) =>
todoList
? todoList.map((todo) => (todo.id === response.id ? responseItem : todo))
: todoList,
);
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

낙관적 업데이트 적용 시 뭔가 버벅이는 부분이 있는데, 어떻게 해결할 수 있을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

우선 화면에서 버벅이거나 깜빡이는 현상이 보인다면, 해결을 위해 내부적으로 어떤 과정때문에 이런 현상이 발생하는지 원인을 파악하는것부터 시작하시는게 좋습니다.

제가 봤을땐 업데이트 과정에서 TODO_QUERY_KEY 를 가진 모든 쿼리 요청을 취소하고있는데, 이 과정으로 인해 추가적인 상태 동기화가 일어나기때문에 쿼리 취소와 관련된 동작을 제거하고, 쿼리와 관련한 옵션을 몇개 수정해보시면 될것같아요.

지금
staleTime: 0,
refetchOnMount: 'always'
이렇게 두개 옵션을 쓰고 계신데 이 옵션값 적용으로 매번 새로운 데이터 요청이 일어나면서, 불필요한 리페치가 일어나고있습니다.

해당 옵션이 꼭 필요하지않다면
캐싱을 고려해 staleTime을 조금 늘려서 불필요한 요청을 감소시키거나,
refetchOnMount: false를 적용해 마운트 시 자동 리페치 과정을 비활성화해주시면 훨씬 화면이 매끄러워질것같네요 :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵넵! 불필요한 로직 수정하고, 옵션들 바꿔보겠습니다

Comment on lines 8 to 38
export default function RootLayout({ children }: { children: React.ReactNode }) {
return (
<html lang='ko'>
<body>
{/* Google tag (gtag.js)*/}
<Script
async
src={`https://www.googletagmanager.com/gtag/js?id=${gtag.GA_TRACKING_ID}`}
></Script>
<Script
id='gtag'
strategy='afterInteractive'
dangerouslySetInnerHTML={{
__html: `
window.dataLayer = window.dataLayer || [];
function gtag(){dataLayer.push(arguments);}
gtag('js', new Date());
gtag('config', '${gtag.GA_TRACKING_ID}', {
page_path: window.location.pathname,
});
`,
}}
/>

<QueryProvider>
<Gnb />
{children}
</QueryProvider>
</body>
</html>
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

app 라우터를 처음 써보는데, 전체적인 구조에 문제가 없을까요?

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.

수고하셨습니다!

이제 심화 프로젝트도 준비하셔야하니,
생각보다 훨씬 디테일한 부분들까지 코드 퀄리티를 올리기위해 신경써보면 좋을것같아요.

관련해서 본문 코멘트에 꼼꼼히 작성해드렸습니다 :)

주요 리뷰 포인트

  • 서버사이드를 고려한 API 인터페이스 및 apiClient 설계 수정
  • 폰트 최적화
  • HydrationBoundary 적용 관련 피드백 및 개선 사항
  • 함수 기반 접근 방식 줄이기 (불필요할 경우)
  • 화면 깜빡임 이슈 해결

Copy link
Collaborator

Choose a reason for hiding this comment

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

사용하지 않는 파일이라면 PR 올리실때는 정리해보세요 :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

혹시, 외부 api를 직접 사용하지않고 이렇게 따로 API 인터페이스를 만들어 사용하는 이유가 있을까요?

만약 추후에 서버 사이드 환경을 고려하고싶은게 그 이유라면,
몇가지 부수적인 작업을 더 해주셔야할 것 같아요.

우선 route handler를 사용해 API 프록시를 구성해줘야할것같네요!
모든 api 경로에 적용할거니까, api 폴더 아래에 [...path] 폴더를 만들고 그 아래 route.ts 파일을 만들어 모든 동적 경로에 적용될수있도록 세팅해주시고,

요청마다 필요한 작업들을 아래와 같이 (참고로만 보세요) 처리해주세요.

  • app / api / [...path] / route.ts 파일 예시
import { NextRequest, NextResponse } from 'next/server';

export async function GET(request: NextRequest, { params }: { params: { path: string[] } }) {
  return handleApiRequest(request, params.path, 'GET');
}

export async function POST(request: NextRequest, { params }: { params: { path: string[] } }) {
  return handleApiRequest(request, params.path, 'POST');
}

export async function PATCH(request: NextRequest, { params }: { params: { path: string[] } }) {
  return handleApiRequest(request, params.path, 'PATCH');
}

export async function DELETE(request: NextRequest, { params }: { params: { path: string[] } }) {
  return handleApiRequest(request, params.path, 'DELETE');
}

async function handleApiRequest(request: NextRequest, pathSegments: string[], method: string) {
  try {
    const baseURL = process.env.BASE_URL || process.env.NEXT_PUBLIC_BASE_URL;
    const tenantId = process.env.TENANT_ID || process.env.NEXT_PUBLIC_TENANT_ID;

    // 경로 세그먼트를 조합하여 외부 API URL 생성
    const apiPath = pathSegments.join('/');
    const targetURL = `${baseURL}/${tenantId}/${apiPath}`;

    // 요청 헤더 준비
    const headers: Record<string, string> = {
      'Content-Type': 'application/json',
    };

    // Authorization 헤더가 있다면 전달
    const authHeader = request.headers.get('authorization');
    if (authHeader) {
      headers['Authorization'] = authHeader;
    }

    // 요청 본문 준비
    let body: string | undefined;
    if (method !== 'GET') {
      body = await request.text();
    }

    // 외부 API로 요청 전송
    const response = await fetch(targetURL, {
      method,
      headers,
      body,
    });

    // 응답 데이터 파싱
    const data = await response.json();

    // 응답 반환
    return NextResponse.json(data, {
      status: response.status,
      headers: {
        'Content-Type': 'application/json',
      },
    });
  } catch (error) {
    console.error('API proxy error:', error);
    return NextResponse.json({ error: 'Internal server error' }, { status: 500 });
  }
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

그 다음, apiClient 를 구성할때도 서버사이드 요청인지 아닌지를 구분해 관련 작업을 처리해주는 과정이 필요해요.
axios는 기본적으로 브라우저 환경을 위한 HTTP 클라이언트라서, 서버 컴포넌트에서는 Node.js의 fetch API나 서버 전용 HTTP 클라이언트를 사용하는 것이 더 적합한 방법입니다.

서버 사이드에서는 지금 파일처럼 내부적으로 만든 API route를 사용하고,
클라이언트 사이드에서는 외부 API를 직접 사용해주셔야겠죠?

const createApiClient = () => {
  // 서버사이드인지 확인
  const isServer = typeof window === 'undefined';

  if (isServer) {
    // 서버사이드에서는 내부 API route를 사용
    return axios.create({
      baseURL: '/api',
      timeout: 10_000,
      headers: { 'Content-Type': 'application/json' },
    });
  } else {
    // 클라이언트사이드에서는 외부 API를 직접 사용
    return axios.create({
      baseURL: process.env.NEXT_PUBLIC_BASE_URL,
      timeout: 10_000,
      headers: { 'Content-Type': 'application/json' },
    });
  }
};

const apiClient = createApiClient();

Copy link
Collaborator

Choose a reason for hiding this comment

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

그러면 이 파일에서 쓰는 함수들을 서버사이드/ 클라이언트 사이드 상관없이 모두 활용할 수 있을거예요!

만약 지금과 같이 따로 내부적인 API route를 사용한 이유가 추후 서버사이드까지 고려한 선택이 아니라면,
과감히 배제해주셔도 좋을 것 같습니다 :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵! 서버사이드 고려하여 다시 수정하겠습니다.

Comment on lines 3 to 9
@font-face {
font-family: 'HS-Regular';
src: url('https://gcore.jsdelivr.net/gh/projectnoonnu/noonfonts_2201-2@1.0/HS-Regular.woff')
format('woff');
font-weight: normal;
font-style: normal;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

woff2 형식이 있다면 앞에 추가하고 없으면 woff를 쓰게끔 해서 최적화를 더 신경쓰면 좋을것같아요.

만약 로컬 폰트로 다운받을수있다면 서브셋 추출 및 적용도 가능해서 최적화에 더 유리하고,
컨텐츠의 특성에 따라 font-display: swap; 과 같이 폰트를 어떻게 보여줄지 결정해줄수도 있는데, swap의 경우 폰트가 로드되는 동안 시스템 폰트로 텍스트를 먼저 표시하고, 폰트가 로드되면 교체하는 방식이라서 텍스트가 중요한 경우 사용해주면 좋습니다.

다양한 최적화 사례가 있으니, 여러 아티클 참고해보시고 적용해보세요 :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵! woff2 형식은 없어서 font-display: swap만 적용하였습니다.

src/app/page.tsx Outdated
Comment on lines 12 to 16
<HydrationBoundary state={dehydrate(queryClient)}>
<div className='flex justify-center pt-6 bg-gray-50 min-h-[calc(100vh-3.75rem)]'>
<TodoContent />
</div>
</HydrationBoundary>
Copy link
Collaborator

Choose a reason for hiding this comment

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

프로젝트 요구사항에 맞춰 선택하시면 좋을것같아요.
각 페이지마다 필요한 데이터만 prefetch를 적용하고싶다면, 지금과 같은 방식을 (페이지 단위로 적용) 유지하시는게 좋습니다.

그런데 이 점때문에 관리 포인트가 추가되거나 코드 중복이 늘어날것같다면 wrapper 용도의 공용 컴포넌트를 만들고 각 컴포넌트에서 필요할 경우에만 선언적인 방식으로 사용되도록 개선해주시는것도 좋겠네요 :)

예시)

...
  return (
    <HydrationWrapper prefetchQueries={[{ queryKey: ['todos'], queryFn: getItemList }]}>
      <div className='flex justify-center pt-6 bg-gray-50 min-h-[calc(100vh-3.75rem)]'>
        <TodoContent />
      </div>
    </HydrationWrapper>
  );

Comment on lines 25 to 45
const baseClass =
'flex items-center text-base font-bold rounded-[1.68rem] border-2 border-slate-900 shadow-offset';

const getSizeClass = (): string => {
const sizeClass: Record<string, string> = {
small: 'p-4',
large: 'min-w-41 gap-1 py-3.5 px-10',
};

return sizeClass[size];
};

const getModeClass = (): string => {
const modeClass: Record<string, string> = {
add: `${disabled ? 'bg-slate-200 text-slate-900' : 'bg-violet-600 text-white'} border-slate-900`,
delete: 'bg-rose-500 text-white border-slate-900',
edit: `${disabled ? 'bg-slate-200' : 'bg-lime-300'} text-slate-900 border-slate-900`,
};

return modeClass[mode];
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

함수 기반으로 접근하는것보다는 코드를 좀 더 간소화할수있는 이런 방식은 어떨까요?

  const baseClasses =
    'flex items-center text-base font-bold rounded-[1.68rem] border-2 border-slate-900 shadow-offset';

  const sizeClasses = {
    'p-4': size === 'small',
    'min-w-41 gap-1 py-3.5 px-10': size === 'large',
  };

  const modeClasses = {
    'bg-slate-200 text-slate-900': (mode === 'add' || mode === 'edit') && disabled,
    'bg-violet-600 text-white': mode === 'add' && !disabled,
    'bg-rose-500 text-white': mode === 'delete',
    'bg-lime-300 text-slate-900': mode === 'edit' && !disabled,
  };

<button
{...rest}
disabled={disabled}
className={clsx(baseClass, getSizeClass(), getModeClass(), className)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
className={clsx(baseClass, getSizeClass(), getModeClass(), className)}
className={clsx(baseClasses, sizeClasses, modeClasses, className)}

Comment on lines 13 to 19
const getMessage = () => {
const message = {
todo: '할 일이 없어요.\nTODO를 새롭게 추가해주세요',
done: '아직 다 한 일이 없어요.\n해야 할 일을 체크해보세요!',
};
return message[mode];
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

함수 기반으로 많이 값에 접근하시네요.
이 경우 mapper를 사용해주는게 여러 이유로 좋습니다.

우선, 해당 함수가 단순히 객체에서 값 반환만 담당하고있기때문에, 성능적으로 불필요한 오버헤드를 제거하는게 좋고 (매번 함수 호출, 함수 내부에서 매번 새 객체 생성) 상수 객체를 컴포넌트 외부로 빼두면 메모리 효율도 고려할 수 있습니다.
가독성 측면에서도 단순 값 접근을 위한 mapper 객체라는 의도가 명확히 드러나기때문에 코드가 더 짧아지면서 이해하기도 쉬워지겠죠?

아래와 같이 컴포넌트 외부에 const obj를 만들어서 TypeScript를 사용해 컴파일 타임에 잘못된 키 접근을 막을 수 있게끔 해주시면 될것같네요 :)

const MESSAGES = {
  todo: '할 일이 없어요.\nTODO를 새롭게 추가해주세요',
  done: '아직 다 한 일이 없어요.\n해야 할 일을 체크해보세요!',
} as const;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

함수로 클래스 반환하는 부분 mapper로 변경하였습니다!

Comment on lines 66 to 89
onMutate: async (updatedTodo: Item) => {
await queryClient.cancelQueries({ queryKey: TODO_QUERY_KEY, exact: true });
const previousTodos = queryClient.getQueryData<Item[]>(TODO_QUERY_KEY);

queryClient.setQueryData<Item[]>(TODO_QUERY_KEY, (todoList) => {
if (!todoList) return [];
return todoList.map((todo) => (todo.id === updatedTodo.id ? updatedTodo : todo));
});

return { previousTodos };
},
onSuccess: (response: ItemDetail) => {
const responseItem = {
id: response.id,
name: response.name,
isCompleted: response.isCompleted,
};

queryClient.setQueryData<Item[]>(TODO_QUERY_KEY, (todoList) =>
todoList
? todoList.map((todo) => (todo.id === response.id ? responseItem : todo))
: todoList,
);
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

우선 화면에서 버벅이거나 깜빡이는 현상이 보인다면, 해결을 위해 내부적으로 어떤 과정때문에 이런 현상이 발생하는지 원인을 파악하는것부터 시작하시는게 좋습니다.

제가 봤을땐 업데이트 과정에서 TODO_QUERY_KEY 를 가진 모든 쿼리 요청을 취소하고있는데, 이 과정으로 인해 추가적인 상태 동기화가 일어나기때문에 쿼리 취소와 관련된 동작을 제거하고, 쿼리와 관련한 옵션을 몇개 수정해보시면 될것같아요.

지금
staleTime: 0,
refetchOnMount: 'always'
이렇게 두개 옵션을 쓰고 계신데 이 옵션값 적용으로 매번 새로운 데이터 요청이 일어나면서, 불필요한 리페치가 일어나고있습니다.

해당 옵션이 꼭 필요하지않다면
캐싱을 고려해 staleTime을 조금 늘려서 불필요한 요청을 감소시키거나,
refetchOnMount: false를 적용해 마운트 시 자동 리페치 과정을 비활성화해주시면 훨씬 화면이 매끄러워질것같네요 :)

@addiescode-sj
Copy link
Collaborator

질문에 대한 답변

멘토에게

  • app 라우터를 처음 써보는데, 전체적인 구조에 문제가 없을까요?
  • HydrationBoundary를 페이지 내에 추가하였는데, layout.tsx에 공통으로 추가하는 것이 좋을까요?
  • 낙관적 업데이트 적용 시 뭔가 버벅이는 부분이 있는데, 어떻게 해결할 수 있을까요?
  • 셀프 코드 리뷰로 해당 부분에 질문 추가하겠습니다

폴더 구조에 대한 질문이시라면 크리티컬하게 실수하거나 개선이 필요한 부분은 없습니다.
나머지는 PR 본문 내에 자세히 코멘트 드렸습니다 :)

@addiescode-sj addiescode-sj merged commit abc1228 into codeit-bootcamp-frontend:Next-배수민 Aug 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