Skip to content

[백지연] sprint11#239

Open
jyeon03 wants to merge 10 commits intocodeit-sprint-fullstack:mainfrom
jyeon03:next-백지연-sprint11

Hidden character warning

The head ref may contain hidden characters: "next-\ubc31\uc9c0\uc5f0-sprint11"
Open

[백지연] sprint11#239
jyeon03 wants to merge 10 commits intocodeit-sprint-fullstack:mainfrom
jyeon03:next-백지연-sprint11

Conversation

@jyeon03
Copy link
Collaborator

@jyeon03 jyeon03 commented Jun 22, 2025

요구사항

공통

  • Github에 위클리 미션 PR을 만들어 주세요.

  • React 및 Express를 사용해 진행합니다.

  • TypeScript를 활용해 프로젝트의 필요한 곳에 타입을 명시해 주세요.

  • any 타입의 사용은 최소화해 주세요.

  • 복잡한 객체 구조나 배열 구조를 가진 변수에 인터페이스 또는 타입 별칭을 사용하세요.

  • Union, Intersection, Generics 등 고급 타입을 적극적으로 사용해 주세요.

  • 타입 별칭 또는 유틸리티 타입을 사용해 타입 복잡성을 줄여주세요.

  • 타입스크립트 컴파일러가 에러 없이 정상적으로 작동해야 합니다.

프론트엔드

  • 기존 React(혹은 Next) 프로젝트를 타입스크립트 프로젝트로 마이그레이션 해주세요.
  • TypeScript를 활용해 프로젝트의 필요한 곳에 타입을 명시해 주세요.

주요 변경사항

스크린샷

image

멘토에게

  • 기능 구현을 완벽하게 못하고 우선 ts, tsx로 변환했습니다.

@jyeon03 jyeon03 requested a review from basilry June 22, 2025 17:34
Copy link

@basilry basilry left a comment

Choose a reason for hiding this comment

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

전체적으로 잘 하셨습니다.

다만 공통화 작업이 조금 부족한게 아쉽네요.

해당 부분들은 리뷰 코멘트로 적었으니 확인해주세요.


export default function AddComment({ articleId, boardType }: AddCommentProps) {
const [content, setContent] = useState("");
const [token, setToken] = useState<string | null>(null);
Copy link

Choose a reason for hiding this comment

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

가급적이면 null은 초기값이나 타입으로 안쓰는게 좋습니다.

잘못하면 프로젝트에 예외적 오류를 발생시켜서 다운될 수 있어요!

await createComment(articleId, { content }, token);
setContent("");
// 댓글 목록 새로고침을 위해 부모 컴포넌트에 알림
window.location.reload();
Copy link

Choose a reason for hiding this comment

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

댓글목록 새로고침을한다고 전체 페이지를 새로고침 하는 것은 좀 비효율적이네요.

댓글을 추가하는 컴포넌트는 말 그대로 순수하게 '댓글 추가' 기능에 집중해야 합니다.

이 컴포넌트를 사용하는 곳에서 댓글 목록을 재조회하는 로직을 넣어야 하는게 맞습니다.


export default function Comments({ articleId }: CommentsProps) {
const [comments, setComments] = useState<Comment[]>([]);
const [loading, setLoading] = useState(true);
Copy link

Choose a reason for hiding this comment

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

오~ 이 부분은 잘 하셨습니다.

근데 tanstack-query를 쓰면 조회 함수가 돌 때 isLoading이 있어서요.

해당 스택을 써보시는 것도 좋을 것 같습니다.

@@ -0,0 +1,20 @@
export function calculateTimeAgo(dateString: string): string {
Copy link

Choose a reason for hiding this comment

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

이 함수는 컴포넌트에 넣는게 아니라,

src/lib/root.ts 같은 곳에 넣는게 맞습니다.

날짜와 관련된 것들, 수학적인 수식들, 정규식들은 모두 공통화해서 관리합니다.

return `${diffInHours}시간 전`;
} else if (diffInDays < 7) {
return `${diffInDays}일 전`;
} else {
Copy link

Choose a reason for hiding this comment

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

이 문장은 switch-case 문으로 변경하는게 낫겠네요.

Authorization: `Bearer ${token}`,
});

export const getAllProducts = async (): Promise<Product[]> => {
Copy link

Choose a reason for hiding this comment

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

흠.. 이 파일을 보아하니 개별적인 CRUD 함수들을 다 일일이 만들어 두셨군요.

저라면 Create, Read, Update, Delete에 관련된 메소드를 기준으로 함수를 공통화 시켜놓고, 각 개별적인 페이지나 컴포넌트에서 가져다가 커스텀해서 썼을 것 같습니다.

그게 아니라면 api 폴더를 따로 구성해서, 공통화된 api.ts에는 CRUD에 대한 기본적인 함수를 구성하고, 개별적인 productApi.ts 같은 것을 만들어서 거기다가 몰아넣는 방식도 있을 수 있겠죠.

지금과 같은 구성은 조금 비효율적으로 보입니다.

@@ -0,0 +1,86 @@
import { Article, CreateArticleRequest } from "@/types";

const API_BASE = process.env.NEXT_PUBLIC_API_URL;
Copy link

Choose a reason for hiding this comment

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

이 문장과 아래의 authHeader가 왜 자꾸 반복되고 있나요?

이건 api.ts에서 관리하고, 이걸 export 시켜서 가져와야합니다.

@@ -0,0 +1,55 @@
import { Comment, CreateCommentRequest } from "@/types";

const API_BASE = process.env.NEXT_PUBLIC_API_URL;
Copy link

Choose a reason for hiding this comment

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

위에서 말씀드린 것처럼 개선 여부가 있죠?

Authorization: `Bearer ${token}`,
});

export const getAllProducts = async (): Promise<Product[]> => {
Copy link

Choose a reason for hiding this comment

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

이 파일은 api.ts와 똑같아 보이네요. 확인해주세요.

@@ -0,0 +1,133 @@
// 사용자 관련 타입
Copy link

Choose a reason for hiding this comment

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

보통 index.ts가 아니라 root.ts라고 합니다.

index는 무조건 진입해야하는 관문같이 여겨지는 단어라서요.

그리고 아래를 보니 Article부터 Product까지 개별 도메인(카테고리 범위 같은 개념)에서 사용하는 인터페이스조차 모아놓으셨네요. 이건 분리할 필요가 있어보입니다.

@basilry
Copy link

basilry commented Jun 24, 2025

브랜치가 컨플릭트 난 부분이 있어서 머지는 안했습니다. 확인해주세요~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants