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

[이승현] Sprint10 #643

Conversation

codefug
Copy link
Collaborator

@codefug codefug commented Jun 7, 2024

요구사항

기본

  • 상품 등록 페이지
  • 상품 상세 페이지
  • []

심화

  • []
  • []

주요 변경사항

  • React-Hook-Form으로 렌더링 최적화하려고 시도중입니다..
  • 데이터를 받아오는 부분은 상품 상세 페이지의 경우 SSR로 할 예정입니다.
  • 인피니티 스크롤을 기존 페이지에 더해볼까 고민중이지만 우선순위를 맨 끝으로 놓을 것 같습니다

스크린샷

멘토에게

  1. 많이 하지 못했습니다.. 최근에 번아웃처럼 조금 와서 시간을 많이 버렸네요. 최대한 주말 이용해서 완성해보겠습니다.

  2. 기존 라이브러리 학습을 모두 문서 통해서 했었는데 react-hook-form은 진입 장벽이 조금 있는 느낌이네요.. 멘토님께서는 어떤 방식으로 학습하시는지 궁금합니다.

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

@codefug codefug added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Jun 7, 2024
@codefug
Copy link
Collaborator Author

codefug commented Jun 9, 2024

document.cookie에 accessToken을 담고 꺼내는 로직을 넣었는데 hydration 때문에 useState(false)가 기본값인 탓에 첫 로딩 때 한번, useEffect로 한번 이렇게 두번 렌더링되는 문제가 있습니다. 버튼이 잠깐 보였다가 사라져서 UX적으로 별로입니다 어떻게 해결해야 할까요?

로그인 화면에서 로그인이 완료된 후에 header 버튼이 바뀌게 하고 싶은데 router.push로 이동하게 되면 header쪽에 리렌더링이 안되서 조건에 만족해도 버튼이 바뀌지 않습니다. reload를 발생시키기에는 조금 아닌거 같고 조언 부탁드립니다!

api파일의 postImage를 이용해서 서버에 이미지를 올렸는데 internal server error가 떠서 게시글에 image 올리는건 테스트하지 못했습니다. 다른 사람들은 되는거 보니깐 제 코드 문제인 것 같은데 blob으로 만든 이미지 url을 postImage로 옮기면 formData > append("image",imageUrl) > multipart/form-data 형식으로 post 요청 이 로직이 아닌걸까요??... 조언 부탁드립니다..

하면 할수록 할 건 많고 부족한건 하나씩 보이네요.. 그래도 열심히 해보겠습니다.

@codefug codefug requested a review from kiJu2 June 11, 2024 03:14
@codefug codefug removed the 미완성🫠 죄송합니다.. label Jun 11, 2024
@kiJu2
Copy link
Collaborator

kiJu2 commented Jun 11, 2024

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

@kiJu2
Copy link
Collaborator

kiJu2 commented Jun 11, 2024

많이 하지 못했습니다.. 최근에 번아웃처럼 조금 와서 시간을 많이 버렸네요. 최대한 주말 이용해서 완성해보겠습니다.

하핳.. 너무 열심히 달리다보면 번아웃이 올 수도 있죠. 잘 하고 계시니까 조금은 쉬엄쉬엄 하셔도 괜찮습니다 !

기존 라이브러리 학습을 모두 문서 통해서 했었는데 react-hook-form은 진입 장벽이 조금 있는 느낌이네요.. 멘토님께서는 어떤 방식으로 학습하시는지 궁금합니다.

리액트 쿼리나 NextJS 같은 경우는 생태를 이해해야하는 도구이다 보니, 시간 내서 공식 문서를 하나씩 다 읽어보는 편입니다 !
tailwind와 같이 비교적 간단한 것은 직접 해보면서 학습합니다
리액트 훅 폼은 잘 안써봐서.. ㅎㅎㅎ 사용할 때는 가끔 찾아보고 적용하는 편이예요 !

@kiJu2
Copy link
Collaborator

kiJu2 commented Jun 11, 2024

