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

Conversation

codingaring
Copy link
Collaborator

@codingaring codingaring commented May 10, 2024

요구사항

기본

  • api 요청에 TanStack React Query를 활용해 주세요.
  • 폴더 이름 변경은 ‘PUT /folders/{folderId}’를 활용해 주세요.
  • 폴더 생성은 ‘POST /folders’를 활용해 주세요.
  • 폴더 삭제는 ‘DELETE /folders/{folderId}’를 활용해 주세요.
  • 링크 삭제는 ‘DELETE /links/{linkId}’를 활용해 주세요.
  • 링크 생성은 ‘POST /links’를 활용해 주세요.
  • 로그인은 POST ‘/auth/sign-in’ 을 활용해 주세요.
  • 회원가입은 POST ‘/auth/sign-up’ 을
이메일 중복확인은 POST ‘/users/check-email’을
활용해 주세요.
  • 폴더의 정보는 GET ‘/folders/{folderId}’,
폴더 소유자의 정보는 GET ‘/users/{userId}’를 활용해 주세요.
  • 링크 공유 페이지에서 폴더의 링크 데이터는 GET ‘/folders/{folderId}/links’를 활용해 주세요.
  • 유효한 access token이 있는 경우 GET ‘/users’로 현재 로그인한 유저 정보를 받아 상단 네비게이션 유저 프로필을 보여 주세요.
  • 유효한 access token이 없는 경우 “로그인” 버튼을 보여 주세요.
  • 폴더 페이지에서 현재 유저의 폴더 목록 데이터는 GET ‘/folders’를 활용해 주세요.
  • 폴더 페이지에서 전체 링크 데이터를 받아올 때 GET ‘/links’, 특정 폴더의 링크를 받아올 때 GET ‘/folders/{folderId}/links’를 활용해 주세요.

주요 변경사항

  • 모달 리팩토링, 리팩토링
  • react-query 사용
  • data -> 어떤 data인지 구분 가능하도록 변경

멘토에게

  • useContext와 react-query를 사용하다, 아래와 같은 오류를 만났고 useEffect의 dependency list로 useQuery에서 받아온 데이터를 넣어주고 있어서 1초마다 리렌더링 되는 것 같습니다. 하지만...어떻게 해결해야하는지 떠오르지 않습니다....ㅠㅠ
    화면 캡처 2024-05-12 161634

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

Copy link

vercel bot commented May 10, 2024

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

Name Status Preview Comments Updated (UTC)
4-weekly-mission ❌ Failed (Inspect) May 12, 2024 6:52am
4-weekly-mission-ursd ❌ Failed (Inspect) May 12, 2024 6:52am

@codingaring codingaring requested a review from SeolJaeHyeok May 10, 2024 02:27
@codingaring codingaring added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성 죄송합니다.. labels May 10, 2024
@codingaring codingaring self-assigned this May 10, 2024
@codingaring codingaring changed the title [Week20 ]김유경 [김유경] Week20 May 10, 2024
Comment on lines 12 to 23
const { handleUserDataState } = useContext(UserContext);
const router = useRouter();
const [profile, setProfile] = useState({
auth_id: "",
email: "",
image_source: "",
});
const Location = useRouter();
const LocationPath = Location.pathname;
const { data: profile } = useQuery({
queryKey: ["loginUserProfile"],
queryFn: getLoginUserInfo,
});

