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

[김보경] week20 #1074

Conversation

bokeeeey
Copy link
Collaborator

@bokeeeey bokeeeey commented May 12, 2024

요구사항

기본

  • 변경된 api를 활용해 주세요.
  • 모달에 필요한 api 요청을 만들어 기능을 완성해 주세요.
  • api 요청에 TanStack React Query를 활용해 주세요.
  • Github에 위클리 미션 PR을 만들어 주세요.
  • React, Next.js를 사용해 진행합니다.

주요 변경사항

  • Tanstack Query로 Auth기능을 구현 하였습니다
  • getServerSideProps으로 유저 쿠키값을 통한 사전렌더링을 시도 해 보았습니다
  • LandingPage를 조금 더 리액트 스럽게 변경해 보았습니다.

스크린샷

멘토에게

  • 처음부터 다시 정리하며 이해해 보자는 생각으로 다시 제작중입니다... 많은 부분 구현하지 못했지만 다음주까지 완성하여 보겠습니다.
  • TanstackQuery의 Mutation을 활용하여 Auth기능을 구현해 보았습니다. Mutation과 Mutate의 로직을 분리하여 사용중인데 알맞게 이해하고 사용하고 있는것일까요?
  • getLayout기능을 활용하여 Layout 구성을 시도해 보았는데 folderId의 페이지 구성에서는 변경되는 부분을 제외한 다른 나머지 전체를 Layout으로 구성해야 할 지 고민입니다. 지금처럼 Navigetion과 Footer만 Layout으로 구성하는게 좋을까요?
  • getServerSideProps의 이해에서 시간이 많이 걸렸는데 �의도에 알맞게 구성을 해 놓은것 일까요?
  • Next의 ServerSide환경에서는 기존 fetch방식만 사용 가능하여 TanstackQuery가 Next환경에서 굳이 필요한가? 라는 의문이 듭니다. 멘토님 의견은 어떠신가요?
  • 폴더구성에 대한 고민이 아직까지 끊이지않고 이어지는듯 합니다. 현재 구성도 크게 마음에 들지 않는부분인데 혹시 폴더 구성에 대해 방향을 잡아주시거나 추천해주실만한 structrue나 방식 혹은 자료가 있을까요?
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

bokeeeey added 23 commits May 5, 2024 22:16
Copy link

vercel bot commented May 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
4-weekly-mission ❌ Failed (Inspect) Jun 11, 2024 6:30am
4-weekly-mission-ursd ❌ Failed (Inspect) Jun 11, 2024 6:30am

@bokeeeey bokeeeey requested a review from devToram May 12, 2024 09:37
@bokeeeey bokeeeey self-assigned this May 12, 2024
@bokeeeey bokeeeey added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성 죄송합니다.. labels May 12, 2024
Copy link
Collaborator

@devToram devToram left a comment

Choose a reason for hiding this comment

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

코드리뷰가 늦어져서 죄송해요ㅠㅠ
수고하셨습니다 :)

TanstackQuery의 Mutation을 활용하여 Auth기능을 구현해 보았습니다. Mutation과 Mutate의 로직을 분리하여 사용중인데 알맞게 이해하고 사용하고 있는것일까요?

사용은 잘 하고 계신데, 개인적으로 이걸 페이지 내부에서 선언해버리면 너무 복잡해져서 코드리뷰에서 말씀드린 것처럼 훅으로 빼시면 좀 더 깔끔해질 거 같아요!

getLayout기능을 활용하여 Layout 구성을 시도해 보았는데 folderId의 페이지 구성에서는 변경되는 부분을 제외한 다른 나머지 전체를 Layout으로 구성해야 할 지 고민입니다. 지금처럼 Navigetion과 Footer만 Layout으로 구성하는게 좋을까요?

음 이건 완전 개인 취향일 거 같아요! (앞으로 확장될 일은 없겠지만) 페이지가 더 많아지는데에 유연하게 대처하고 싶으시면 지금이 맞고, 중복을 줄이는데에는 최대한 Layout 에 끼워넣는 게 맞을 거 같아요. 저는 보통 전자를 하는 편입니다.

getServerSideProps의 이해에서 시간이 많이 걸렸는데 �의도에 알맞게 구성을 해 놓은것 일까요?

넵 완전..!

Next의 ServerSide환경에서는 기존 fetch방식만 사용 가능하여 TanstackQuery가 Next환경에서 굳이 필요한가? 라는 의문이 듭니다. 멘토님 의견은 어떠신가요?

