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 #693

Conversation

PixeIDark
Copy link
Collaborator

@PixeIDark PixeIDark commented Jun 14, 2024

요구사항

기본

회원가입

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

로그인

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

메인

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

심화

  • 로그인, 회원가입 기능에 react-hook-form을 활용해봅니다.

주요 변경사항

  • https://neview11.netlify.app/
  • interceptor에 401에러시 토큰을 업데이트하는 로직을 추가하고, AuthContext로 로그인과 유저를 관리했습니다.
  • 스프린트 미션10에서는 댓글작성이 정상적으로 작동했는데, 미션11에서는 댓글이 10%의 확률로 작성성공 됩니다.. 원인을 모르겠지만 최대한 찾아보겠습니다.

스크린샷

image

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@PixeIDark PixeIDark requested a review from kiJu2 June 14, 2024 13:09
@PixeIDark PixeIDark self-assigned this Jun 14, 2024
@PixeIDark PixeIDark added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jun 14, 2024
@PixeIDark PixeIDark changed the base branch from main to Next.js-김민재 June 14, 2024 15:26
@kiJu2
Copy link
Collaborator

kiJu2 commented Jun 17, 2024

수고 하셨습니다 ! 스프리트 미션 하시느라 정말 수고 많으셨어요.
학습에 도움 되실 수 있게 꼼꼼히 리뷰 하도록 해보겠습니다.

@kiJu2
Copy link
Collaborator

kiJu2 commented Jun 17, 2024

스프린트 미션10에서는 댓글작성이 정상적으로 작동했는데, 미션11에서는 댓글이 10%의 확률로 작성성공 됩니다.. 원인을 모르겠지만 최대한 찾아보겠습니다.

...?! 메이플 주문서인가요..? 혹시 어떤 에러가 발생하나요 ..?

@kiJu2
Copy link
Collaborator

kiJu2 commented Jun 17, 2024

interceptor에 401에러시 토큰을 업데이트하는 로직을 추가하고, AuthContext로 로그인과 유저를 관리했습니다.

크으 ~! 좋습니다. 확인해보져

@@ -0,0 +1 @@
BASE_URL = https://panda-market-api.vercel.app
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 파일은 프로젝트와 무관하므로 .gitignore에 다음과 같이 작성하시면 git의 추적을 무시할 수 있습니다.

.env

Copy link
Collaborator

Choose a reason for hiding this comment

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

.gitignore에 env 파일을 추가해 주셨군요!

하지만 아직 github에서 확인되는 걸 보니 캐시를 지워줘야 할 것으로 보입니다 :)

  git rm .env --chached

gitignore에 .env 파일을 추가해도 commit history에 env파일이 남아있게 됩니다!

git에 env파일을 완전히 지우려면 추가 작업이 필요해요 😂
아래 블로그를 참고해 보시면 좋을 것 같아요!

[Git] 실수로 올린 env 삭제하기 (commit history까지 완전 삭제)

@@ -13,6 +13,8 @@
/.next/
/out/

/.env
Copy link
Collaborator

Choose a reason for hiding this comment

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

어라? .gitignore에 env 파일을 추가해 주셨군요!

하지만 아직 github에서 확인되는 걸 보니 캐시를 지워줘야 할 것으로 보입니다 :)

  git rm .env --chached

gitignore에 .env 파일을 추가해도 commit history에 env파일이 남아있게 됩니다!

git에 env파일을 완전히 지우려면 추가 작업이 필요해요 😂
아래 블로그를 참고해 보시면 좋을 것 같아요!

[Git] 실수로 올린 env 삭제하기 (commit history까지 완전 삭제)

},
});
return data;
await axiosInstance.post<any>("/articles", payload);
Copy link
Collaborator

Choose a reason for hiding this comment

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

우리 any를 하나하나 지워보도록 합시다 ~!

Suggested change
await axiosInstance.post<any>("/articles", payload);
await axiosInstance.post<PostArticleResponse>("/articles", payload);

혹은

Suggested change
await axiosInstance.post<any>("/articles", payload);
await axiosInstance.post<{success: boolean, id: string 등등..}>("/articles", payload);

Copy link
Collaborator

Choose a reason for hiding this comment

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

/**
 * @todo 나중에 고쳐야됨
*/

