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

[이승현] Sprint11 #314

Conversation

leesh7048
Copy link
Collaborator

@leesh7048 leesh7048 commented Aug 23, 2024

요구사항

기본

회원가입

  • 유효한 정보를 입력하고 스웨거 명세된 “/auth/signUp”으로 POST 요청해서 성공 응답을 받으면 회원가입이 완료됩니다.
  • 회원가입이 완료되면 “/login”로 이동합니다.
  • 회원가입 페이지에 접근시 로컬 스토리지에 accessToken이 있는 경우 ‘/’ 페이지로 이동합니다.

로그인

  • 회원가입을 성공한 정보를 입력하고 스웨거 명세된 “/auth/signIp”으로 POST 요청을 하면 로그인이 완료됩니다.
  • 로그인이 완료되면 로컬 스토리지에 accessToken을 저장하고 “/” 로 이동합니다.
  • 로그인/회원가입 페이지에 접근시 로컬 스토리지에 accessToken이 있는 경우 ‘/’ 페이지로 이동합니다.

메인

  • 로컬 스토리지에 accessToken이 있는 경우 상단바 ‘로그인’ 버튼이 판다 이미지로 바뀝니다.

멘토에게

  • 일단 전 스프린트미션에서 피드백 받은 부분만 적용시켰고, 스프린트11요구사항은 주말까지 추가해보도록 하겠습니다..

@leesh7048 leesh7048 requested a review from 201steve August 23, 2024 13:28
@leesh7048 leesh7048 added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Aug 23, 2024
Copy link
Collaborator

@201steve 201steve left a comment

Choose a reason for hiding this comment

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

수고많으셨습니다!!

개발하시면서 어? 이 코드 아까 썼던것같은데? 가 생각나면 가까운 상위 폴더로 추출해서 재사용 해보는 연습이 리팩터링 하는데에 도움이 되더라구요.

지금도 충분히 잘 하고계십니다. 힘내세요!!