저는 그래도 프리페칭 측면에서 필요하다고 생각해요!

폴더구성에 대한 고민이 아직까지 끊이지않고 이어지는듯 합니다. 현재 구성도 크게 마음에 들지 않는부분인데 혹시 폴더 구성에 대해 방향을 잡아주시거나 추천해주실만한 structrue나 방식 혹은 자료가 있을까요?

저도 지금 저희 팀 프로젝트 구성 맘에 안들어요...ㅋㅋㅋㅋㅋㅋㅋㅋㅋ 정답은 진짜 없지만 그래도 괜찮은 구조라고 생각하는 글 넣어두고 가요^^;;
https://miriya.net/blog/cliz752zc000lwb86y5gtxstu

Copy link
Collaborator

Choose a reason for hiding this comment

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

svg 파일들은 .svg 파일로 저장하시고 불러올 때는 import Ic_Eye_Off from '@/public/svgs/asdf.svg'; 식으로 불러올 수 있어요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

svgr을 사용하면 해결될 문제이긴 한데... 재사용성을 위해서 컴포넌트 식으로 선언해서 쓰는방식은 좋지 않은 방법일까요?

return (
<main className={cn("container")}>
{LANDING_CONTENTS.map((content, i) => {
return <Content key={i} content={content} id={i} />;
Copy link
Collaborator

Choose a reason for hiding this comment

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

key 를 index 로 주는 건 안주는 것과 똑같아요! (key 안주면 리액트 내부적으로 index 로 줌)
unique 한 값이 없어 어쩔 수 없이 준 것이라면 주석이라도 다시는 걸 추천해요!

Comment on lines +24 to +29
if (favorite) {
setFolderId(favorite.id);
return;
}

setFolderId(null);
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
if (favorite) {
setFolderId(favorite.id);
return;
}
setFolderId(null);
setFolderId(favorite?.id ?? null);

Find in-depth information about Next.js features and&nbsp;API.
</p>
</a>
export const getServerSideProps: GetServerSideProps = async (context) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 부분 로직을 보면 쿠키에 accessToken 이 있는지 확인하고, 토큰에 따라 prefetch 하는 로직이 계속 반복되는 거 같아요!
HOF 를 이용하면 중복 제거할 수 있을 거 같습니다
https://velog.io/@navyjeongs/Next.js-SSR%EC%97%90%EC%84%9C-HOF%EB%A5%BC-%ED%99%9C%EC%9A%A9%ED%95%98%EC%97%AC-%EC%BF%A0%ED%82%A4-%EC%A0%84%EC%86%A1%ED%95%98%EA%B8%B0

Comment on lines +34 to +46
const { mutate: loginMutation } = useMutation({
mutationFn: postSignIn,
mutationKey: ["token"],
onSuccess: () => {
// token값을 cookie에 저장하여 사용중인데 localStorage관련한 로직을 추가적으로 넣어주는게 맞는지 고민중입니다
// queryClient.invalidateQueries({
// queryKey: ["accessToken"],
// });
// queryClient.invalidateQueries({
// queryKey: ["refreshToken"],
// });
},
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

react-query 부분을 커스텀 훅으로 빼면 실제 페이지부에 로직이 명확하게 보이는 장점이 있어서 고려해보심 좋을 거 같아요!
https://velog.io/@codns1223/React-react-query-%EC%BB%A4%EC%8A%A4%ED%85%80-%ED%9B%85%EC%9C%BC%EB%A1%9C-%ED%99%9C%EC%9A%A9

interface InputFieldProps extends InputHTMLAttributes<HTMLInputElement> {
label?: string;
errorMessage?: string;
suffixIcon?: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

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

boolean 변수명이라 needSuffixIcon 정도 추천드려요!

const { name } = favorite || { name: "전체" };

return (
<button
Copy link
Collaborator

Choose a reason for hiding this comment

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

type 명시해주시면 좋아용!

Copy link
Collaborator

Choose a reason for hiding this comment

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

date-fns 와 같은 라이브러리로 간단히 할 수 있을 거 같은데 참고해보셔요~
https://date-fns.org/

@devToram
Copy link
Collaborator

@bokeeeey 님, 충돌 해결 부탁드립니다..! (머지를 해야해서ㅠ)

@devToram
Copy link
Collaborator

요거 머지를 해야하는데에... 컨플릭 해결 부탁드립니다... 후엥... @bokeeeey

@devToram devToram merged commit c0b9744 into codeit-bootcamp-frontend:part3-김보경 Jun 25, 2024
1 of 3 checks 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