refactor: 모든 API 호출 로직이 codegen을 통하도록 수정함 #226
Conversation
짱나네; |
일단 정상화 하지만 첫 요청 때 토큰 없는 상태로 호출하고 무조건 클라에서 토큰 재발급 로직을 타는거 같다 |
왠지 next/headers cookie 아니면 서버에서 쿠키 못가져오는 모양인데 |
나참 |
| {enableGoToSignUpButton(pathname) ? ( | ||
| <S.GoToSignUpButton onClick={() => router.push(ROUTE.SIGN_UP)}> | ||
| 피둥 시작하기 | ||
| </S.GoToSignUpButton> | ||
| ) : ( | ||
| <GoToSignUpButton /> | ||
| getAccessTokenFromCookie() && ( | ||
| <Suspense> | ||
| <Profile /> | ||
| </Suspense> | ||
| ) |
There was a problem hiding this comment.
조건이 이렇게 걸리면 피둥을 사용하지 않는(혹은 로그인하지 않은 유저)가 타인의 구독리스트를 보러 들어 갔을 때 오른쪽 상단에 아무 것도 노출되지 않게 되는데 의도하신걸까요? 👀 개인적으로는 타인의 구독 리스트를 보러 갔을때 우측 상단에 '피둥 시작하기' 버튼을 띄워주면 유입의 가능성이 있다고 생각해서 피둥시작하기 버튼 or 프로필 노출의 분기 조건을 로그인 여부로 사용하면 어떨까 하는 의견이에요
There was a problem hiding this comment.
@eunsonny 깜빡임이 있어서 저렇게 처리가 되었던 것인데 말씀해주신 케이스는 제가 놓쳤어요
적절한 조건을 찾아서 수정해볼게요
There was a problem hiding this comment.
깜빡임의 원인을 찾았는데요 말씀하신 대로 로그인 시 보이도록 하되
- 일단 AsyncLocalStorage를 쓰면 대강 구현을 시도해보고 실패하면
- app router로 마이그레이션 완료되기 까지 Nav 내 유저 데이터 / 버튼 간 깜빡임 이슈는 known issue로 두고 빠르게 나머지 경로도 app router로 마이그레이션 했으면 해요.
핵심은 아래 함수인데요
const getIsomorphicCookies = () =>
isServer() && isAppRouter() ? getNextCookies() : Cookies여기에서 앱 라우터 내 로직이면 getNextCookies를 타면서 cookie를 불러오고, 페이지 라우터 & 클라이언트 로직이면 js-cookie의 Cookies 로직을 타도록 되어 있어요.
근데 제가 놓지고 있던게 js-cookie는 서버 측에서는 기본적으로 동작하질 않아요
https://github.com/js-cookie/js-cookie/blob/main/src/api.mjs#L4-L8
function init(converter, defaultAttributes) {
function set(name, value, attributes) {
if (typeof document === 'undefined') {
return
}깔려 있지만 사용되지 않고 있는 nookies라는 라이브러리를 사용하면 동작하긴 하는데 그러려면 getServerSideProps에서 ctx 객체에 접근해 cookie 값을 읽고 내려줘야 하는데... 현재 코드 구조 상 이걸 전역적으로 주고 받을 수가 없어서(app router에서는 전역적으로 주고 받기 위한 구현이 추가되었어요) 쿠키 관련 로직 사용하기가 좀 곤란해요.
이 문제를 해결하기 위해 서버 측에서 AsyncLocalStoarge를 사용하도록 하거나 잠시 흐린 눈 하는 방법을 선택해야 할거 같아요
There was a problem hiding this comment.
There was a problem hiding this comment.
아하 이해했습니당
- 일단 AsyncLocalStorage를 쓰면 대강 구현을 시도해보고 실패하면
- Nav 내 유저 데이터 / 버튼 간 깜빡임 이슈는 known issue로 두고 빠르게 나머지 경로도 app router로 마이그레이션
이 방향으로 진행하는 것으로 알고 있겠습니다. 저는 app router로 빠르게 마이그레이션에 힘을 보탤게요(...)
| }) | ||
| } | ||
|
|
||
| export const useGetUserProfileByUsername = ( | ||
| username: string, | ||
| options: Omit<UseQueryOptions<UserProfile>, 'queryKey'> = {} | ||
| options: Omit<UseQueryOptions<PublicUserInfoResponse>, 'queryKey'> = {} | ||
| ) => { | ||
| return useQuery({ | ||
| return useQuery<PublicUserInfoResponse>({ | ||
| queryKey: [CACHE_KEYS.user, username], | ||
| queryFn: () => getUserInfoByUsername(username), | ||
| queryFn: async () => getPublicUserInfoUsingGET(username), |
There was a problem hiding this comment.
여기에는 왜 async가 들어간걸까요? 이유가 있을까요? 🤔
There was a problem hiding this comment.
이거저거 시도해보던 흔적입니다 지워야 해요 ㅎㅎ
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| export const getNextCookies = () => require('next/headers').cookies() |
| const { cookies } = require('next/headers') | ||
| cookies() |
There was a problem hiding this comment.
여기서도 쿠키가 필요한건가요? 테스트용이 아니였나 싶습니다 🤔 (??)
There was a problem hiding this comment.
@eunsonny app router인지 여부를 app router 전용인 next/headers의 cookies 함수 호출 성공 여부로 판단하고 있기 때문에 필요한 구현입니다. 요상해보이겠지만 정식 API를 제공하지 않아서 이런 work around 말고는 방법이 없네요 ㅎ..
There was a problem hiding this comment.
아하 그래서 try catch로 나누어 리턴하는거군요
Motivation 🤔
Key Changes 🔑
To Reviews 🙏🏻
next/headers를 require로 불러온 이유는 정적으로 import 하면 cookie 관련 로직을 공용으로 사용할 수 없기 때문입니다. 그리고 만약 require 대신import('next/headers')를 사용하면 외부 로직들이 전부 비동기 함수가 되기 때문에 사용하지 못했습니다.