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

[김유경] Week21 #1090

Open
wants to merge 65 commits into
base: part3-김유경
Choose a base branch
from

Conversation

codingaring
Copy link
Collaborator

@codingaring codingaring commented May 17, 2024

요구사항

기본

  • 프로젝트 전반에 필요한 리팩토링 기능 개선을 진행해주세요.
  • 즐겨찾기 설정된 카드의 별은 파랑색이 됩니다. 파랑색 별을 다시 클릭하면 즐겨찾기 설정이 해제되고 회색으로 돌아갑니다.
  • 즐겨찾기 설정/해제는 PUT ‘/links/{linkId}’ 를 활용해 주세요.

심화

  • 드래그 앤 드롭 기능을 만들어주세요.

주요 변경사항

  • 지난 주차 코드 리뷰 최대한 반영해보았습니다!

코드리뷰 이후 추가

  • authAPI로 api 함수 객체화
  • accessToken의 유무로 리다이렉트 시켰더니, 유효하지 않은 경우에도 넘어가서 수정

멘토에게

  • 멘토링 시간에 다루었던 추상화를 해보려고 합니다..! 아직 전부 반영하지 못했지만, 몇 가지 url이 같은데 get post put delete 하는 API의 경우 하나의 객체로 한 번 더 모아서 사용할까 하는데 괜찮은 걸까요?

codingaring added 30 commits May 9, 2024 14:39
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.

고생하셨어요 유경님! 리뷰가 너무 늦어버렸네요ㅠㅠ 지난번 보다 코드가 더 깔끔해진거 같아용!


멘토링 시간에 다루었던 추상화를 해보려고 합니다..! 아직 전부 반영하지 못했지만, 몇 가지 url이 같은데 get post put delete 하는 API의 경우 하나의 객체로 한 번 더 모아서 사용할까 하는데 괜찮은 걸까요?

넵 객체로 모으는 기준이 명확하면 괜찮을거 같습니다!

도메인별(Ex. 인증, 유저 등)으로 응집시켜서 관리해도 될거 같고, 화면 단위로 응집시켜서 관리해도 될거 같고 방법은 여러가지가 있을거 같은데 명확한 기준만 세워주세요:)

Comment on lines +16 to +20
onSuccess: () => {
queryClient.invalidateQueries({
queryKey: ["folderList"],
});
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Query Key 관리하는건 tkdodo 형님의 이 글이 바이블처럼 여겨지는거 같아서 읽어보시고 적용해보시는걸 추천드립니다!

components/common/Modals/AddToFolder/AddToFolder.tsx Outdated Show resolved Hide resolved
data-access/checkSignin.ts Outdated Show resolved Hide resolved
Comment on lines +1 to +71
import axios, { AxiosResponse } from "axios";
import { axiosInstance } from "./axios/axiosInstance";

export function createHttpClient() {
async function get<R>(url: string): Promise<R> {
try {
const response: AxiosResponse<R> = await axiosInstance.get(url);
return response.data;
} catch (error) {
if (axios.isAxiosError(error)) {
throw new Error(error.message);
} else {
throw new Error("데이터를 불러오는데 실패했습니다.");
}
}
}

async function del<R>(url: string): Promise<R> {
try {
const response: AxiosResponse<R> = await axiosInstance.delete(url);
return response.data;
} catch (error) {
if (axios.isAxiosError(error)) {
throw new Error(error.message);
} else {
throw new Error("데이터를 삭제하는데 실패했습니다.");
}
}
}

async function put<T, R>(data: T, url: string): Promise<R> {
try {
const response: AxiosResponse<R> = await axiosInstance.put(url, data);
return response.data;
} catch (error) {
if (axios.isAxiosError(error)) {
throw new Error(error.message);
} else {
throw new Error("데이터를 저장하는데 실패했습니다.");
}
}
}

async function post<T, R>(
data: T,
url: string,
headers?: string
): Promise<R> {
try {
const response: AxiosResponse<R> = await axiosInstance.post(url, data, {
headers: {
"Content-Type": headers ? headers : "application/json",
},
});
return response.data;
} catch (error) {
if (axios.isAxiosError(error)) {
throw new Error(error.message);
} else {
throw new Error("데이터를 불러오는데 실패했습니다.");
}
}
}

return {
get,
post,
put,
del,
};
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

너무 좋습니다!!

하나 더 말씀드리면 지금처럼 throw new Error()를 통해 에러를 던질때는 string 값만 던질 수 있는데, 객체나 특정한 값을 에러에 담아서 보내려면 어떻게 해야할까요?!?

util/handleToken.ts Outdated Show resolved Hide resolved
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