-
Notifications
You must be signed in to change notification settings - Fork 44
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
[박상준] Week19 #495
The head ref may contain hidden characters: "part3-\uBC15\uC0C1\uC900-week15"
[박상준] Week19 #495
Conversation
fix: react-hook-form정상적용
Feature query
refactor: 폴더 리스트 페이지 전체 로직 리팩토링
Feature: 검색기능 리팩토링, 링크 리스트, 로그인 리액트 쿼리 적용
Feature query
Feature query
Feature query
Feature query
상준님~ 먼저 질문 사항에 대한 답 드릴게요
멘토링에서는 빠르게 요약하기 위해 캐싱 레이어에 대해서만 이야기를 했는데요. 우선 React Query를 왜 사용해왔는지 먼저 고민해보면 좋겠어요. React Query는 화면 상 존재하는 데이터를 관리하기 위해 활용되는 라이브러리에요. 그런만큼, 복잡한 클라이언트 측 데이터 요청을 관리해주고, query key 기반으로 데이터를 캐싱하고, 조건에 따라 client side data를 update해주는 등 기능을 제공해 줍니다. NextJS는 App router를 제공하며 framework를 제대로 사용하기 위해 client side와 server side를 어던 경우 사용해야 하는지를 명시했는데요. https://nextjs.org/docs/app/building-your-application/rendering/composition-patterns 여기서 NextJS가 제공하는 기능과 React Query가 제공하는 기능 중 겹치는 부분들이 발생합니다.
가볍게 정리해봐도 상당히 많은 부분들이 공통으로 겹쳐지는걸 확인할 수 있죠? 위 관점에서 명확하게 React Query가 사용되어야 할 이유가 없다면 굳이 NextJS 프로젝트에 React Query를 사용할 필요가 없지 않을까요? |
feat: nextAuth 적용, 미들웨어 적용, 로그인 로직 리액트 쿼리 삭제
Refactor approuter
주말에 nextjs 튜토리얼을 따라서 해보고 직접 적용해보려고 일단 위클리 미션에 적용했습니다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
상준님, 프로젝트 리팩토링 하느라 고생 많으셨습니다!
app router를 바로 적용하기가 쉽지 않으시죠 ㅎㅎ
그래도 데이터 페칭 부분을 어느정도 서버단으로 옮겨오신 만큼, 천천히 공식문서 참고하며 해보면 server component와 client component의 조합, 그리고 server side fetching, loading, error handling등 금방 익숙해지시리라 생각합니다.
전반적으로 콤포넌트 복잡도가 높은 편인데요, 이전 멘토링 때 이야기 했던 로직의 분리, 관심사의 분리를 한번 잘 생각해보며 리팩토링 해보면 좋겠어요.
또한 콤포넌트와 함수, 그리고 변수들의 이름들은 조금 더 직관적으로 사용되어도 좋겠습니다.
자세한 내용은 코드단 리뷰에 남겨두었으니 참고 부탁드려요.
고생 많으셨습니다!
@@ -52,9 +30,9 @@ export async function getSampleFolder() { | |||
} | |||
} | |||
|
|||
export async function getFolder(id: string) { | |||
export async function getFolder() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getFolder
등 UI에 필요한 불러오는 서비스 레이어에서 각 함수들이 어떤 값을 반환하는지 타입을 설정해주면 UI에서 사용하기 편하지 않을까요?
또한, 에러핸들링의 경우, 던져진 오류를 catch로 잡아주었는데, 굳이 다시 던져주는 이유가 있을까요~?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
api 함수는 초기에 작성하고 리펙토링을 따로 진행하지 않아 다른 부분에서 코드를 인용해서 사용하고 그대로 뒀습니다. 그래서 따로 에러를 다시 던지는 이유는 없습니다.
let query; | ||
if (folderId) { | ||
try { | ||
const query = `/${id}/links?folderId=${folderId}`; | ||
const { data } = await axios.get(`/users${query}`); | ||
return data.data; | ||
} catch (error) { | ||
console.error('Error fetching folderList:', error); | ||
throw error; | ||
} | ||
} else if (!folderId) { | ||
try { | ||
const query = `/${id}/links`; | ||
const { data } = await axios.get(`/users${query}`); | ||
return data.data; | ||
} catch (error) { | ||
console.error('Error fetching folderList:', error); | ||
throw error; | ||
} | ||
query = `/folders/${folderId}/links`; | ||
} else { | ||
query = '/links'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
쿼리는 무언가를 조회하기 위한 조건을 이야기합니다.
그렇다면, 네트워크 요청이 실제 발생하는 요소는 뭐라고 하는게 좋을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
url이라는 이름으로 사용해도 될까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
url은 전체 주소를 주로 이야기를 하죠, 이 경우는 endpoint라는 변수로 만들어 사용해주면 어떨까 싶습니다
const result = await axios.get(`${query}`); | ||
return result; | ||
} | ||
|
||
export async function getUser(accessToken: string) { | ||
try { | ||
const { data } = await axios.get('/users', { | ||
headers: { | ||
Authorization: accessToken, | ||
}, | ||
}); | ||
return data.data; | ||
} catch (error) { | ||
console.error('Error fetching user:', error); | ||
throw error; | ||
} | ||
export async function getUser() { | ||
const { data } = await axios.get('/users'); | ||
return data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요 녀석들도 적합한 에러 핸들링이 필요해요
api/api.ts
Outdated
email: id, | ||
password: password, | ||
}); | ||
localStorage.setItem('token', data.accessToken); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accessToken은 로그인한 사용자에 대한 중요한 정보를 담고있어요.
누군가가 이 정보를 해킹한다면 바로 사용자 계정으로 접근해 핵심 정보를 추출해올 수 있겠죠.
그렇게 중요한 데이터를 localStorage에 관리하는건 좋ㅈ ㅣ않아 보이는데요, 어떻게 개선해볼 수 있을지 고민해볼까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
우선 nextauth를 사용해서 세션에 저장하도록 변경했습니다.
if (data.favorite) { | ||
await putLinkLike(data.linkId, false); | ||
} else { | ||
await putLinkLike(data.linkId, true); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onMutate: async (data: { linkId: string; favorite: boolean }) => { | ||
if (folderId) { | ||
const result = queryClient.getQueryData<any>(["links", folderId]); | ||
const refetchLink = { ...result.data[index], favorite: !data.favorite }; | ||
let refetchLinkArr = [...result.data]; | ||
refetchLinkArr[index] = refetchLink; | ||
queryClient.setQueryData(["links", folderId], (prev: any) => { | ||
return { | ||
...prev, | ||
data: refetchLinkArr, | ||
}; | ||
}); | ||
} else { | ||
const result = queryClient.getQueryData<any>(["links"]); | ||
const refetchLink = { ...result.data[index], favorite: !data.favorite }; | ||
let refetchLinkArr = [...result.data]; | ||
refetchLinkArr[index] = refetchLink; | ||
queryClient.setQueryData(["links"], (prev: any) => { | ||
return { | ||
...prev, | ||
data: refetchLinkArr, | ||
}; | ||
}); | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이부분은 로직이 많이 복잡하네요, 콤포넌트가 전달받는 prop을 개선시킴으로 훨씬 간편하게 해볼 수 있겠는데요.
한번 콤포넌트 설계를 다시해볼 시간이 있다면 도전해보아도 좋겠어요!
fill | ||
/> | ||
</S.StarIcon> | ||
{isActive && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isActive라는 이름보다는 showFavoriteButton을 프롭값으로 해주는건 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음 아래를 보니, 단순 showFavoriteButton로 해결되는게 아니라, 유저가 로그인 되어있는지에 따라 보이는 값이 달라지는 듯 싶네요. 그럼 showFavoriteButton보단, showButtons이 더 좋지 않을까 싶네요
@@ -14,45 +15,48 @@ export type LinkData = { | |||
|
|||
export interface Links extends Array<LinkData> {} | |||
|
|||
function useGetFolder(id: string, searchKeyword: string, folderId: string) { | |||
function useGetFolder(deBounceValue: string, folderId: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런식으로 훅을 만들게 되면 앞으로 존재하는 모든 http method에 따라 훅이 다 만들어지게 될 것 같은데요.
useFolder
라는 훅 하나를 만들어 get all, create, update, delete 메소드를 제공해주도록 일원화 시키면 어떨까요?
function useGetFolderList(userId: string, folderId?: string) { | ||
const [link, setLink] = useState<Folders>([]); | ||
const [linkLoading, setLinkLoading] = useState(false); | ||
function useGetFolderList() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
배열을 반환하는 경우라면 복수형을 사용해주세요!
useFolders
요구사항
기본
주요 변경사항
멘토에게