-
Notifications
You must be signed in to change notification settings - Fork 3
♻️ Refactor(#237): queryCache를 이용한 리다이렉트 개선 #241
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
Conversation
src/components/Redirect/index.tsx
Outdated
| }, [currentPath, router.query.id]); | ||
|
|
||
| return children; | ||
| return <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>; |
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.
useModal 훅을 사용하기 위해서 QueryClientProvider가 Redirect 안으로 이동했습니다. (App에서는 사용불가)
App에서 QueryClientProvider를 바로 확인할 수 없어서 헷갈릴 수 있을 것 같습니다.
개선방안 있으면 말씀해주세요!
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.
프로바이더를 내부에 넣은 건 어쩔 수 없을 거 같아서 괜찮을 거 같습니다 헷갈리진 않을거 같아요 🫡
|
|
||
| useEffect(() => { | ||
| const handleRedirect = async () => { | ||
| const handleCheckMember = async () => { |
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.
멤버인지 체크하는 원리는, 멤버만 할 수 있는 요청을 보내고 catch에서 실패할 경우 멤버가 아니라고 판단하는 것입니다.
이게 여전히 잘 동작하는 걸로 봐서, Query Cache의 onError는 모든 재요청 후, 실패에 대한 try catch까지 거친 뒤에도 핸들링되지 않은 요청을 받는 것 같습니다.
retry가 있는 경우 리다이렉션이 느려져서 실패한 경우에도 retry하지 않도록 설정했습니다.
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.
리트라이는 더블체크용도 였어서 중요한 부분은 아니여서 없어도 괜찮을거 같아요 테스트해보다가 문제가 있다면 그때 다시 추가하면 될거 같습니다
| headers: { memberTest: true }, | ||
| }); | ||
| } catch { | ||
| redirectIfNotMember(); |
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.
리다이렉션 관련 코드 다 지웠습니다!
| if (id) handleInitIsPublic(); | ||
| }, [id]); | ||
|
|
||
| useEffect(() => { |
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.
여기도 리다이렉션 관련 코드 다 지웠습니다.
| return response.data; | ||
| } catch (error) { | ||
| throw new Error('데이터를 불러오는 중 에러 발생: ' + error); | ||
| throw error; |
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.
원본 에러 보내주도록 했습니다.
src/hooks/useFetchData.ts
Outdated
| throw error; | ||
| } | ||
| }, | ||
| retry: 1, |
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.
retry 하지 않도록 설정했습니다.
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.
리다이렉트 훅 깔끔하게 하나로 통일했습니다.
| <ReactQueryDevtools initialIsOpen={false} /> | ||
| </ThemeProvider> | ||
| </Redirect> | ||
| </QueryClientProvider> |
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.
QueryClientProvider가 Redirect 내부로 이동했습니다.
wjsdncl
left a comment
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.
고생하셨습니다! 완전 깔끔해졌네요 👍
|
|
||
| useEffect(() => { | ||
| const handleRedirect = async () => { | ||
| const handleCheckMember = async () => { |
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.
리트라이는 더블체크용도 였어서 중요한 부분은 아니여서 없어도 괜찮을거 같아요 테스트해보다가 문제가 있다면 그때 다시 추가하면 될거 같습니다
src/components/Redirect/index.tsx
Outdated
| }, [currentPath, router.query.id]); | ||
|
|
||
| return children; | ||
| return <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>; |
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.
프로바이더를 내부에 넣은 건 어쩔 수 없을 거 같아서 괜찮을 거 같습니다 헷갈리진 않을거 같아요 🫡
wayandway
left a comment
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.
코드가 많이 덜어내졌네요 ㅎㅎ 잘 읽었습니다!!
retry를 0으로 줄여서 빠르게 한 것 좋네요 혜원님 수고하셨습니다!

연관된 이슈
작업 내용
코멘트 및 논의 사항