const PostRefreshToken = async (
refreshToken: string | null
): Promise<string> => {
const PostRefreshToken = async (refreshToken?: string): Promise<Token> => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

컴포넌트가 아닌 ts혹은 js 함수는 일반적으로 카멜케이스로 작성합니다:

Suggested change
const PostRefreshToken = async (refreshToken?: string): Promise<Token> => {
const postRefreshToken = async (refreshToken?: string): Promise<Token> => {

const axiosConfig: AxiosRequestConfig = {
baseURL: "https://panda-market-api.vercel.app/",
baseURL: `${process.env.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.

굳굳굳 ~! 환경 변수를 적용하셨군요 😊

Comment on lines +43 to +53
<label className={labelBasicStyle}>
*제목
<input
name="title"
type="text"
value={postData.title}
onChange={onChangeInput}
className={inputRecipe()}
placeholder="제목을 입력해주세요"
/>
</label>
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳 ~! labelinput을 연결했군요 ! 👍


function Header() {
const router = useRouter();
const [isToken, setIsToken] = useState<string | undefined>("");
Copy link
Collaborator

Choose a reason for hiding this comment

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

다음과 같이 처리하면 string | undefined가 됩니다 !

Suggested change
const [isToken, setIsToken] = useState<string | undefined>("");
const [isToken, setIsToken] = useState<string>();

Copy link
Collaborator

Choose a reason for hiding this comment

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

그리고 토큰이 빈 값인 경우는 "없음"이란 의미로 보기 어려울 것 같아요 ! 차라리 undefined로 들어가있으면 어떨까요?

Comment on lines +37 to +39
const response = await postSignin(userData);
saveTokenToLocalStorage(response);
setUser(response.user as User);
Copy link
Collaborator

Choose a reason for hiding this comment

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

API 함수에User 반환타입을 지정하는게 어떨까요?

Suggested change
const response = await postSignin(userData);
saveTokenToLocalStorage(response);
setUser(response.user as User);
const response = await postSignin(userData); // 여기서 User 타입을 반환
saveTokenToLocalStorage(response);
setUser(response.user);

흔히 타입스크립트에는 두 가지 금기사항이 있습니다 🤣:

  1. any를 사용하지 말 것.
  2. 타입 어설션(as)을 사용하지 말 것.

createAccount: async () => {},
});

function AuthProvider({ children }: AuthProviderProps) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳 ! Context로 유저 정보를 관리하고 있군요 👍👍👍

이렇게 하면 드릴링 없이, 어디서든 상태를 참조할 수 있겠어요 😊

Copy link
Collaborator

Choose a reason for hiding this comment

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

혹시나.. 도움되실 수도 있을 것 같아서요.. ☺️

다음 코드는 이 전에 강의를 진행하며 작성했던 AuthContext 예시 코드입니다 😊:

import React, { createContext, useState } from "react";

type User = {
  accessToken: string;
};

export const AuthContext = createContext<{
  user: User | null;
  login: (accessToken: string) => void;
  logout: () => void;
}>({
  user: null,
  login: (accessToken: string) => {},
  logout: () => {},
});

export function AuthProvider({ children }: { children: React.ReactNode }) {
  const [user, setUser] = useState<User | null>(null);

  const login = (accessToken: string) => {
    localStorage.setItem("accessToken", "123456");
    document.cookie = `accessToken=${localStorage.getItem("accessToken")}`;
    setUser({ accessToken });
  };
  const logout = () => setUser(null);

  return (
    <AuthContext.Provider value={{ user, login, logout }}>
      {children}
    </AuthContext.Provider>
  );
}

export const useAuth = () => React.useContext(AuthContext);

참고만 해주세요 =)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

감사합니다! 참고해봤더니 코드가 더 깔끔해졌습니다.

Comment on lines +74 to +79
if (
(router.pathname === "/signin" || router.pathname === "/signup") &&
accessToken
) {
router.push("/");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 되면 잠깐의 깜빡임이 있겠군요 🤔

리액트 앱이 띄워지고 로컬 스토리지에서 확인하고 pathname을 확인한 후에 리다이렉트를 시키면 깜빡임이 있겠군요.
추 후 쿠키로 관리하고 서버사이드에서 클라이언트가 요청했을 때 redirect처리를 해준다면 많은 비용을 아낄 수 있을거예요.

Copy link
Collaborator

Choose a reason for hiding this comment

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

const notAllowed = (router.pathname === "/signin" || router.pathname === "/signup") && accessToken;
if (notAllowed) {

@kiJu2
Copy link
Collaborator

kiJu2 commented Jun 17, 2024

정말 수고 많으셨습니다 민재님 ~!!
훌륭합니다. 모든 요구사항을 멋지게 해내셨군요 !! 👏👏👏

항상 꾸준히 스프린트 미션 제출하고, 멋지게 해내는 모습. 멋있습니다.
리뷰 중 궁금하신거 있으시면 사전 질의를 통해서 남겨주시거나 멘토링 미팅 때 질문주세요 ㅎㅎㅎ

@kiJu2 kiJu2 merged commit 31b8e54 into codeit-bootcamp-frontend:Next.js-김민재 Jun 17, 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