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

feat: Next / TypeScript / React Query / Zustand / Vanilla Extract를 사용하여 Real World 1차 기능 구현을 완료합니다. #155

Merged
merged 50 commits into from
May 16, 2024

Conversation

hyeon9782
Copy link
Collaborator

📌 이슈 링크

  • close #

📖 작업 배경

  • Next.js / TypeScript / React Query / Zustand / Vnilla Extract를 사용하여 Real World를 구현했습니다.

🛠️ 구현 내용

  • useArticles 커스텀 훅을 통해 Article 조회 기능 구현 (Global / Feed / Tags / My / Favorited), 좋아요, 좋아요 취소 기능 구현
  • Article 등록, 수정, 삭제 기능 구현
  • 댓글 조회, 등록, 삭제 기능 구현
  • useProfile 커스텀 훅을 통해 Follow , UnFollow 기능 구현
  • useAuth 커스텀 훅을 통해 계정 관련 기능 구현 (Login, Logout, Signup)
  • 회원 정보 수정 기능 구

💡 참고사항

  • cookies를 httpOnly로 사용하라고 하셔서 route handler를 통해 구현했는 데 더 좋은 방법이 있을까요?
  • Error 처리는 어떤식으로 하는 것이 효율적으로 하는 것인지 모르겠습니다. 현재 생각나는 방식은 http 모듈에서 error 또는 status 값이 error 코드가 넘어오면 Error를 throw 하여 React Query의 onError 또는 error를 통해 처리하는 것입니다.
  • axios 대신 fetch를 사용하고 있습니다. Client Side에서 호출하는 것과 Server Side에서 호출할 때 BASE_URL이 달라지는 데 하나의 http 모듈을 사용하는 것이 맞는지? 현재는 Server Side에서 fetch를 사용할 때만 사용하고 있습니다.
  • 아 그리고.. Next.js를 사용할 때 속도가 심각하게 느린 거 같은데 혹시 이거 저만 그런건가요..? React에서는 이런 속도가 아니였는 데 제가 설정을 잘못 건드린 건가 싶지만 딱히 뭘 한 게 없어서 왜 이런지 모르겠네요.
  • 원래는 리팩토링을 해서 깔끔하게 PR을 올리고 싶었지만 아직 너무 부족해서 제가 하고 있는 것들이 맞는 방향인지 판단을 못하겠네요.. 인성님 말씀대로 일단 올려봅니다.
  • 해당 프로젝트에 사용했던 모든 기술 스택은 다 처음 사용해보는 것이기 때문에 제가 구현하는 방식이 맞는 방식인지 모르겠습니다. 진짜 사소한 것부터 구조적인 부분까지 더 개선할 부분이 있다면 말씀해주시면 열심히 고쳐보겠습니다. 감사합니다.

아래는 스터디가 끝나고 개인적으로 진행하고 싶은 부분입니다.

  • cookies 넣는 부분 util 함수로 빼기
  • route handler Response 일관성 있게 통일하기
  • Error Message에 따라 알맞은 에러 처리
  • 사용하지 않는 함수들 제거하기
  • 좋아요 & 팔로우 버튼
    • Optimistic Updates를 활용한 사용자 경험 향상
    • 일관된 UI를 위해 button 크기 고정 (좋아요 수가 99개가 넘어갈 경우 99+로 표시)
  • ArticlePreview
    • 제목 크기 고정 및 크기를 넘어가면 ... 처리
    • 한 번 봤던 게시글 표시하기 (체크 표시 또는 배경색을 다르게)
  • alert을 사용하지 않고 Dialog 컴포넌트 구현
  • 페이지 별 스켈레톤 UI 적용
  • Vanilla Extract 기능을 활용하여 CSS 정리 (급하게 하느라 너무 막 짠 거 같습니다..)
  • 테스트 코드 추가

🖼️ 스크린샷


InSeong-So and others added 30 commits September 2, 2023 17:29
@hyeon9782 hyeon9782 added the feature A new feature label Oct 8, 2023
@hyeon9782 hyeon9782 requested a review from InSeong-So October 8, 2023 19:56
@hyeon9782 hyeon9782 self-assigned this Oct 8, 2023
Copy link
Member

@InSeong-So InSeong-So left a comment

Choose a reason for hiding this comment

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

89개.... 😇
핵심 내용은 코멘트 남겨드렸어요!
7주동안 고생 많았습니다 👏👏👏

@@ -0,0 +1,43 @@
import { HTTP_METHOD, COMMON_HEADERS } from '@/constants/api';
import { API_BASE_URL } from '@/constants/env';
Copy link
Member

Choose a reason for hiding this comment

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

환경변수는 상수 파일보다 .env에서 관리하면 좋을 것 같네요! 네이밍에서 그 의도를 드러낸 바와 같이 말이죠

Comment on lines +6 to +8

constructor() {}

Copy link
Member

Choose a reason for hiding this comment

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

이러면 생성자 함수가 의미가 있을까... 싶습니다!
객체와 클래스의 차이가 무엇일까요?

Comment on lines +9 to +10
async request(url: string, options: any, method: string) {
const response = await fetch(`${this.BASE_URL}${url}`, {
Copy link
Member

Choose a reason for hiding this comment

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

fetch API의 options는 타입 선언 파일에 제공되므로, any 보다는 해당 타입을 확장해서 사용하거나 차용하는 것을 추천드려요.

Comment on lines +19 to +23
if (!response.ok) {
throw new Error(`HTTP error! Status: ${response.status}`);
}

return response;
Copy link
Member

Choose a reason for hiding this comment

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

서버 에러 또는 네트워크 에러일 때 핸들링은 어떻게 될까요?

app/[username]/page.tsx Show resolved Hide resolved
components/article/ArticlePreview.tsx Show resolved Hide resolved
Comment on lines +21 to +32
const queryClient = useQueryClient();

const onSuccess = () => {
queryClient.invalidateQueries({ queryKey: ['articles', tab] });
};

const onError = () => {
// 권한이 없을 경우 login 페이지로 이동
router.push('/login');
};

const { favorite, unFavorite } = useArticles({ onSuccess, onError });
Copy link
Member

Choose a reason for hiding this comment

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

커스텀 훅으로 분리시키면 어떨까요? 한 컴포넌트에서 너무 많은 역할을 갖고 있네요!

middleware.ts Show resolved Hide resolved
Comment on lines +4 to +18
const useAuth = ({
loginSuccess,
loginError,
signupSuccess,
signupError,
signoutSuccess,
signoutError,
}: {
loginSuccess?: (res: UserResponse) => void;
loginError?: (err: Error) => void;
signupSuccess?: (res: UserResponse) => void;
signupError?: (err: Error) => void;
signoutSuccess?: (res: UserResponse) => void;
signoutError?: (err: Error) => void;
}) => {
Copy link
Member

Choose a reason for hiding this comment

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

비슷한 프로퍼티가 너무 많은 것 같은데, 로그인/로그아웃, 회원가입/탈퇴로 묶는 건 어떨까요?

hooks/useArticles.ts Show resolved Hide resolved
@InSeong-So InSeong-So merged commit 01f8117 into pagers-org:team4/hyeon9782 May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants