[FEAT]: 관리자 매치 정보 수정 구현 #126
Conversation
refactor: 리그 id 컨텍스트, 리그 정보 조회, 컴포넌트 재작명 style: Card 시인성 향상 chore: LeagueIdWrapper layout으로 공통 적용
feat: 매치 정보 수정 컴포넌트, 수정 쿼리 구현 feat: select box 컴포넌트 구현 chore: 스포츠쿼터조회 api league -> match.ts 이동, 쿼리 추가
refactor: 관리자 도메인 접근 권한 예외처리 추가
|
|
||
| return config; | ||
| }); | ||
|
|
||
| let retryCounter = 0; | ||
|
|
||
| adminInstance.interceptors.response.use( |
There was a problem hiding this comment.
궁금한 게 있는데, tanstack query의 retry 옵션으로 재요청을 보내는 것과 어떤 차이가 있을까요?
There was a problem hiding this comment.
아 tanstack query는 그 성격이 react hook이다보니까 jsx(tsx)내부에서밖에 사용하지 못합니다. 그래서 해당 파일은 일반 타입스크립트 파일이기 때문에 해당 기능을 사용하지 못했습니다!
| <section> | ||
| <AsyncBoundary | ||
| errorFallback={props => <MatchInfoDetail.ErrorFallback {...props} />} | ||
| loadingFallback={<div>로딩 중입니다...</div>} |
There was a problem hiding this comment.
나중에 loading fallback도 추가해주시면 좋을 것 같아요!
There was a problem hiding this comment.
넵 확인했습니다. 에러/로딩 fallback도 레포 분리 이후에 제대로 작성하는 시간을 가져야할 것 같습니다.
| import AsyncBoundary from '@/components/common/AsyncBoundary'; | ||
| import AuthMatchInfoFetcher from '@/queries/admin/match/useMatchInfoByIdWithAuth/Fetcher'; | ||
|
|
||
| export default function Page({ |
There was a problem hiding this comment.
컴포넌트명을 단순히 Page로 작성한 이유가 있을까요?
There was a problem hiding this comment.
이것또한 레포가 분리돼있지 않아서 페이지 컴포넌트명을 작명하는데 좀 어려움이 많아 그냥 Page로 통일시켜버렸습니다. 저희 프로젝트 구조상 페이지 컴포넌트 자체에 기능들이 곧바로 작성돼있는게 아니라 기능별 서브 컴포넌트로 분리되어 호출하고 있는 방식이기 때문에 페이지 컴포넌트 자체에 이름을 붙이는게 큰 의미가 없다고 판단했습니다.
추후 레포를 분리하게 되면 이 부분에 있어 제약이 줄어듦에 따라 다른 컨벤션을 적용할 수도 있을 것 같습니다.
| useEffect(() => { | ||
| setInParams(QUERY_PARAMS.league, leagueId); | ||
| checkPermission(); | ||
| }); |
There was a problem hiding this comment.
해당 useEffect는 컴포넌트 최초 마운트시에만 작동하도록 의도해서 의존성 배열을 추가한다하더라도 빈 배열을 추가하기 때문에 딱히 의미가 없긴 한데 그래도 추가하는게 좋을까요? 왜냐하면 빈 배열을 추가하는게 에러는 아니지만 lint에도 Warning : React Hook useEffect has missing dependencies: 'leagueId' and 'setInParams'. Either include them or remove the dependency array. 이란 경고가 매번 뜨는데 되도록이면 최초 마운트 시에만 동작하는 useEffect들은 의존성 배열을 경고처럼 제거해서 경고가 안뜨게 하는게 맞지 않나란 생각이 듭니다.
|
@seongminn 저는 개인적으로 존댓말로 작성하는게 한 번 더 생각을 거쳐야한다고 생각해서 좀 귀찮다 생각했는데, 사실 크게 지장이 있는 정도는 아니라 보류해도 괜찮습니다. 레포 분리는 제가 지금 로컬에서 시범으로 혼자 해보고 있는데, 꽤나 복잡해서 찾아가면서 하느라 시간이 오래 걸렸습니다. yarn berry로 버전업하는덴 성공했는데 이를 충돌없이 진행하려면 sentry/nextjs의 버전을 최신버전으로 업데이트해야하는 등 조금의 의존성 변경사항이 요구됨을 파악했습니다. 이는 내일 오프라인으로 만나면 그 때 전달드리도록 하고, 이제 딱 앱을 분리하면 되는 단계인데 분리하려하니 공통으로 사용하고 있는 컴포넌트들이 중구난방으로 흩어져있고 이를 일정한 컨벤션에 맞게 모아둘 장소도 생각해야해서 다같이 이야기를 나눠봐야할 것 같습니다. |
🌍 이슈 번호
✅ 작업 내용
📝 참고 자료
♾️ 기타
/라우트 컴포넌트를 그대로 차용하고 있기 때문에, 루미큐브 경기들은/match라우트가 아닌/rummikube라우트로 링크됨에 따라 매치 정보 수정 페이지가 생성되지 않는(404) 에러가 있습니다.❗ 현재 셀렉트박스를 펼치면 이전에 대화 나눴던([STYLE] Radix-UI 사용시 max-w-md 적용 위치 #125 ) 페이지 찌그러짐 현상이 발생합니다. radix-ui 라이브러리를 사용하는 이상 body 태그에 적용된
max-w-md속성을 한 단계 위인 html 태그로 옮기는게 적합하다 생각합니다(모달 컴포넌트도 동일하게 영향받음)./queries/admin/ 디렉토리에 포함돼있는 관리자용 쿼리 함수 및 패쳐의 변수명과 관련해, 특히 사용자용 쿼리와 유사한 useMatchInfoById 쿼리에 간이로 API 함수 컨벤션인 'WithAuth' 접미사를 붙여 구분하도록 하였습니다.
추가로 PR에서도 반말 문화를 적용하는건 어떨까요? 코드리뷰할 때야말로 반말이 주는 소통 비용 절감 이점을 누릴 수 있지 않을까 생각합니다 😄