Skip to content

Conversation

@hyeonjiroh
Copy link
Collaborator

요구사항

기본

  • 스프린트 미션 1 ~ 7에 대해 typescript를 적용해주세요

심화

  • any타입을 최소한으로 써주세요

주요 변경사항

  • 삼항 연산자(? :) 대신 논리 AND 연산자(&&)를 사용을 통한 불필요한 빈 문자열 렌더링을 방지 및 코드 가독성 개선
  • HomePage.jsx
    • HomePage.jsx - section 태그 사용
  • itemApi.js
    • URLSearchParams 사용
  • AllItemsSection.jsx
    • 5페이지 이후의 페이지로 갔을 때 현재 페이지 버튼에 활성화된 스타일이 적용되지 않는 문제 해결
  • ItemCommentSection.jsx
    • useCallback 사용을 통한 eslint 의존성 문제 해결
  • ItemCommentCard.jsx
    • useEffect의 dependency list를 올바르게 설정하여 eslint 의존성 문제 해결

멘토에게

  • 로그인 되지 않은 상태의 상단바는 로그인 기능을 구현한 후 적용 예정입니다.
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@hyeonjiroh hyeonjiroh requested a review from kiJu2 March 8, 2025 17:03
@hyeonjiroh hyeonjiroh self-assigned this Mar 8, 2025
@hyeonjiroh hyeonjiroh added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Mar 8, 2025
@kiJu2
Copy link
Collaborator

kiJu2 commented Mar 10, 2025

스프리트 미션 하시느라 수고 많으셨어요.
학습에 도움 되실 수 있게 꼼꼼히 리뷰 하도록 해보겠습니다. 😊

import { Product, Comment } from "../utils/types";

const instance = axios.create({
baseURL: "https://panda-market-api.vercel.app",
Copy link
Collaborator

Choose a reason for hiding this comment

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

base URL은 환경 변수에 저장하시는게 좋습니다!

환경 변수(Environment Variable): process.env에 내장되며 앱이 실행될 때 적용할 수 있는 값입니다!

다음과 같이 적용할 수 있습니다:

// .env.development
REACT_APP_BASE_URL="http://localhost:3000"

// .env.production
REACT_APP_BASE_URL="http://myapi.com"

// 사용시
<a href={`${process.env.REACT_APP_BASE_URL}/myroute`}>URL</a>

왜 환경 변수에 저장해야 하나요?

개발(dev), 테스트(test), 실제 사용(prod) 등 다양한 환경에서 앱을 운영하게 되는 경우, 각 환경에 따라 다른 base URL을 사용해야 할 수 있습니다. 만약 코드 내에 하드코딩되어 있다면, 각 환경에 맞춰 앱을 배포할 때마다 코드를 변경해야 하며, 이는 매우 번거로운 작업이 됩니다. 하지만, 환경 변수를 .env.production, .env.development, .env.test와 같이 설정해두었다면, 코드에서는 단지 다음과 같이 적용하기만 하면 됩니다.

const apiUrl = `${process.env.REACT_APP_BASE_URL}/api`;

이러한 방식으로 환경 변수를 사용하면, 배포 환경에 따라 쉽게 URL을 변경할 수 있으며, 코드의 가독성과 유지보수성도 개선됩니다.

실제 코드 응용과 관련해서는 다음 한글 아티클을 참고해보세요! => 보러가기

export async function getItemComments(
productId: string,
{ cursor = 0 }: { cursor: number }
): Promise<GetCommentsResponse> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

오호 ~ 반환 타입을 정의하셨군요? 👍👍

pageSize: string;
order: string;
}): Promise<GetItemsResponse> {
const query = new URLSearchParams({ page, pageSize, orderBy: order });
Copy link
Collaborator

Choose a reason for hiding this comment

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

굿굿 ! 피드백 반영 너무 좋습니다 ~! 😊

Comment on lines +4 to +30
interface PrimaryButtonProps {
children: React.ReactNode;
className?: string;
onClick?: React.MouseEventHandler<HTMLButtonElement>;
disabled?: boolean;
type?: "button" | "submit" | "reset";
}

function PrimaryButton({
children,
className = "",
onClick = () => {},
disabled = false,
type = "submit",
}: PrimaryButtonProps) {
const buttonClass = `${styles.button} ${className}`;
return (
<button
type={type}
className={buttonClass}
onClick={onClick}
disabled={disabled}
>
{children}
</button>
);
}
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
interface PrimaryButtonProps {
children: React.ReactNode;
className?: string;
onClick?: React.MouseEventHandler<HTMLButtonElement>;
disabled?: boolean;
type?: "button" | "submit" | "reset";
}
function PrimaryButton({
children,
className = "",
onClick = () => {},
disabled = false,
type = "submit",
}: PrimaryButtonProps) {
const buttonClass = `${styles.button} ${className}`;
return (
<button
type={type}
className={buttonClass}
onClick={onClick}
disabled={disabled}
>
{children}
</button>
);
}
interface PrimaryButtonProps extends ButtonHTMLAttributes<HTMLButtonElement> {
className?: string;
children?: React.ReactNode;
}
function PrimaryButton({
children,
className = "",
}: PrimaryButtonProps) {
const buttonClass = `${styles.button} ${className}`;
return (
<button
type={type}
className={buttonClass}
{...rest}
>
{children}
</button>
);
}

ButtonHTMLAttributes<HTMLButtonElement> 타입으로 확장하면 컴파일 단계에서 버튼의 속성들만 받을 수 있도록 할 수 있습니다 😊

const PAGE_ARRAY = [1, 2, 3, 4, 5];
const PAGE_CHUNK_SIZE = 5;

type Order = "recent" | "favorite";
Copy link
Collaborator

Choose a reason for hiding this comment

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

굿굿 ! string이 아닌 유니온 타입으로 잘 설정하셨네요:

(이어서)

Comment on lines +23 to +27
order = "",
}: {
page: string;
pageSize: string;
order: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

(이어서) 여기도 타입을 "recent" | "favorite"로 하시면 어떨까요?

Suggested change
order = "",
}: {
page: string;
pageSize: string;
order: string;
order = "",
}: {
page: string;
pageSize: string;
order: "recent" | "favorite";

그리고 "recent" | "favorite"getItems 함에 필요한 타입이므로 타입 선언을 해당 파일에서 선언하셔도 될 것 같습니다 =)

};