async function handleLoadUserInfo() {
try {
const { data } = await getLoginUserInfo();
const { email, image_source, auth_id } = data[0] || [];
setProfile({
auth_id: auth_id,
email: email,
image_source: image_source,
});
handleUserDataState({ userId: auth_id });
} catch (error) {
router.push("/signin");
useEffect(() => {
if (profile) {
handleUserDataState({ isLogin: true, userId: profile.id });
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

useQuery로 받아오는 데이터가 1초마다 계속해서 받아오다보니, useContext의 데이터도 계속해서 새로 받아지는 것 같습니다. 무한 렌더링...이 되고 있는 것 같은데 이런 경우에 useQuery말고 그냥 fetch 함수를 사용하는 것이 방법일지, useQuery 말고도 다르게 개선할 수 있는지 궁금합니다 ...!

Copy link
Collaborator

Choose a reason for hiding this comment

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

음.. 1초마다 계속해서 요청을 한다는 말씀이실까요?!?

제가 확인해보니 위 이유보다는 useEffect의 의존성 배열에 handleUserDataState가 있는게 문제인거 같아요! 함수의 경우는 매 렌더링 시 새롭게 생성되어 참조값이 변하기 때문에 무한 루프에 빠지게 되는 원인이 될 수 있어요! 그래서 일반적으로는 함수에 직접 의존하는 순수한 값만 넣어주는게 좋아요!

해당 의존성 배열에서 handleUserDataState를 제거하면 문제가 사라집니다!

export const ModalDim = styled.div`
export const ModalDim = styled.button`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넘겨주는 함수의 매개변수 타입이 event : MouseEvent라서 태그를 버튼으로 바꿨는데, 그것보단 타입을 ButtonElement 말고 HTMLElement 로 바꾸는게 더 적절한 지 여쭤보려고 했으나 적으면서 생각해보니 그게 더 적절한 것 같습니다.

Comment on lines +10 to +20
const router = useRouter();
const { loginData } = useContext(UserContext);

const handleGoFolders = () => {
if (loginData.isLogin) {
router.push("/folder");
} else {
router.push("/signin");
}
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

위의 useContext 무한 렌더링 때문인지, 이 부분이 정상적으로 작동하지 않습니다. url은 바뀌지만 리다이렉트 되지 않는 문제가 있습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 앞서 말한 useEffect의 의존성에서 handleUserDataState를 제거하면 해결될 거 같아요~!

Copy link
Collaborator

@SeolJaeHyeok SeolJaeHyeok left a comment

Choose a reason for hiding this comment

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

20주차 미션 고생 많으셨습니다 유경님🎉
고민의 흔적이 코드에서 보이는거 같아서 개인적으로 좋은 경험이었어요! ㅎㅎ 고생하셨습니다:)

질문에 대한 답변은 코드 리뷰 내에 작성했으니 확인해주시면 됩니다!

const [selectFolderId, setSelectFolderId] = useState<number>();
const addToFolderMutation = useMutation({
mutationFn: ({ url, folderId }: { url: string; folderId: number }) =>
postAddToFolder({ url: url, folderId: folderId }),
Copy link
Collaborator

Choose a reason for hiding this comment

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

중요하지는 않은 부분이지만 postAddToFolder({url, folderId}) 로 축약할 수 있을거 같아요!

Comment on lines +8 to 11
export interface DeleteLinkProps extends BaseModalProps {
deleteURL: string;
linkId: number;
}
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 +15 to +18
const { data: profile } = useQuery({
queryKey: ["loginUserProfile"],
queryFn: getLoginUserInfo,
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

useQuery에 대해서는 멘토링 시간에 따로 이야기드리도록 하겠습니다!

Comment on lines 12 to 23
const { handleUserDataState } = useContext(UserContext);
const router = useRouter();
const [profile, setProfile] = useState({
auth_id: "",
email: "",
image_source: "",
});
const Location = useRouter();
const LocationPath = Location.pathname;
const { data: profile } = useQuery({
queryKey: ["loginUserProfile"],
queryFn: getLoginUserInfo,
});

async function handleLoadUserInfo() {
try {
const { data } = await getLoginUserInfo();
const { email, image_source, auth_id } = data[0] || [];
setProfile({
auth_id: auth_id,
email: email,
image_source: image_source,
});
handleUserDataState({ userId: auth_id });
} catch (error) {
router.push("/signin");
useEffect(() => {
if (profile) {
handleUserDataState({ isLogin: true, userId: profile.id });
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

음.. 1초마다 계속해서 요청을 한다는 말씀이실까요?!?

제가 확인해보니 위 이유보다는 useEffect의 의존성 배열에 handleUserDataState가 있는게 문제인거 같아요! 함수의 경우는 매 렌더링 시 새롭게 생성되어 참조값이 변하기 때문에 무한 루프에 빠지게 되는 원인이 될 수 있어요! 그래서 일반적으로는 함수에 직접 의존하는 순수한 값만 넣어주는게 좋아요!

해당 의존성 배열에서 handleUserDataState를 제거하면 문제가 사라집니다!


function toggleContents(event: MouseEvent<HTMLButtonElement>) {
event.preventDefault();
setIsOpenContents(!isOpenContents);
Copy link
Collaborator

Choose a reason for hiding this comment

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

state를 업데이트할 때는 함수형 업데이트를 사용하는게 보다 안전한 방식입니다!

Comment on lines +86 to 88
<Button onClick={handleCategoryActive} id={null} value="전체">
전체
</Button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

idnull을 넣어주신 특별한 이유가 있으실까요?!?

Comment on lines +10 to +20
const router = useRouter();
const { loginData } = useContext(UserContext);

const handleGoFolders = () => {
if (loginData.isLogin) {
router.push("/folder");
} else {
router.push("/signin");
}
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 앞서 말한 useEffect의 의존성에서 handleUserDataState를 제거하면 해결될 거 같아요~!

userId: "",
userId: 0,
Copy link
Collaborator

Choose a reason for hiding this comment

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

0으로 초기값을 넣어주신 이유가 있으실까용?
초기값이 없다면 null을 넣어주는게 좋아보여요!

Comment on lines +1 to +13
import { axiosInstance } from "./axiosInstance";

export async function deleteFolder({
folderId,
}: {
folderId: number | string;
}) {
const response = await axiosInstance.delete(
`/linkbrary/v1/folders/${folderId}`
);

return response.status;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

지금은 API Error에 대한 별도의 처리가 없는거 같아요!
axios intercepter를 보게 되면 에러가 발생했을 때, Promise.reject(error) 처리를 하고 있는데 이 에러를 처리하는 부분이 따로 안보이는거 같네용?

useMutation을 사용하시면 onError 콜백을 사용해서 에러를 처리할 수 있습니다!

++
추가적으로 API를 관리하는 방식에 대해서 드리고 싶은 말이 있는데, 이 부분은 멘토링 때 이야기 하도록 할게요!

Comment on lines +1 to +25
const ACCESS_TOKEN = "accessToken";
const REFRESH_TOKEN = "refreshToken";

export const setToken = (accessToken: string, refreshToken: string) => {
if (typeof window !== "undefined") {
localStorage.setItem(ACCESS_TOKEN, accessToken);
localStorage.setItem(REFRESH_TOKEN, refreshToken);
}
};

export const getToken = () => {
if (typeof window !== "undefined") {
const accessToken = localStorage.getItem(ACCESS_TOKEN);
const refreshToken = localStorage.getItem(REFRESH_TOKEN);

return { accessToken, refreshToken };
}
};

export const removeToken = () => {
if (typeof window !== "undefined") {
localStorage.removeItem(ACCESS_TOKEN);
localStorage.removeItem(REFRESH_TOKEN);
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

토큰 관련 로직 하나의 파일로 관리하는거 좋네요!

@SeolJaeHyeok SeolJaeHyeok merged commit cf45e50 into codeit-bootcamp-frontend:part3-김유경 May 20, 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