document.cookie에 accessToken을 담고 꺼내는 로직을 넣었는데 hydration 때문에 useState(false)가 기본값인 탓에 첫 로딩 때 한번, useEffect로 한번 이렇게 두번 렌더링되는 문제가 있습니다. 버튼이 잠깐 보였다가 사라져서 UX적으로 별로입니다 어떻게 해결해야 할까요?

움 브라우저가 쿠키에서 꺼내와야 하는 과정에서 앱이 한 번 렌더링이 된 후 다시 리렌더링 되어 로그인 상태가 되어 필연적으로 한 번 깜빡 거린다는거지요? getServerSideProps로 유저 데이터를 초기에 전달하면 해결할 수 있을 것 같아요 !

@kiJu2
Copy link
Collaborator

kiJu2 commented Jun 11, 2024

로그인 화면에서 로그인이 완료된 후에 header 버튼이 바뀌게 하고 싶은데 router.push로 이동하게 되면 header쪽에 리렌더링이 안되서 조건에 만족해도 버튼이 바뀌지 않습니다. reload를 발생시키기에는 조금 아닌거 같고 조언 부탁드립니다!

AuthContext와 같이 루트에 컨텍스트를 감싸놓으면 어떨까요?

다음 코드를 확인해보시겠어요?:

tl;dr

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);

nextjs-instructional-materials

Comment on lines +15 to +22
export function BestPostCard({
title,
image,
nickname,
likeCount,
createdAt,
id,
}: Props) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

오호 해당 컴포넌트가 사용하는 Props� 들로만 이루어졌군요 !

Copy link
Collaborator

Choose a reason for hiding this comment

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

오호 shadcn/ui 인가욤? 후기가 궁금하네요 ☺️

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

러닝커브 하나도 없이 적용하면 됐었습니다. shadcn자체에서 무언가를 하기 보다는 시니어 개발자분이 좋은 라이브러리를 활용해서 만든 더 쉬운 라이브러리를 활용하는 기분이었어요! 잘할려면 더 봐야되긴 하겠지만 사용하기 편한것 같습니다 😊

Comment on lines +22 to +39
<DropdownMenu>
<DropdownMenuTrigger>
<Image
width={24}
height={24}
src={"/images/ic_kebab.png"}
alt="edit button"
/>
</DropdownMenuTrigger>
<DropdownMenuContent align="end" className="rounded-lg">
<DropdownMenuItem className="cursor-pointer px-5 py-3">
수정
</DropdownMenuItem>
<DropdownMenuItem className="cursor-pointer px-5 py-3">
삭제
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
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 Author

Choose a reason for hiding this comment

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

(●'◡'●)