const handleFileChange = (event) => {
const handleFileChange = (event: ChangeEvent<HTMLInputElement>) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

굿굿 ! 이벤트 타입을 잘 설정하셨네요 👍👍

@kiJu2
Copy link
Collaborator

kiJu2 commented Mar 10, 2025

수고하셨습니다 현지님 ! 이번 미션 정말 잘 수행하셨습니다 😊😊
나날이 실력이 늘어가시는군요 !
추가로 PrimaryButton도 추 후 이 전 피드백 참고해서 리팩토링 해보셔도 좋을 것 같아요 !

미션 수행하시느라 정말 고생 많으셨습니다 !

(참고) 이전 피드백:

MenuButton, DeleteButton, PrimaryButton.. 더 나아가서 SecondaryButton, UpdateButton 등등 목적에 따라 버튼이 생겨날 수 있을 것 같아요.
만약 디자인 시스템이 바뀌어서 일괄적으로 버튼의 radius를 바꿔야하는 상화이 온다면 유지보수하기가 어려워질 수 있겠지요?

예를 들어서 다음과 같은 컴포넌트를 만들어볼 수 있을 것 같아요:

import styles from "./Button.module.css";

function Button({
  children,
  variant = "primary",
  icon,
//  onClick = () => {},
  className = "",
//  disabled = false,
  ...rest
}) {
  return (
    <button
      className={`${styles.button} ${styles[variant]} ${className}`}
//      onClick={onClick}
//      disabled={disabled} 해당 내용들은 `rest`에 자동 등재
      {...rest}
    >
      {icon && <img src={icon} alt="" className={styles.icon} />}
      {children}
    </button>
  );
}

export default Button;

@kiJu2 kiJu2 merged commit 1248d6a into codeit-bootcamp-frontend:React-노현지 Mar 10, 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.

2 participants