-
Notifications
You must be signed in to change notification settings - Fork 43
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
[김찬기] Sprint7 #250
The head ref may contain hidden characters: "React-\uAE40\uCC2C\uAE30-sprint7"
[김찬기] Sprint7 #250
Conversation
- product,auth 서비스 함수들 axios로 변경 - comment 서비스 함수 작성 - interceptor로 요청시 header에 토큰 주입 및 응답시 refresh token 사용 로직 추가 - authcontext 수정 (interceptor로 빠진 로직 제거) - abort case 수정 (axios와 fetch의 error name 차이)
- textarea에 size props 추가 - module css로 변경
- onRemoveItem이 있을경우에만 onclick, removable 전달
- 업로드 컴포넌트에서 string타입으로 이미지주소를 받으면 preview로 바로 넘기도록 개선
- 등록, 수정 페이지에 서비스 함수들을 가져와서 submit 함수를 만들어서 공용등록폼에 전달해서 실행
- comment 컴포넌들을 공용으로 분리 - products, articles 에서 공용으로 쓸 수 있게 개선 - useComment, useComments 훅 생성
- useSearchParams 내부를 보니, SetURLSearchParams가 navigateOpts을 선택적으로 받을수 잇게 되어있음을 보고, 스크롤 리셋 방지 옵션을 true 설정
- 게시물 페이지네이션 카운트수에도 적용
- product페이지에서만 사용하는 훅으로 변경
- variant props 추가 - color css 추가 (secondary, error)
- 앱 마운트시, 토큰 만료 검증후 재발급 하는 과정 추가 - 토큰이 재발급거나, 토큰이 유효한 경우에 유저정보를 불러옴 - 리프레시토큰으로 재발급요청하는 instance를 분리(기존 instance를 사용하면 다시 검증과 요청과정을 거치게 됨)
228e4d7
to
f5ceef0
Compare
- useref를 활용하여 유저정보조회 훅의 실행을 조절 - 토큰 재발급이 필요한지 체크하는 훅이 끝난후에 useref를 변경하여 유저정보조회가 가능하도록 개선
- 중복된 요청의 abort를 진짜 사용할곳에서만 하도록 변경 - 모든 요청에 중복요청을 못하도록 막으니까, 모든 요청의 에러에 aborterror일경우에는 다르게 핸들링하는 코드를 넣어야됨 - 인증관련 interceptor 설정을 auth context 내부로 들고옴 (state와 local storage의 sync를 위해)
- 맨토링시간에 배운 context api 개선건 적용
const DropdownStateContext = createContext(); | ||
const DropdownDispatchContext = createContext(); | ||
|
||
const useDropdownState = () => { | ||
const context = useContext(DropdownStateContext); | ||
if (!context) { | ||
throw new Error("Dropdown components must be used within an Dropdown."); | ||
} | ||
return context; | ||
}; | ||
|
||
const useDropdownDispatch = () => { | ||
const context = useContext(DropdownDispatchContext); | ||
if (!context) { | ||
throw new Error("Dropdown components must be used within an Dropdown."); | ||
} | ||
return context; | ||
}; |
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.
금요일 멘토링 시간에 보여주신 context api를 state와 dispatch로 구분해서 제공하는 방법 적용했습니다.
- 멘토링시간에 배운 cloneElement활용 - 썸네일 컴포넌트를 Fullscreen으로 감싸면 작동하도록 작업
export function Fullscreen({ children }) { | ||
const [imgSrc, setImgSrc] = useState(null); | ||
|
||
function handleOpenScreen(src) { | ||
setImgSrc(src); | ||
} | ||
|
||
function handleCloseScreen() { | ||
setImgSrc(null); | ||
} | ||
|
||
return ( | ||
<> | ||
{cloneElement(children, { onOpenScreen: handleOpenScreen })} | ||
{imgSrc && | ||
createPortal( | ||
<ImgViewer src={imgSrc} onCloseScreen={handleCloseScreen} />, | ||
document.querySelector("#root") | ||
)} | ||
</> | ||
); | ||
} | ||
|
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.
썸네일 컴포넌트를 감싸게될 Fullscreen 컴포넌트가 자식요소인 썸네일 컴포넌트에게 props를 주도록 작업했습니다.
확대보기컴포넌트는 포탈을 이용해서 dom 렌더링 위치를 root바로 아래로 옮겼습니다.
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.
이후에 Modal 컴포넌트를 만들일이 있을 때,
createPortal 하는 기능을 분리해서 Modal 컴포넌트를 만들고,
FullScreen은 Modal에 ImgViewer를 얹는 방향으로 구현해도 좋을 것 같아요.
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.
스프린트 미션 하느라 수고 많으셨어요~👏🏻
#### react router의 loader를 사용할시 추가로 설정해줘야했던 설정들 | ||
|
||
- React의 Suspense와 fallback, React router의 loader, hydrateFallback을 조사해보고 정리해보았습니다. | ||
- https://heavy-bear.tistory.com/13 |
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.
👍
<Author avatar={image} nickname={nickname} updatedAt={updatedAt} /> | ||
</div> | ||
</div> | ||
<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.
More 기능이 댓글 작성자에게만 보여지는게 자연스럽다고 생각해서
스프린트 미션에 디테일한 요구사항이 없긴 하지만, hasEditPermission 같은 데이터로 렌더링 여부를 결정하는게 더 좋지 않을까 생각해요.
조금 덧붙이면 comment데이터를 넘겨주는 useComments 레벨에서 useAuth를 호출해서 권한에 대한 데이터도 comments에 함께 담아 넘겨줘도 좋을 것 같아요.
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.
white-space: pre-wrap; | ||
} | ||
|
||
.footer { |
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.
module.scss를 사용하고, 컴포넌트 안에서 클래스 이름이 중복되지 않는 경우가 일반적이라
클래스 이름을 부모 자녀 관계가 드러나지 않고 flat하게 작성해도 좋아요.
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.
감사합니다. nesting한 css를 한번 flat하게 변경해보겠습니다.
export function Fullscreen({ children }) { | ||
const [imgSrc, setImgSrc] = useState(null); | ||
|
||
function handleOpenScreen(src) { | ||
setImgSrc(src); | ||
} | ||
|
||
function handleCloseScreen() { | ||
setImgSrc(null); | ||
} | ||
|
||
return ( | ||
<> | ||
{cloneElement(children, { onOpenScreen: handleOpenScreen })} | ||
{imgSrc && | ||
createPortal( | ||
<ImgViewer src={imgSrc} onCloseScreen={handleCloseScreen} />, | ||
document.querySelector("#root") | ||
)} | ||
</> | ||
); | ||
} | ||
|
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.
이후에 Modal 컴포넌트를 만들일이 있을 때,
createPortal 하는 기능을 분리해서 Modal 컴포넌트를 만들고,
FullScreen은 Modal에 ImgViewer를 얹는 방향으로 구현해도 좋을 것 같아요.
@@ -2,20 +2,20 @@ import { useEffect, useState } from "react"; | |||
import { getDeviceType } from "@util/breakpoints"; | |||
import { debounce } from "@util/debounce"; | |||
|
|||
export default function usePageSize( | |||
initialPageSize = { | |||
export default function useReponsive( |
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.
useResponsive 라는 이름이 반응형에 따라 일반적으로 사용하려는 걸로 보여서
나머지 pageSize, setPageSize도 같이 이름을 변경하면 좋을 것 같아요.
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.
훅의 용도를 일반적으로 바꾸면서 내부 state변수명을 생각을 못했네요;;
감사합니다. ㅎㅎ
// 두 요청을 거의 동시에 보내도록 우선 await없이 promise를 각 변수에 할당함 | ||
// 여기서 미리 await을 작성하면 그 요청을 기다린후 다음줄의 코드가 실행되어서 동시에 요청이 안됨 | ||
const detail = getProduct(id); | ||
const comments = getComments("products", { productId: id, limit: 5 }); |
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.
getComments를 먼저 실행하고
await getProduct 실행해도
return 에서 await 하는 거랑 결과가 같지 않나요?
return 에서 await 하는걸 수정하면 좋겠다는 의도에서 하는 얘기는 아니고 그냥 궁금해서 남겨요ㅎㅎ
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.
결과가 같게 나옵니다. 😱
- 왜 코드작성할때는 자꾸 코멘트를 가져오는 구문을 꼭 상품상세 밑쪽으로 적으려했는지 ㅠㅠ
- 리뷰해주신내용처럼 코멘트 promise를 실행시켜버리고, 상품데이터 promise를 await하는 방식이 더좋은것같아요;;;
만약에 리턴에서 깨끗하게 await을 안쓰고 동일한 방식으로 동작하려면 이렇게 작성해야하더라구요.
(promise를 실행시키는건 거의 동시적으로 일어나면서, 한쪽은 다시 await하는 방식)
코드 한줄이 더늘어나는것 같아서 리턴문에서 기다리자하고 수정했었는데, 멘토님이 말해주신방법이 더 좋은것 같습니다ㅎㅎ
요구사항
기본
상품 상세
상품 문의 댓글
심화
주요 변경사항
스크린샷
멘토에게
axios interceptor의 response에서 토큰 재발급을 받는 과정과 interceptor에서 재요청시 abort시키는 과정을 합치다보니 axios 인스턴스가 좀 복잡해졌습니다. (두 과정을 따로 구글링으로 알게 되어서 혼합 했을때 스스로 생각하기에 순서가 이렇게 되어야 할 것 같은 순서로 재조합 했습니다.)