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

[담] 간단 코드 리뷰 #1

Open
j2h30728 opened this issue Aug 13, 2023 · 0 comments
Open

[담] 간단 코드 리뷰 #1

j2h30728 opened this issue Aug 13, 2023 · 0 comments

Comments

@j2h30728
Copy link

j2h30728 commented Aug 13, 2023

안녕하세요.

재현님.

리액트 스터디가 끝난지 어언 3주가 넘어가버렸네요.

마지막 조활동을 제일 열심히 해주셨어서 정말 감사했어요.
그 마음을 어떻게 전달 해드릴까 고민하다가 이전에 꼭 하겠다던 코드리뷰를 해야겠다는 생각이 들더라구요.
물론 코드리뷰를 바라지 않으셨을수도 있겠지만 열심히 작성해보겠습니다.

백엔드 지식은 재현님이 더 풍부할거에요. 혹시나 잘못된 부분이 있다면 알려주세요.

Index

const tweets = await prisma.tweet.findMany({
      include: {
        _count: { select: { likes: true } },
        user: { select: { username: true } },
      },
      orderBy: {
        createdAt: "desc",
      },
    });
  • 저렇게 가져와서 createdAt에 인덱싱을 해놓고 싶은데 SQlite를 사용하는 프리즈마는 지원하지 않는 것 같다.
    • 인덱싱: 값들을 미리 정리해놔 탐색할 때 빠르게 값들을 입력, 탐색, 삭제 할수 있게 해놓는 것
    • 기본키는 인덱싱이 항상 되어있고, 이 앱에서는 id가 클 수록 최신에 만든 것이니 id 역순으로 데이터를 가져와도 큰 무리는 없을 것 같다.

혹시 여기서 말씀 하셨던 index가 이것인가요?

[Prisma Schema Index](https://www.prisma.io/docs/concepts/components/prisma-schema/indexes)

예시를 확인해보니 @@index([author, created_at(sort: Desc)]) 와 같이 사용되고 있습니다.

api 타입관련

Next.js 의 API Route를 사용해 저희 가 손쉽게 response 형태를 정할 수 있다면, 타입을 공통으로 쓰는 것은 어떨까요?
저는 백엔드 단에서와 클라이언트 단에서 동일하게 타입을 사용하고있습니다. 물론 필요하면 수정은 하지만요.

export interface ResponseType<T> {
  [key: string]: any; 
  data: T | null;
  isSuccess: boolean;
  message: null | string;
  statusCode: number;
}

이렇게 제네릭을 사용해서 api response type통일 시켰습니다.

  • api route : async function handler(req: NextApiRequest, res: NextApiResponse<ResponseType<User>>) {
  • client : const [mutate, { data, error, isLoading }] = useMutation<ResponseType<User>>();

재현님의 page route들을 보면서 제가 초기제작때 생각 했던 것이 생각나더라구요.
'페이지마다 타입을 계속 선언해주지않고 api 마다 반환되는 타입을 동일하게 두어 유지보수가 편하게 만드는 것이 어떨까?' 라는 생각이었어요.
동일하지는 않지만 관련 내용을 디스코드에서 질문에 남겼었습니다. (스터디 질문 스레드 링크)

회원가입

  • 현재 회원가입 api 경로가 sign-in. 으로 되어있습니다. sign-up 또는 create-account는 어떨까요?
image
  • API Route 내의 prisma 에러 처리 코드 보고 많이 배웠습니다! 이런 방식도 있다니... 공식문서를 좀 더 꼼꼼하게 읽어야겠어요.

메인페이지

저희 오프라인모각코 했을 때, 제가 재현님 디자인을 참 좋아했었던걸로 기억해요. 버튼을 누르면 위로 슝 올라오는 작성폼이요.
코드로 보니 더 신기하네요. 저는 이런 경우에는 모달을 바로 생각하는데 많이 배웟습니다.

저번에 커스텀훅을 어떻게 나누냐고 저에게 여쭤보셨던 것 같아요.
저는 비즈니스 로직을 작성하는게 좀 힘들어할 때도 많아서 커스텀 훅을 잘 작성하는 편도아니고, 처음부터 만들고 시작하지도 않아요.
코드를 읽었을 때 과한 구체가 들어가 있거나 다른 페이지에서도 필요한 동작이라면 만들었던 것 같아요.

예를들면, 트위터 작성 모드가 켜지면 스크롤을 막는 함수를 커스텀 훅으로 바꾸고, useLockBodyScroll(isCreateTweetMode) 대충 이렇게 쓰지않았을 까 싶어요. 아니면 조건문 쓸 수도 있고? (아! 커스텀 훅 이야기를 먼저 꺼내주셨어서 이야기 해보는거에요.)
제가 리액트1기때 캐럿마켓졸업작품을 하면서 코드를 설명하는 주석을 굉장히 많이 달았었거든요.
그런 주석을 보지않으면 코드가 이해가지 않을 정도로 양도 많고 추상화 되지않은 코드였기 때문이에요.
그래서 2기에는 그런 부분을 방지하고 싶어서 식별자 명이나 그런 부분들을 생각해서 추상화 시키는걸 고려해 볼 것 같아요.

헤헤 실컷 다 써놓긴했지만, 저보단 더 고수이신 분들께 여쭤보는 건 어떨까요? 저도 사실 잘 모르겠어요. 다른 분들 코드 볼 때마다 깜짝 깜짝 놀래~

useUser 훅과 Next.js의 middleware

저는 로그인 여부를 확인하려고 데이터베이스를 확인한다는 부분에서 되게 의문을 가졌어요. 물론 swr 은 같은 엔드포인트에 대해서 캐싱 처리는 해주지만, 데이터베이스를 계속 확인하는 방법 말고 다른건 없을까 궁금했었습니다.

Next.js Middleware
물론 여러가지 대체할 수 있는 방법이 있겠지만, 저는 넥스트에서 제공하고있는 미들웨어를 보자마자, useUser 훅을 삭제 해버렸습니다.
프레임워크가 써라고 만든 기능! 바로 호로록!
페이지라우팅 되는 직전에 미들웨어가 동작해서 요청에서 쿠키가 있는지 확인 하는 미들웨어로 생성했습니다.

재현님도 다른 방법을 생각하고 있지 않으시다면, 미들웨어를 한 번 적용 해보셔요!

빠빠링

개인적으로 재현님의 코드리뷰를 위해 코드를 뜯어본 경험은 정말 값진 시간이었어요.
사실 모르는것들도 많아서 다른 분께도 질문하고 그랬습니다.

리액트2기 1조 활동 정말 열심히 해주셨어서 감사했어요.
제가 리액트 마스터 챌린지와 넥스트시작하기에서 조활동으로 힘들어했어서, 스터디 초기에 재현님이 정말로 잘해주셨던걸 잊고 있었어요.
첫 조활동 뿐만 아니라 타입스크립트 챌린지 발표, 그리고 마지막 캐럿마켓 졸업작품까지 조활동 열심히 해주셔서 감사했어요.
저한테 미안하다고 말하지마요. 제가 더 미안해요. 헤헤.

제가 잘못된 내용으로 작성했다면 꼭 말씀해주세요.

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

No branches or pull requests

1 participant