Skip to content
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

refactor: type + axios interceptor #305

Merged
merged 6 commits into from
Dec 9, 2024
Merged

refactor: type + axios interceptor #305

merged 6 commits into from
Dec 9, 2024

Conversation

rabyeoljji
Copy link
Contributor

@rabyeoljji rabyeoljji commented Dec 8, 2024

개요

  • 우선 인터셉터 쪽에서 getSession을 사용할 수 있다는 걸 알게되어 이렇게 구현했습니다
  • 급한 연결은 다 해결됐고 나중에라도 발견하기 쉽게 클라이언트/서버 둘 다 사용되는 fetch함수는 보이는 대로 주석도 달아뒀습니다ㅎㅎ
  • 앞으로 하나씩 발견될 때마다 해결하면 될 것 같습니다!
  • getSession은 이전 버전용, 최신 버전에서 사용하는 게 맞는 방법일까?
  • provider로 구현하는 쪽 고려
  • 변경된 스웨거 타입 방식이 전부 옵셔널로 반환되는데 deepRequired를 사용하면 유니언 타입에서 undefined가 포함되는 현상

세부 내용

관련 링크

@rabyeoljji rabyeoljji self-assigned this Dec 8, 2024
@rabyeoljji rabyeoljji requested a review from jw-r December 8, 2024 12:58
Copy link
Member

@jw-r jw-r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넘 좋아용~!
정말 고생하셨습니다👍
몇가지 코맨트만 확인 부탁드려욥!

@@ -1,5 +1,5 @@
import RandomQuizView from '@/features/quiz/screen/random-quiz-view'
import { getBookmarkedCollections } from '@/requests/collection'
import { getBookmarkedCollections } from '@/requests/collection/client'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server로 로직이 변경되어야하는 것 아닌가용?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 해당 함수가 hook에서도 사용되고 page에서도 사용되고 있어서 일단 client에만 뒀었는데 그런 애들은 그럼 일괄적으로 server파일에 똑같이 만들어서 import 해두겠습니다

const AuthContext = createContext<string | null>(null)

/** 사용할 때 useToken */
export const AuthProvider = ({ children }: { children: React.ReactNode }) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getSession으로 session으로 호출할 수 있다면 auth provider는 불필요하지 않나용?
getSession은 이전 버전에서 사용하던 건데 현재 버전에서도 사용가능하군요 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 사용은 가능한데 말씀하신 것처럼 이전 버전용이고 그래서 provider를 이용해서 하는 게 더 나은 방법 같기도 해서 나중에 추가했어요ㅎㅎㅠㅠㅠ provider를 이용한 코드로 전체적으로 수정하고 getSession을 사용을 안하는 코드로 변경해서 업데이트하려고 합니다...!

type Record = QuizRecord
type Result = UpdateQuizResultPayload['quizzes'][number]

type Type = Exclude<DeepRequired<components['schemas']['QuizDto']['quizType']>, undefined>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

와웅 이걸로 어떤 부분이 해결될 수 있었나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

흠 일단 옵셔널을 deeprequired로 바꾸다보니까 유니언 타입에서 undefined가 같이 껴서 반환이 돼서 이걸 최대한 없애보고자 이렇게 작성해보긴 했는데 다른 좋은 방법은 없을까 고민중입니다ㅠㅠ

@rabyeoljji rabyeoljji merged commit 6d34383 into develop Dec 9, 2024
1 of 5 checks passed
@rabyeoljji rabyeoljji deleted the refactor-zustand branch December 9, 2024 08:08
rabyeoljji added a commit to rabyeoljji/pick-toss-next that referenced this pull request Dec 9, 2024
* refactor: type + axios interceptor

* fix: cspell

* feat: zustand

* refactor: 주석 추가

* fix: type

* fix: token
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants