Skip to content

Conversation

@cskime
Copy link
Collaborator

@cskime cskime commented Aug 7, 2025

요구사항

  • [김참솔] Sprint 7 #103 코드 리뷰 변경사항을 반영하고 일부 코드를 개선해서 기초 프로젝트 이전에 스프린트 미션을 마무리하기 위해 추가 PR을 요청합니다.

주요 변경사항

  • CSS class name으로 style을 설정하는 코드를 모두 styled-components를 사용하도록 변경했습니다.
  • 파일 이름을 모두 kebab case로 변경하고, 더 이해하기 쉬운 이름으로 변경했습니다.
  • [김참솔] Sprint 7 #103 리뷰 내용을 반영했습니다.

멘토에게

  • 코드를 정리하면서 아래와 같이 폴더 구조도 변경했는데 잘 나눈 것인지 궁금합니다.
    src
    ㄴ api
    ㄴ assets
    ㄴ components
    ㄴ features
      ㄴ comment
        ㄴ api
        ㄴ components
      ㄴ product
        ㄴ api
        ㄴ components
    ㄴ hooks
    ㄴ layouts
    ㄴ pages
    ㄴ styles
    ㄴ utils
    ㄴ app.jsx
    ㄴ main.jsx
    
    • features 폴더를 추가하고 아래에 기능 별 파일들을 모아 두었습니다.
    • features/*/components에는 별도의 파일로 분리했지만 특정 기능에 종속된 component들을 모아 두었습니다.
    • app.jsxmain.jsx는 root component 및 entry point 이므로 src 바로 아래에 위치했습니다.
  • Component에서 api 요청 함수를 호출하는 로직을 custom hook으로 만들어서 사용한다는 글을 본 적이 있는데요. 위와 같은 폴더 구조를 사용한다면 그런 파일들은 hooks에 들어가야 할까요? 아니면 api에 들어가야 할까요?

@cskime cskime requested a review from kiJu2 August 7, 2025 12:19
@cskime cskime self-assigned this Aug 7, 2025
@cskime cskime added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Aug 7, 2025
@kiJu2
Copy link
Collaborator

kiJu2 commented Aug 8, 2025

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

@kiJu2
Copy link
Collaborator

kiJu2 commented Aug 8, 2025

코드를 정리하면서 아래와 같이 폴더 구조도 변경했는데 잘 나눈 것인지 궁금합니다.

src
ㄴ api
ㄴ assets
ㄴ components
ㄴ features
  ㄴ comment
    ㄴ api
    ㄴ components
  ㄴ product
    ㄴ api
    ㄴ components
ㄴ hooks
ㄴ layouts
ㄴ pages
ㄴ styles
ㄴ utils
ㄴ app.jsx
ㄴ main.jsx
  • features 폴더를 추가하고 아래에 기능 별 파일들을 모아 두었습니다.
    • 👍👍
  • features/*/components에는 별도의 파일로 분리했지만 특정 기능에 종속된 component들을 모아 두었습니다.
    • 👍👍
  • app.jsx와 main.jsx는 root component 및 entry point 이므로 src 바로 아래에 위치했습니다.
    • 👍👍

넵넵 FSD와 전통적인 리액트 폴더 구조를 반영하여 잘 나누신 것으로 보입니다 👍👍

@kiJu2
Copy link
Collaborator

kiJu2 commented Aug 8, 2025

Component에서 api 요청 함수를 호출하는 로직을 custom hook으로 만들어서 사용한다는 글을 본 적이 있는데요. 위와 같은 폴더 구조를 사용한다면 그런 파일들은 hooks에 들어가야 할까요? 아니면 api에 들어가야 할까요?

src
ㄴ api
ㄴ assets
ㄴ components
ㄴ features
  ㄴ comment
    ㄴ api
    ㄴ components
  ㄴ product
    ㄴ api
    ㄴ components
ㄴ hooks
ㄴ layouts
ㄴ pages
ㄴ styles
ㄴ utils
ㄴ app.jsx
ㄴ main.jsx

공통 자원인 경우에는 상위의 hooks에 넣는게 어떨까 싶습니다 😉
제가 제안한 근거는 책임의 분리인데요.
"데이터를 외부 서버로 부터 받아온다"의 책임을 가진 API 함수와,
"외부 통신에 의한 리액트 상태를 관리한다"는 hooks를 분리하는 것이 관심사 분리에 용이할 것으로 사료되어 hooks 안에 넣는게 어떨지 제안드려봅니다. 😊

사실, 스트럭쳐 영역은 컨벤션의 의도에 따라 다르기에 명확한 가이드를 제안하기는 어려우나, 선임 개발자 입장에서 답변드려봅니다. 😊

Comment on lines +24 to +45
class HttpClient {
constructor({ baseUrl }) {
this.baseUrl = baseUrl;
}

async get(target, params) {
let url;
if (this.baseUrl) {
url = createUrl(this.baseUrl, target, params);
} else {
url = target;
}

const response = await fetch(url, { method: "GET" });
if (!response.ok) {
throw new Error(`HTTP error (status ${response.status}`);
}

const json = await response.json();
return json;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

크으 직접 만들어보시는걸 택하셨군요 👍👍

훌륭한 도전입니다.

get에 대해서 잘 작성하셨네요 👍

favorite: "좋아요순",
};
const ORDER_BY_VALUES = Array.from(Object.keys(ORDER_BY_TITLE));
export const ORDER_BY_DEFAULT = "recent";
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 상수는 관련 api 함수에 있어도 좋을 것 같네요 !

현재 위치는 order-by-select.jsx이네요.
"해당 컴포넌트의 기본 값"을 의미한다면 현재 그대로 둬도 좋으나,
"API 통신에 필요한 파라메터의 기본 값"을 의미한다면 api 함수 파일로 옮겨도 되겠어요 😉

Comment on lines +2 to +7

async function fetchProducts({
keyword = "",
page = 1,
pageSize = 10,
orderBy = "recent",
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
async function fetchProducts({
keyword = "",
page = 1,
pageSize = 10,
orderBy = "recent",
const ORDER_BY_DEFAULT = "recent";
async function fetchProducts({
keyword = "",
page = 1,
pageSize = 10,
orderBy = ORDER_BY_DEFAULT,

Comment on lines +5 to +20
const PROFILE_INFO_STYLE = {
[USER_PROFILE_CARD_SIZE.large]: {
infoGap: 2,
fontSize: 14,
lineHeight: 24,
cardGap: 16,
avatarSize: 40,
},
[USER_PROFILE_CARD_SIZE.small]: {
infoGap: 4,
fontSize: 12,
lineHeight: 18,
cardGap: 8,
avatarSize: 32,
},
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

굿굿 ! 함수를 지우고 객체로 만드셨군요 ! 👍👍👍

function을 실행시키는게 아니라 객체 인덱싱 구조로 변경되었기에 성능도 더 개선되었을거라 사료됩니다. 😊

Comment on lines +24 to +30
const sortedComments = useMemo(
() =>
comments.sort(
(a, b) => Date.parse(b.updatedAt) - Date.parse(a.updatedAt)
),
[comments]
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

굿굿 ! 정렬은 자원이 많이 들어갈 수 있지요 👍

메모이제이션을 활용하여 성능을 높이셨군요 👍

@kiJu2 kiJu2 merged commit 4d7d33f into codeit-bootcamp-frontend:React-김참솔 Aug 8, 2025
@kiJu2
Copy link
Collaborator

kiJu2 commented Aug 8, 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