Comment on lines +4 to +14
const useDateFormat = () => {
const format = useCallback((date: Date) => {
const newDate = new Date(date);
const formatDate = `${newDate.getFullYear()}.${String(
newDate.getMonth() + 1
).padStart(2, "0")}.${String(newDate.getDate()).padStart(2, "0")}`;
return formatDate;
}, []);

return { format };
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

반복 사용되는 기능을 잘 분리해주셨습니다!
아마 useCallback을 쓰기위해서 커스텀 훅으로 만드신것같아요. dateFormat을 반환하는 함수라서 많은 계산이 필요한 함수는 아닐것같아요. 그래서 useCallback없이 utill 함수로 분리해도 될것같아요.

@@ -8,6 +8,7 @@ import Link from "next/link";
import { getArticles } from "@/lib/articleApi";

const BestArticles = () => {
const MAX_COUNT = 9999;
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳. 상수를 변수에 담아서 어떤 뜻인지 명확하게 해주는방식 좋습니다. 👍

Comment on lines +2 to +18
export const API_ENDPOINTS = {
getArticles: (
page: number,
pageSize: number,
orderBy?: string,
keyword?: string
): string => {
const orderByQuery = orderBy ? `&orderBy=${orderBy}` : "";
const keywordQuery = keyword ? `&keyword=${keyword}` : "";
return `/articles?page=${page}&pageSize=${pageSize}${orderByQuery}${keywordQuery}`;
},
getArticle: (articleId: number): string => `/articles/${articleId}`,
getComments: (articleId: number, limit: number): string =>
`/articles/${articleId}/comments?limit=${limit}`,
addUser: "/auth/signUp",
loginUser: "/auth/signIn",
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

address용으로 잘 분리 해 주셨는데,
querystring은 컴포넌트에서 추가,수정,삭제, 업데이트 하는건 어떨까요? address에서는 queryString에 대한 내용을 몰라도 되고, 컴포넌트에서 직접 추가,수정,삭제, 업데이트하면 주소 객체를 관리하기 좀더 수월할 것같아요.

만약 지금의 방식대로면 getArticles에 query가 늘어날 때마다 변수가 하나씩 늘어나거나,
수정될 때마다 문자열에 오타는 없는지 찾아가면서 유지보수하는 시간이 늘어날 것같아요.

next.js 공식문서에 searchParams, params사용법 입니다 참고 해 보세요 :-)

Comment on lines 3 to 5
const instance = axios.create({
baseURL: "https://panda-market-api.vercel.app/",
baseURL: process.env.NEXT_PUBLIC_API_BASE_URL,
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

인스턴스 잘 만들어주셨습니다!
instance함수가 호출되려면 이 파일이 import 되어있어야 하는데, main.ts 혹은 최상단에서 import 하고있는지, interceptor는 잘 동작하는지 한번 확인 해주시는것도 좋아요 :-)

Comment on lines +13 to +15
} catch (error) {
console.error("Error in signup: ", error);
throw error;
Copy link
Collaborator

Choose a reason for hiding this comment

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

ctach 문에서 다루고 있는건 없지만
회원가입이 실패하는 조건이 어떤게 있는지 한번 생각해보는것도 도움이 될것같아요 :-)

<button className={styles.commentSubmitBtn}>등록</button>
<button
className={styles.commentSubmitBtn}
disabled={commentValue ? false : true}
Copy link
Collaborator

Choose a reason for hiding this comment

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

문자열에 공백도 문자열의 길이로 취급됩니다.
commentValue가 있는지 없는지로 판단하기에는 조금 범위가 넓은것같아요

const onSubmit = async (data: LoginUserRequest) => {
try {
const res: { data: AuthResponse } = await loginUser(data);
console.log(res);
Copy link
Collaborator

Choose a reason for hiding this comment

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

commit 하기전에 console.log 한번씩 훝어주시는 습관을 가지시면 좀더 편하실꺼에요 :-)

},
pattern: {
value:
/^[0-9a-zA-Z]([-_\.]?[0-9a-zA-Z])*@[0-9a-zA-Z]([-_\.]?[0-9a-zA-Z])*\.[a-zA-Z]{2,3}$/i,
Copy link
Collaborator

Choose a reason for hiding this comment

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

정규표현식도 변수에 담거나, 정규표현식에 입력값을 test해서 반환된 결과를 사용하는것도 가능합니다.
JSX를 쭉~ 읽다가 이렇게 복잡한 코드가 나오면 코드를 읽어내려가는데에 중간에 텈! 막히기도 해서 변수에 담아서 추상화 해주는것도 읽기 좋은 코드를 만드는 작~은 방법입니다.

그리고 이 정규표현식이 어떤 조건인지 주석으로 추가 설명 해주면 더더욱 좋을것같아요!
예를들어
// @ 왼쪽은 영문 대소문자, 숫자, 특수문자(-_/.)가능, @오른쪽은 ~ 이런식으로 코드를 볼 사람을 위해서 남겨두면 이해하기 쉬울것같아요

Comment on lines +87 to +96
{...register("password", {
required: {
value: true,
message: "비밀번호를 입력해주세요.",
},
minLength: {
value: 8,
message: "비밀번호를 8자 이상 입력해주세요.",
},
})}
Copy link
Collaborator

Choose a reason for hiding this comment

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

const passwordRules = {
              required: {
                value: true,
                message: "비밀번호를 입력해주세요.",
              },
              minLength: {
                value: 8,
                message: "비밀번호를 8자 이상 입력해주세요.",
              },
            }
...
...

{...register("password", passwordRules)}

요렇게 추상화 해보는건 어떨까요?

Comment on lines +61 to +72
{...register("email", {
required: {
value: true,
message: "이메일을 입력해주세요.",
},
pattern: {
value:
/^[0-9a-zA-Z]([-_\.]?[0-9a-zA-Z])*@[0-9a-zA-Z]([-_\.]?[0-9a-zA-Z])*\.[a-zA-Z]{2,3}$/i,
message: "유효한 이메일 형식을 입력해주세요.",
},
})}
/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

login이랑 같은 코드를 쓰는 부분이 있는것같아요. 추출해서 반복사용해보는건 어떨까요??

@201steve 201steve merged commit 306bc69 into codeit-bootcamp-frontend:Next-이승현 Aug 26, 2024
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