Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[이현우] sprint5 #85

Conversation

gealot
Copy link
Collaborator

@gealot gealot commented Oct 27, 2024

요구사항

기본 요구사항

공통

  • Github에 스프린트 미션 PR을 만들어 주세요. ✅ 2024-10-27
  • React를 사용해 진행합니다. ✅ 2024-10-17

중고마켓 페이지

  • PC, Tablet, Mobile 디자인에 해당하는 중고마켓 페이지를 만들어 주세요. ✅ 2024-10-27
  • 중고마켓 페이지 url path는 별도로 설정하지 않고, ‘/’에 보이도록 합니다. ✅ 2024-10-27
  • 상품 데이터는 https://panda-market-api.vercel.app/docs/에 명세된 GET 메소드 “/products” 를 사용해주세요. ✅ 2024-10-27
  • 상단 네비게이션 바, 푸터는 랜딩 페이지와 동일한 스타일과 규칙으로 만들어주세요. ✅ 2024-10-27
  • 상품 데이터는 https://panda-market-api.vercel.app/docs/에 명세된 GET 메소드 “/products” 를 활용해주세요.
    • 상품 목록 페이지네이션 기능을 구현합니다.
    • 드롭 다운으로 “최신 순” 또는 “좋아요 순”을 선택해서 정렬을 구현하세요.
    • 상품 목록 검색 기능을 구현합니다.
  • 베스트 상품 데이터는 https://panda-market-api.vercel.app/docs/에 명세된 GET 메소드 “/products”의 정렬 기준 favorite을 사용해주세요.

심화 요구사항

공통

  • 커스텀 hook을 만들어 필요한 곳에 활용해 보세요.

중고마켓 페이지

  • 중고 마켓의 카드 컴포넌트 반응형 기준은 다음과 같습니다.
    • 베스트 상품
      • Desktop : 4열
      • Tablet : 2열
      • Mobile : 1열
    • 전체 상품
      • Desktop : 5열
      • Tablet : 3열
      • Mobile : 2열
  • 반응형에 따른 페이지 네이션 기능을 구현합니다.
    • 반응형으로 보여지는 물품들의 개수를 다르게 설정할때 서버에 보내는 pageSize값을 적절하게 설정합니다.

멘토에게

  • 처음부터 FSD, Next, Typescript 등 욕심(의욕)이 앞서서 익숙하지 않은 세팅에 너무 많은 시간을 들여서 정작 메인 부분을 거의 진행하지 못했습니다.
  • 아직 구현을 완성하지 못했는데 계속 하겠습니다.
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

NextJS 디렉터리 구성을 FSD(Feature Sliced Design) 패턴을 따르도록 설정하였습니다.
Commit 시마다 ESLint Linting 및 Prettier Formatting을 자동으로 수행하도록 설정
@svgr/webpack과 ky를 디펜던시에 추가했습니다.
- @svgr/webpack 패키지를 설치하여 svg 파일을 React 컴포넌트로 다룰 수 있도록 설정
- 데이터 fetch를 위해 ky 라이브러리를 디펜던시에 추가
- 공통 UI 컴포넌트(Header, Footer 등) 스타일 구성
- 프로젝트의 FSD 디렉터리 구성에 따라 tailwind.config.ts 파일 수정
- Commit 시마다 ESLint Linting 및 Prettier Formatting을 자동으로 수행하도록 설정
- husky v9 기준으로 shell script 재작성
공통적으로 사용하기 위해 shared에 ky wrapper(apiClient)를 작성하였습니다
@gealot gealot added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 완성은 아니지만 제출합니다... labels Oct 27, 2024
@gealot gealot requested a review from kimjong95 October 27, 2024 14:07
Copy link
Collaborator

@kimjong95 kimjong95 left a comment

Choose a reason for hiding this comment

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

프로젝트 전체에 사용된 기술스펙이 많이 보이는것 같아요.
첫 멘토링시간에도 강조했듯이 사용하는게 중요한게아니라 각각의 스펙들이 어떤 이점을 가져올 수 있고, 또 내부적으로는 어떻게 동작하는지 이해하는게 중요합니다.

또한 Next는 현재 스프린트과제에 포함되지 않는 기술스펙으로 알고있어요. 하지만 사용하신다면, 또한 앱라우터 기준으로 구성해주신만큼 그에맞는 컴포넌트 구성방식이 필요합니다. 폴더구조나 서버컴포넌트, RSC의 사용방식, 비동기처리, 다이나믹에 관련된 내용들 포함해서 많은 내용들에 정리가 필요할 것 같아요. 이후에도 이스펙으로 계속 구성하신다면 그에맞는 리뷰로 다시 돌아오겠습니다!

@@ -0,0 +1,7 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

