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

Conversation

YehaYoo
Copy link
Collaborator

@YehaYoo YehaYoo commented Jun 14, 2024

요구사항

해당 미션 피그마 링크

스프린트 미션 11


기본

  • Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.
  • 피그마 디자인에 맞게 페이지를 만들어 주세요.
  • 기존의 스프린트 미션 8에 이어서 React, Typescript를 사용합니다.
    스프린트 미션 10에 이어서 구현했습니다.(Next.js, React, Typescript 사용) 다음 주까지 마이그레이션 하겠습니다.

회원가입 페이지

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

로그인 페이지

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

메인 페이지

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

심화

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

주요 변경사항

  • 회원가입, 로그인 기능 구현

스크린샷

image image image image 스크린샷 2024-06-17 오후 7 30 53
  • 코드 리뷰, 멘토링 피드백 반영 체크리스트
    • 페이지네이션 추가, 검색 기능 수정 보완. (검색 시 api 호출 후 keyword값으로 데이터 filter)
    • getBestArticlesLimit 함수 useEffect 안에서 호출
    • base URL 환경 변수에 저장
    • 404페이지 추가

멘토에게

  • 스프린트 미션 10 코드 리뷰에 남겨주신 interceptors를 통해 미들웨어로 인가 처리하는 예제 거의 그대로 반영해보았습니다!
  • 기능이 추가되면서 재밌는 만큼 알아야 할 것도, 신경쓸 것도 많네요😭 유저기능 원리, React 사용자 기능이 다음 주까지 커리큘럼에 잡혀 있어 아직 진도를 다 나가지 못했습니다. 부족한 부분이 많을 듯 합니다. 코드리뷰 잘 참고하도록 하겠습니다! 매주 감사합니다😀

@YehaYoo YehaYoo requested a review from kiJu2 June 14, 2024 13:13
@YehaYoo YehaYoo added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jun 14, 2024
@kiJu2
Copy link
Collaborator

kiJu2 commented Jun 17, 2024

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

@kiJu2
Copy link
Collaborator

kiJu2 commented Jun 17, 2024

스프린트 미션 10 코드 리뷰에 남겨주신 리뷰 거의 그대로 반영해보았습니다!

오옷 좋습니다 ! 도움이 된 것 같아 기분이 좋네요 =) 혹시 더 피드백 드릴게 있는지 꼼꼼히 볼게요 !

기능이 추가되면서 재밌는 만큼 알아야 할 것도, 신경쓸 것도 많네요😭 유저기능 원리, React 사용자 기능이 다음 주까지 커리큘럼에 잡혀 있어 아직 진도를 다 나가지 못했습니다. 부족한 부분이 많을 듯 합니다. 코드리뷰 잘 참고하도록 하겠습니다! 매주 감사합니다😀

괜찮습니다 !! 매주 꾸준히 제출하는게 가장 중요하지요 ㅎㅎㅎ 그리고 예하 코드는 매주 성장해가는 모습이 눈에 띄게 보입니다 ! 저야말로 저의 리뷰에 대해 긍정적으로 피드백해주셔서 항상 고맙습니다 예하님. 이번에도 꼼꼼히 봐볼게요 😊😊😊