Comment on lines +6 to +15
export default function LoginPage() {
const theme = useContext(LoginContext);
const handleClick = () => {
(async () => {
const token = await getToken();
setCookie("accessToken", token.accessToken, { "max-age": 3600 });
setCookie("refreshToken", token.refreshToken);
theme?.setIsLogin(true);
})();
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

음. 지금 보니까 Context를 잘 사용하고 계신 것 같은데, router로 페이지 이동을 해도 깜빡임이 발생하실까요?

Copy link
Collaborator Author

@codefug codefug Jun 12, 2024

Choose a reason for hiding this comment

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

깜빡임이라기 보다는 로그인 후에 새로고침을 누르면 로그인 버튼에서 프로필사진으로 바뀌는게 잠깐 보여요 document.cookie에만 담아놔서 SSR에서는 가져올 수가 없어요

지금 구현한 로그인 로직이

로그인 (refresh token, access token을 cookie에 담음) > 컴포넌트가 access token이나 refresh token을 감지하면 변경 > access token이 없을 경우 api를 호출하는 함수에서 refresh token을 이용해서 access token을 받고 api 호출

이런 로직인데 컴포넌트에서 token을 감지해야 변경되다 보니 이 감지 되는 시점이전에 발생하는 것 같아요

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 +44 to +46
const response = await fetch(
`${BASE_URL}/articles/${router.query.id}/comments?limit=10${cursor !== null ? `&&cursor=${cursor}` : ""}`,
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

다음과 get 메서드는 객체로 다룰 수도 있습니다 !

Suggested change
const response = await fetch(
`${BASE_URL}/articles/${router.query.id}/comments?limit=10${cursor !== null ? `&&cursor=${cursor}` : ""}`,
);
const query = new URLSearchParams({ limit: '10', ...(cursor !== null && { cursor }) }).toString();
const response = await fetch(
`${BASE_URL}/articles/${router.query.id}/comments?${query}`,
);

URLSearchParams: URLSearchParams 인터페이스는 URL의 쿼리 문자열을 대상으로 작업할 수 있는 유틸리티 메서드를 정의합니다.

쿼리를 생성하실 때에 참고해서 사용해보세요 😊

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 +24 to +29
if (typeof id !== "string") {
return { notFound: true };
}

const articles = await getArticleWithId(id as unknown as string);
const comments = await getCommentWithId(id as unknown as string, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이미 idstring이 아닐 수가 없습니다 !

Suggested change
if (typeof id !== "string") {
return { notFound: true };
}
const articles = await getArticleWithId(id as unknown as string);
const comments = await getCommentWithId(id as unknown as string, 0);
if (typeof id !== "string") {
return { notFound: true };
}
const articles = await getArticleWithId(id);
const comments = await getCommentWithId(id, 0);

위와 같이 타입 어설션을 지우셔도 될 듯 합니다. 😊

} = useForm<IFormInput>();
const router = useRouter();
const onSubmit: SubmitHandler<IFormInput> = (d) => {
postArticle(image !== null ? { ...d, image } : { ...d });
Copy link
Collaborator

Choose a reason for hiding this comment

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

try ... catch를 사용해볼까요?

Suggested change
postArticle(image !== null ? { ...d, image } : { ...d });
const onSubmit: SubmitHandler<IFormInput> = async (d) => {
try {
await postArticle(image !== null ? { ...d, image } : { ...d });
router.push("/boards");
} catch (error) {
console.error("Failed to post article", error);
alert(error.message);
}
};

<div className="relative h-12 w-12">
<Image fill src="/icons/plusButton.png" alt="plusButton" />
</div>
<p>이미지 등록</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

구문같아 보이지는 않습니다 !

차라리 divspan과 같은 태그는 어떨까요 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

p랑 span이랑 헷갈렸네요 🤣

Comment on lines +37 to +41
const query = new URLSearchParams({
orderBy,
keyword,
pageSize: pageSize.toString(),
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳굳 !! 👍

keyword,
pageSize: pageSize.toString(),
});
fetch(`${BASE_URL}/articles?${query.toString()}`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

보통 api 함수들은 따로 빼놓고 사용하는게 일반적입니다 ! 😊

프로젝트에서는 API 호출 로직을 별도의 파일로 분리하여 관리하는 것이 일반적이예요. 이렇게 하면 컴포넌트, 페이지, 훅 등 어디든 사용될 수 있겠죠?😊
코드의 재사용성을 높이고, 유지보수가 쉬워질 수 있습니다 ! API 함수를 모듈화하여 사용하면 코드가 더 깔끔하고 읽기 쉬워집니다. 다음은 프로젝트의 디렉토리 구조와 API 함수 예제입니다:

// src/services/apis/user.api.ts (예시입니다 !)
export const getUserInfo = async () => {
	try {
		const { data } = await axios.get('/sample/user');
		return data;
	} catch(error) {
		throw error;
	}
};

@kiJu2
Copy link
Collaborator

kiJu2 commented Jun 11, 2024

굳굳 ! 승현님 수고 많으셨습니다.
API 함수를 따로 분류해보시고 axios도 한 번 고려해보시면 어떨까 싶어요 ㅎㅎㅎ
빠르게 발전해나가고 탐구력이 좋으셔서 금방 적용하실 것 같습니다 ! 😊

@kiJu2 kiJu2 merged commit 10edc79 into codeit-bootcamp-frontend:Next.js-이승현 Jun 11, 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