[김찬기] Sprint12#330
Hidden character warning
Conversation
- 전부 클라이언트 사이드에서 처리하도록 교체 (기존은 서버액션)
| let cachedClientAccessToken: string | null = null; | ||
|
|
||
| const getAccessTokenAtServer = cache(async () => { | ||
| const session = await auth(); | ||
| return session ? session.accessToken : null; | ||
| }); | ||
|
|
||
| const getAccessTokenAtClient = async () => { | ||
| if (!cachedClientAccessToken) { | ||
| const session = await getSession(); | ||
| return session ? session.accessToken : null; | ||
| } | ||
| return cachedClientAccessToken; | ||
| }; | ||
|
|
||
| axiosInstance.interceptors.request.use( | ||
| async (config) => { | ||
| const session = | ||
| typeof window === "undefined" ? await auth() : await getSession(); | ||
| const accessToken = session?.accessToken; | ||
| const accessToken = | ||
| typeof window === "undefined" | ||
| ? await getAccessTokenAtServer() | ||
| : await getAccessTokenAtClient(); | ||
|
|
||
| if (accessToken) { | ||
| config.headers.Authorization = `Bearer ${accessToken}`; | ||
| } else { | ||
| delete config.headers.Authorization; |
There was a problem hiding this comment.
이전 코드리뷰에서 말해주신 내용중에, 매번 session을 요청하는것에 대해서 이렇게 개선해보았습니다.
- 클라이언트 사이드에서 토큰을 헤더에 넣을때는 캐시된것이 있으면 재사용하고,
- 서버사이드에서 토큰을 헤더에 넣을때에는 매번 새로운 요청으로 토큰을 받아오도록 했습니다.
Lanace
left a comment
There was a problem hiding this comment.
고생하셨어요ㅎㅎ!
컴포넌트를 깔끔하게 잘 만드셔서 그런지 재사용하기도 편하고 확장성도 열려있어서 좋네요
일관성도 있어서 덕분에 코드리뷰도 편하게 했네여!
감사합니다~!!
| import ProductForm from "@/components/market/ProductForm"; | ||
| import { useProductAdd } from "@/service/product.queries"; | ||
|
|
||
| export default function AddItemPage() { | ||
| const { mutateAsync: handleProductAdd } = useProductAdd(); | ||
|
|
||
| return ( | ||
| <PageWrapper> | ||
| <ProductAddForm /> | ||
| <ProductForm mode="add" onFormSubmit={handleProductAdd} />; | ||
| </PageWrapper> | ||
| ); |
There was a problem hiding this comment.
좋은 구조인것같아요...! 추가인지 수정인지에 따라 로직 수정도 쉽고 좋네여!
조금 더 개선한다면 ProductForm에 isLoading 같은 props를 받아서 로딩처리도 같이 할 수 있으면 좋겠네여ㅎㅎ!
아 그리고 외부에 submit 버튼이 있는경우에도 사용할 수 있으려면 formId를 받아서 처리하는것도 방법이에요
| if (isPending) { | ||
| return <Loading>게시물 정보를 가져오는 중입니다.</Loading>; | ||
| } | ||
|
|
||
| if (!detail) { | ||
| notFound(); | ||
| } |
There was a problem hiding this comment.
useGetArticle 에서 enabled 조건이 없어서 isPending 이 처음부터 true 인 경우엔 의도하신대로 동작할것같아요!
근데 만약에 enabled에 조건이 들어가서 바로 fetching이 되지 않는 경우엔 isPending이 false 가 되고, defail은 undefined라서 notFound 페이지로 빠질것같아요.
isFetched 상태도 같이 활용하는것도 방법일것같구요
There was a problem hiding this comment.
생각을 못했던 부분인것 같습니다. 감사합니다! 다시 한번 작성해보겠습니다.
| detail: Product; | ||
| } | ||
|
|
||
| export default function ProductDetail({ detail }: ProductDetail) { | ||
| const { | ||
| id, | ||
| images, | ||
| name, | ||
| price, | ||
| description, | ||
| tags, | ||
| ownerId, | ||
| ownerNickname, | ||
| updatedAt, | ||
| favoriteCount, | ||
| isFavorite, | ||
| } = detail; | ||
| const { data: session } = useSession(); | ||
| export default function ProductDetail() { | ||
| const router = useRouter(); | ||
| const { handleLike, handleProductDelete } = useProductActions(id); | ||
| const isOwner = ownerId === Number(session?.user?.id); | ||
| const { data: session } = useSession(); | ||
| const { id } = useParams<{ id: string }>(); | ||
| const productId = Number(id); |
There was a problem hiding this comment.
이렇게 해도 틀린건 아니고 사실 많이 이렇게 하긴 해요ㅎㅎ;;
components/.../ProductDetail.tsx 이니 가급적 props를 통해서 id 값을 넘겨받는게 어떨까 싶긴 해요
외부 의존성을 최대한 줄이고, props로 받아와서 로직적인 부분에 대한 제어를 부모쪽에서 하는게 재사용하기 좋은 구조가 될꺼에요~!
id값을 number로 부모에서 받으면 좋을것같아요!
| if (confirm("정말 삭제할까요?")) { | ||
| try { | ||
| await handleArticleDelete(); | ||
| alert("상품을 삭제했습니다."); | ||
| await deleteArticle(); | ||
| alert("게시글을 삭제했습니다."); | ||
| router.replace("/boards"); | ||
| } catch (err) { | ||
| console.log(err); | ||
| console.error(err); |
There was a problem hiding this comment.
관심사를 분리하는쪽으로 로직을 나누면 좋을것같아요!
deleteArticle이 성공했을때와 실패했을때를 각각 나누었다면 차라리 여기선 useArticleDelete 에서 onSuccess 와 onError 를 callback으로 받아서 처리할 수 있도록 하면 어떨까 싶어요ㅎㅎ!
원래 useMutaion에 있는 onError와 onSuccess 처럼요
| } | ||
| await handleLike(!isLiked); | ||
| router.refresh(); | ||
| toggleLike(!isLiked); |
There was a problem hiding this comment.
이미 알고 계시긴 할텐데, 습관적으로 이전 상태값 기준으로 상태를 변경하면 좋긴 해요ㅎㅎ;;
(너무 사소하긴 하지만ㅠ)
toggleLike(prev => !prev);There was a problem hiding this comment.
toggleLike가 useMutation의 mutate함수를 이름을 다르게 지정해서 쓴거라서
usestate의 기존 상태값을 통해서 상태변경을 하는 방식을 못썼습니다.
제가 다르게 이해한건지, 한번 더 체크해볼겠습니다!
There was a problem hiding this comment.
useMutation의 onMutate 콜백에서 이전 상태값을 기준으로 변경하는 방식으로 다시 작업해보겠습니다!
| } | ||
|
|
||
| return ( | ||
| <> |
There was a problem hiding this comment.
아마 lint에 잡혔을것같긴 한데, 자식이 1개일떈 <>...</> 는 지워도 될것같아요!
| } catch (error) { | ||
| const message = isAxiosError(error) | ||
| ? error.response?.data.message | ||
| : "알 수 없는 에러가 발생했어요."; | ||
|
|
||
| throw new Error(message); |
There was a problem hiding this comment.
이부분은 반복되는 내용들이 조금 있는것같아서 유틸쪽으로 합쳐도 괜찮을것같네여ㅎㅎ!
| // https://github.com/nextauthjs/next-auth/issues/9465 | ||
| // redirect false로 응답을 받아볼때, 로그인실패도 ok가 true 전달되고 있음 | ||
| // 임시로 message와 코드로 실패처리 | ||
| if (response?.error === "CredentialsSignin") { | ||
| throw new Error(response.code); |
There was a problem hiding this comment.
오... 정말 그러네요;; ㄷㄷ
주석으로 이슈가 되는 링크도 그렇고 대처도 그렇도 너무 좋네요ㅎㅎ!
| useComments(Number(id), name, data); | ||
| const { isLoading, error, data, hasNextPage, fetchNextPage } = useGetComments( | ||
| name, | ||
| { id: Number(id), limit: 3 } |
There was a problem hiding this comment.
여기도 만약 id를 props로 받아오면 조금더 의존성을 줄일 수 있을것같아요~
아 그리고 좋은점이 만약 id값이 이상하게 들어오는 경우를 대비하여 NaN 검사도 해야하는데, 그러한 로직을 부모 컴포넌트로 올리고 CommentList.tsx 는 로직에만 집중할 수 있다는 장점도 생기겠네요ㅎㅎ!
| let cachedClientAccessToken: string | null = null; | ||
|
|
||
| const getAccessTokenAtServer = cache(async () => { | ||
| const session = await auth(); | ||
| return session ? session.accessToken : null; | ||
| }); | ||
|
|
||
| const getAccessTokenAtClient = async () => { | ||
| if (!cachedClientAccessToken) { | ||
| const session = await getSession(); | ||
| return session ? session.accessToken : null; | ||
| } | ||
| return cachedClientAccessToken; | ||
| }; | ||
|
|
||
| axiosInstance.interceptors.request.use( | ||
| async (config) => { | ||
| const session = | ||
| typeof window === "undefined" ? await auth() : await getSession(); | ||
| const accessToken = session?.accessToken; | ||
| const accessToken = | ||
| typeof window === "undefined" | ||
| ? await getAccessTokenAtServer() | ||
| : await getAccessTokenAtClient(); | ||
|
|
||
| if (accessToken) { | ||
| config.headers.Authorization = `Bearer ${accessToken}`; | ||
| } else { | ||
| delete config.headers.Authorization; |
요구사항
기본
중고마켓
상품 상세
상품 등록
심화
주요 변경사항
스크린샷
멘토에게