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] token 필요없는 fetch custom hook 만들기 #445

Merged
merged 4 commits into from
Sep 7, 2024

Conversation

eonseok-jeon
Copy link
Member

@eonseok-jeon eonseok-jeon commented Sep 6, 2024

Related Issue : Closes #444


🧑‍🎤 Summary

  • axios를 이용한 instance에서 fetch를 이용한 instance로 변경
  • 기존 axios instance 이용한 부분 fetch instance로 변경

🧑‍🎤 Comment

코드 짜기 전에는 hook으로 분리해줄까 했었는데
짜고 보니 분리할 필요가 없더라고요?
원래 방식처럼 그냥 바로 instance 가져다가 사용하면 됐어요

사용법은 비슷해요

첫 번째 인자로 url 넣어주고 두 번째 인자로 options들 넣어주면 됩니다~
단 body는 JSON.stringify()로 감싸줘야 해요!

const api요청 = async (인자들) => {
  const res = await fetchInstance(url, {
    method: 'POST',
    headers: {
      // ...
    },
    body: JSON.stringify({
      // ...
    }),
    // ...
  });

  return res;
};

@eonseok-jeon eonseok-jeon linked an issue Sep 6, 2024 that may be closed by this pull request
Copy link

height bot commented Sep 6, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Copy link

@wuzoo wuzoo left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 !

하나의 의견이 있는데, 현재 headers에서 Record 유틸리티 타입으로 string, string key-value를 받고 있는데, 잘못된 key 값을 헤더에 넣었을 시 에러가 발생할 수도 있을 것 같다는 생각을 했습니다.

fetch api를 사용하기로 했으므로 Union 타입으로 headers key 값에 들어갈 스트링을 제한해주면 어떨까 싶습니다 !

@wuzoo
Copy link

wuzoo commented Sep 6, 2024

고생하셨습니다 !

하나의 의견이 있는데, 현재 headers에서 Record 유틸리티 타입으로 string, string key-value를 받고 있는데, 잘못된 key 값을 헤더에 넣었을 시 에러가 발생할 수도 있을 것 같다는 생각을 했습니다.

fetch api를 사용하기로 했으므로 Union 타입으로 headers key 값에 들어갈 스트링을 제한해주면 어떨까 싶습니다 !

Changes request 잘못 눌렀네요 ㅎㅎ..

@eonseok-jeon
Copy link
Member Author

@wuzoo
좋은 거 같습니다! 모든 걸 다 제한하기엔 너무 많아서 대표적으로 사용되는 애들로만 일단 제한을 걸었어요

type StandardHeaders = 'Content-Type' | 'Authorization' | 'Accept' | 'Cache-Control' | 'User-Agent';

Copy link

@wuzoo wuzoo left a comment

Choose a reason for hiding this comment

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

뽕쌈한 것 같습니다 !!

Copy link
Member

@lydiacho lydiacho left a comment

Choose a reason for hiding this comment

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

좋아요 ~ ~ ~ ~ 🚀 🚀

저도 주용오빠와 같은 맥락의 의견 내자면 method option도 타입 좁혀주는거 어떠신가요 ~??
물론 header key 에 비해서는 저희가 잘 알고있는 친구들이라 추론 없이도 큰 탈 없이 사용해줄 수 있겠지만
기존 axios 사용시에는 AxiosInstance 가 가지는 메소드로 추론이 됐었는데 현재 쓰이는 RequestInit의 method는 string으로만 되어있어서
대표적으로 사용하는 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' 타입만 아주 간단히 정의해주면 axios쓸때랑 똑같은 기능을 얻을 수 있어서 추가해줘도 좋을 것 같아용

@wuzoo
Copy link

wuzoo commented Sep 6, 2024

완동 입니다 ~ 🚀

@eonseok-jeon
Copy link
Member Author

eonseok-jeon commented Sep 6, 2024

@wuzoo @lydiacho
너무 좋네요~ 반영 했습니다 :)

@eonseok-jeon eonseok-jeon merged commit 64c8410 into develop Sep 7, 2024
@eonseok-jeon eonseok-jeon deleted the refactor/#444_fetch-without-token branch September 7, 2024 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Refactor] token 필요없는 fetch custom hook 만들기
3 participants