Comment on lines +11 to +12
const [isAuthenticated, setIsAuthenticated] = useState(false);
const [isLoading, setIsLoading] = useState(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

오호 로딩을 처리하셨군요? 😊

처음 로드가 할 때 깜빡이는 것 때문에 로딩처리를 하신 것 같군요 !

Copy link
Collaborator

Choose a reason for hiding this comment

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

다른 방법으로는 getServerSideProps로 서버사이드에서 인가 처리를 후 클라이언트로 내려주는 방법이 있습니다 !
이렇게 하면 리액트 앱에서 판별하지 않고(즉 깜빡이지 않고) 처음부터 유저 정보의 유무를 알고 렌더링될 수 있습니다.

Comment on lines +23 to +28
const token = localStorage.getItem("accessToken");
if (token) {
setIsAuthenticated(true);
} else {
setIsAuthenticated(false);
}
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
const token = localStorage.getItem("accessToken");
if (token) {
setIsAuthenticated(true);
} else {
setIsAuthenticated(false);
}
const token = localStorage.getItem("accessToken");
setIsAuthenticated(!!token);

조건문 안에서 true, false를 분기하여 작성할 일이 있다면 "조건문 자체를 핸들링 하는 것"도 고려해볼 수 있습니다 ! 😊

Comment on lines +23 to +28
const token = localStorage.getItem("accessToken");
if (token) {
setIsAuthenticated(true);
} else {
setIsAuthenticated(false);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

그리고 accessToken을 가진 것만으로 로그인 취급이 되어버린다면..

accessToken을 가진 것만으로 로그인이 된 것으로 되어버린다면 정상적인 인가처리로 보기 어려울 것 같아요.
실제로 백엔드에 통신을 해보고 로그인 되어있는지 확인해보지 않는다면, 브라우저에서 그냥 쿠키를 임의로 지어도 리액트 앱은 인가처리가 되는 것으로 볼 수 있을 것 같습니다 !

Comment on lines +17 to +29
const maxVisiblePages = 5;

let startPage = 1;

if (totalPageNum > maxVisiblePages) {
startPage = Math.max(activePageNum - Math.floor(maxVisiblePages / 2), 1);
startPage = Math.min(startPage, totalPageNum - maxVisiblePages + 1);
}

const pages = range(
startPage,
Math.min(startPage + maxVisiblePages, totalPageNum + 1)
);
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
const maxVisiblePages = 5;
let startPage = 1;
if (totalPageNum > maxVisiblePages) {
startPage = Math.max(activePageNum - Math.floor(maxVisiblePages / 2), 1);
startPage = Math.min(startPage, totalPageNum - maxVisiblePages + 1);
}
const pages = range(
startPage,
Math.min(startPage + maxVisiblePages, totalPageNum + 1)
);
const pages = useMemo(() => {
const maxVisiblePages = 5;
let startPage = 1;
if (totalPageNum > maxVisiblePages) {
startPage = Math.max(activePageNum - Math.floor(maxVisiblePages / 2), 1);
startPage = Math.min(startPage, totalPageNum - maxVisiblePages + 1);
}
return range(
startPage,
Math.min(startPage + maxVisiblePages, totalPageNum + 1)
);
}, [activePageNum, totalPageNum]);

메모이제이션을 통하여 불필요한 재계산을 방지할 수 있습니다만.....!!
그리고, 최적화를 한다고해서 무조건 좋지는 않아요 ! 다음 아티클은 예하님께서 흥미로워할것 같아서 첨부드립니다 😊

When to useMemo and useCallback

Comment on lines +8 to +21
function formatRelativeDate(dateString: number) {
const date = new Date(dateString);
const currentDate = new Date();
const diffTime = Math.abs(currentDate.getTime() - date.getTime());
const diffDays = Math.ceil(diffTime / (1000 * 60 * 60 * 24));

if (diffDays === 0) {
return "오늘";
} else if (diffDays === 1) {
return "어제";
} else {
return `${diffDays}일 전`;
}
}
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 +14 to +20
if (diffDays === 0) {
return "오늘";
} else if (diffDays === 1) {
return "어제";
} else {
return `${diffDays}일 전`;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

if ... else가 여러번 들어갈 때는 switch로 작성할 수 있습니다 !:

Suggested change
if (diffDays === 0) {
return "오늘";
} else if (diffDays === 1) {
return "어제";
} else {
return `${diffDays}일 전`;
}
switch (diffDays) {
case 0:
return "오늘";
case 1:
return "어제";
default:
return `${diffDays}일 전`;
}

그리고 또 다른 방법(Mapping) !!

  const dayDifferenceMap: { [key: number]: string } = {
    0: "오늘",
    1: "어제",
  };

  return dayDifferenceMap[diffDays] || `${diffDays}일 전`;

성능 차이도 있다고 하네요 😊

재미로 봐주세요 ~!

세 가지 조건 방법

Comment on lines +23 to +26
const [currentOrder, setCurrentOrder] = useState<Order>("recent");
const [currentPage, setCurrentPage] = useState<number>(1);
const [searchKeyword, setSearchKeyword] = useState<string>("");
const [totalPages, setTotalPages] = useState<number>(initialTotalPages);
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
const [currentOrder, setCurrentOrder] = useState<Order>("recent");
const [currentPage, setCurrentPage] = useState<number>(1);
const [searchKeyword, setSearchKeyword] = useState<string>("");
const [totalPages, setTotalPages] = useState<number>(initialTotalPages);
const [pagination, setPagination] = useState<PaginationState>({
currentOrder: "recent",
currentPage: 1,
searchKeyword: "",
totalPages: initialTotalPages,
});

위와 같이 연관된 상태들을 하나의 객체로 관리하면 불필요한 리렌더링을 줄이고 성능을 높일 수 있습니다 !

Comment on lines +9 to +16
if (typeof window !== "undefined") {
const accessToken = localStorage
.getItem("accessToken")
?.replace(/"/gi, "");
if (accessToken) {
config.headers.Authorization = `Bearer ${accessToken}`;
}
}
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 +35 to +48
try {
const refreshToken = localStorage.getItem("refreshToken");
if (refreshToken) {
const response = await axios.post("/auth/refresh", { refreshToken });
const newAccessToken = response.data.accessToken;
localStorage.setItem("accessToken", newAccessToken);
originalRequest.headers.Authorization = `Bearer ${newAccessToken}`;
return axios(originalRequest);
} else {
console.error("Refresh token is missing.");
}
} catch (refreshError) {
console.error("Failed to refresh access token:", refreshError);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

와우 굳굳굳 리프레시까지 멋지게 해냈군요 !!

훌륭합니다 !!

Copy link
Collaborator

Choose a reason for hiding this comment

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

다만, 지금은 여러 통신이 한 번에 들어올 경우 액세스 토큰이 401로 반려되었을 때 각자 액세스 토큰을 발급 받고 순서가 보장되지 않고 처리되겠군요 !
5개의 요청이 동시에 왔을 때 한 번의 리프레시 후(액세스 토큰 갱신 후) 요청 순서가 보장된 상태로 처리되게 할 수도 있을 것 같아요.

저는 예전에 학습할 때 다음과 같이 코드를 작성한 기억이 나네요(참고만 해주세요 😊):

this.http.interceptors.response.use(
      (response) => response,
      async (error) => {
        const originalRequest = error.config;

        if (error.response?.status === 401 && !originalRequest._retry) {
          if (this.isRefreshing) {
            return new Promise((resolve, reject) => {
              this.failedQueue.push({ resolve, reject });
            })
              .then((token) => {
                originalRequest.headers.Authorization = `Bearer ${token}`;
                return this.http(originalRequest);
              })
              .catch((err) => Promise.reject(err));
          }

          originalRequest._retry = true;
          this.isRefreshing = true;

          if (!this.refreshToken) {
            console.error('!! Refresh token is not found');
            return Promise.reject(error);
          }

          try {
            this.accessToken = await this.getAccessTokenByRefreshToken();

            // 갱신된 토큰으로 기존 큐의 요청을 다시 시도
            this.failedQueue.forEach((prom) => {
              prom.resolve(this.accessToken);
            });
            this.failedQueue = [];

            // 갱신된 토큰을 사용하여 원래 요청을 다시 시도
            originalRequest.headers.Authorization = `Bearer ${this.accessToken}`;
            return this.http(originalRequest);
          } catch (err) {
            this.failedQueue.forEach((prom) => {
              prom.reject(err);
            });
            this.failedQueue = [];
            return Promise.reject(err);
          } finally {
            this.isRefreshing = false;
          }
        }

        return Promise.reject(error);
      },
    );

Comment on lines +33 to +39
<h1 className={styles.featureTitle}>
인기 상품을{" "}
<span className={styles.breakOnDesktop}>
<br />
</span>
확인해 보세요
</h1>
Copy link
Collaborator

Choose a reason for hiding this comment

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

<h1> 태그는 문서에 하나만 넣는게 어떨까요?

페이지 당 하나의 <h1>만 사용하세요. 여러 개를 써도 오류는 나지 않겠지만, 단일 <h1>이 모범 사례로 꼽힙니다. 논리적으로 생각했을 때도, <h1>은 가장 중요한 제목이므로 전체 페이지의 목적을 설명해야 할 것입니다. 두 개의 제목을 가진 책이나, 여러 개의 이름을 가진 영화는 볼 수 없죠! 또한 스크린 리더 사용자와 SEO에도 더 적합합니다.

h1에 대해서: MDN

@kiJu2
Copy link
Collaborator

kiJu2 commented Jun 17, 2024

크으.. 정말 정말 재밌게 봤습니다 예하님 !!
그리고 정말 열심히 하시는군요... 멘토로서 너무 뿌듯하게 지켜보고 있습니다 🥺🥺🥺🥺
팀 프로젝트 때에도 빛을 발하실 것 같아요 ! 여기 팀은 참 기대가 많이 됩니다 😊

리뷰 중 궁금하신거 있으시면 사전 질의를 통해서 남겨주시거나 멘토링 미팅 때 질문주세요 ㅎㅎㅎ

@kiJu2 kiJu2 merged commit 33689bf 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