eslint를 사용해주셧군요!
lint는 협업에서 중요한 요소중 하나가 될것입니다. 기본적인 컨벤션 이외에도 커스텀룰들을 가지고 팀의 코드를 관리하기 용이하죠.
기회가 되면 한번 소개해보겠습니다!

@@ -0,0 +1,4 @@
#!/usr/bin/env sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

husky도 사용해주셨군요?!

husky도 협업에서 필요한 요소를 포함합니다. 또한 CI/CD를 위해서도 사용하기도하고요 이것도 기회가 되면 실제 사례를 소개해볼게요!

}
],
"paths": {
"@/app": ["src/app"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

tsconfig로 path를 잘게 나누어주셨네요! 여기서

 "@/app": ["src/app"],
 "@/app/*": ["src/app/*"],

이렇게 두가지는 와일드카드패턴이기때문에 같은 경로를 확인해주고 있어서 중복인것 같아요.

 "@/app/styles/*": ["src/app/styles/*"],
 "@styles/*": ["./src/app/styles/*"],

여기도 같은 경로를 보고있습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

패키지매니저를 pnpm으로 구성해주었네요!
패키지매니저는 크게 3가지가 존재하는데 가각 서로 장단점을 보유하고있습니다. 이것도 기회가되면 소개하고싶네요

"dependencies": {
"3-sprint-mission-fe": "link:",
"ky": "^1.7.2",
"next": "^14.2.16",
Copy link
Collaborator

Choose a reason for hiding this comment

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

next를 통해서 프로젝트를 구성해주셧네요.
next는 프레임워크로써 리액트의 수퍼셋으로 사용되고있습니다. 하지만 리액트보다 많은 복잡도와 기술들으 요구합니다.

프로젝트를 구성할땐 기술을 사용하는 명확한 이유가 있어야된다고 생각합니다. 단순히 경험해보고싶어서 라는 이유가 있을 수는 있지만 실제 면접에서도 이렇게 말하면... 좋지않은 대답이 될 수 있겠죠?
현우님이 이 프로젝트에서 next를 사용하고 싶은 이유를 찾아서 정리해주셔야 합니다. (ex. 커머스 서비스이기때문에 SSR또는 SEO의 이점을 가지고 가고싶다, next/image를 통해 이미지 최적화를 하고싶다 등.. 그리고 이어서 이게 어떻게 next통해서 가능한지가지)

이런점이 없다면 단순히 많은 기술을 사용하는것은 구현스펙에 맞지않는 오버엔지니어링이 될 수 있습니다.

const productApi = {
async getProducts(params: ProductsParams) {
try {
const queryParams = new URLSearchParams(params as URLSearchParams);
Copy link
Collaborator

Choose a reason for hiding this comment

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

일반객체 타입인 ProductsParamsURLSearchParams로 캐스팅하는건 좋은방법이 아닐것 같습니다.
타입자체를 as로 단언하는것이 타입안정성을 깨는 첫번째 요소입니다.

axios를 사용하신다면 queryParams, queryString을 구성하는방법이 여러개 존재합니다. 그중 기본적으로 내장되어있는 paramsSerializer 를 활용할 수 있을것 같아요

const response = await apiClient.get<ApiResponse<Product>>('products', { searchParams: queryParams }).json();
return response;
} catch (error) {
console.error('Products 조회에 실패하였습니다.', error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

에러에서 로그만 남기고있습니다. 실제로 에러가발생했을때 대처방안이 필요할 수 있습니다.

또한 API에러는 서비스에 심각한 버그를 유발할 수 있으니 어떤방식으로든 사용자가 알 수 있도록 상위로 throw하거나 UI적인 요소로 노출할 수 있어야합니다. 또한 개발자가 확인할 수 있도록 로그 이외의방식으로도 에러를 컨트롤 할 수 있어야 합니다.

</ul>
<ul className='flex items-center justify-between gap-6'>
<li className='signin'>
<a
Copy link
Collaborator

Choose a reason for hiding this comment

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

next를 사용하는만큼 preload를 위해 <Link /> 사용하는것이 더 유리할수 있겠네요

import { facebookLogo, instagramLogo, twitterLogo, youtubeLogo } from '@/shared/assets/icons';

const Footer = () => {
return (
Copy link
Collaborator

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.

svg파일들을 src 하위에서 관리중인걸 볼 수 있는데요,
svg를 리액트 컴포넌트로써 동적으로 사용하려면 유효할 수 있지만 현재는 static요소로사용하고 있는것 같아요.
next에서는 이미지와 같은 static을 관리하는 방식이 존재합니다. 한번 보시면 좋을 것 같아요

@kimjong95 kimjong95 merged commit 211515a into codeit-sprint-fullstack:react-이현우 Nov 1, 2024
1 check passed
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