Migration ax fetch

[seonghunYang]

📌 작업 내용

구현 내용 및 작업 했던 내역

• user 도메인 로직을 fetch-ax로 마이그레이션하기
• fetch-ax 전용 에러 이벤트 핸들러 만들기
• fetch-ax 인스턴스 생성하기

🤔 고민 했던 부분

• 가현이 말대로, requestInterceptor에서 쿠키가 잘 가져와지네요! user 쪽 코드를 보시면 아시겠지만, 코드량이 유의미하게 줄었습니다. fetch-ax 개발의 보람을 느낄 수 있어요.
• 마이그레이션하면서 fetch-ax의 개선점을 3가지 정도 발견했습니다. 여러분들의 의견을 듣고 싶네요.
  1. next가 확장하는 속성 타입이 없는 문제: fetch RequestInit 타입에 Next.js가 'next'라는 속성을 확장해 놨는데, fetch-ax에서 해당 타입을 볼 수 없는 문제가 있었습니다. 이 문제는 중대해 보여서 fetch-ax에 이미 수정사항을 올려놨습니다.
  2. 일부 로직(예: resetPassword)에서는 응답값이 JSON이 아니라 responseType을 text로 지정해줘야 했습니다. 이런 경우 에러가 발생하기도 하고, 개발 경험 향상을 위해 자동으로 파싱 처리를 해주면 좋겠다는 생각이 들었습니다.
  3. baseURL이 지정되어 있지 않은 상태에서 '/user/sr'와 같은 상대 경로를 넣으면 Invalid URL 에러가 발생합니다. (그래서 URL 쪽 일부를 수정했습니다.) baseURL 지정이 안 되어 있을 때 해당 origin으로 요청을 보내도록 개선하면 좋을 것 같습니다.

[gahyuun]

고생하셨습니다!

1. next 확장하는 속성을 사용하지 못하는 건 큰 에러네요,, 발견해주셔서 감사합니다
2. 개발 생산성이 향상되는 것에 동의하며 기능 추가가 되면 좋을 것 같습니다
3. 이 부분은 어차피 404 에러가 날거여서 크게 고려하지 못했네요 이 부분도 해결이 필요할 것 같습니다!

그리구 스토리북 크로마틱 실패뜨는거 확인 부탁드려요!

[gahyuun]

  } catch (error:FetchAxError<ErrorData>) {
    const status = error.statusCode;
    switch (status) {
      case HttpStatusCode.Unauthorized:
        throw new UnauthorizedError({
          message,
          response,
        });
  
  }
}

FetchAX에서 error를 throw해줘서 responseRejectedInterceptor를 사용하지 않아도 위처럼 catch문에서 처리할 수 있었을 것 같은데 따로 responseRejectedInterceptor를 사용한 이유가 있나요?!

[seonghunYang]

• 일단 해당 API가 Authorization 속성에 값이 들어가면 에러를 발생시키기 때문에, 이 부분만 기존 instance 대신 fetchAx를 사용했습니다. 에러 핸들러를 추가한 이유는 다른 코드들과 마찬가지로 에러 인스턴스를 받아 일관적으로 처리하는 것이 가독성과 유지보수 측면에서 더 유리하다고 판단했기 때문입니다.

[gahyuun]

아하 넵 감사합니다!
혹시 이 부분에 대해서 조금만 더 설명해주실 수 있나요!?
해당 API가 Authorization 속성에 값이 들어가면 에러를 발생

[seonghunYang]

말그대로 수환이가 만든 저 요청에만 이상하게 Authorization속성에 값을 넣어주면 에러가 발생하더라구요!

[gahyuun]

responseType을 text로 지정하지 않으면 에러가 발생한다는 말씀 맞을까요?

[seonghunYang]

• 네 맞습니다!

[gahyuun]

utils의 api 폴더에 있는게 더 적절하지 않을까요?

[seonghunYang]

c3c1db2

[seonghunYang]

좋은 의견이네요! 수정했습니다

[gahyuun]

'Content-Type': 'application/json' 은 기본 값이어서 넣어주지 않아도 되지 않나요!?

[seonghunYang]

• 리뷰가 누락되어서 다시 남깁니다.
• 저는 디폴트 값인 경우에도 종종 명시해주는 경우가 있는데, 이는 디폴트 값을 모르는 사용자도 쉽게 코드를 이해하고 사용할 수 있게 하기 위함입니다. 해당 헤더 값도 명시해주면 사용자가 더 쉽게 이해하고 활용할 수 있을 것 같아 이를 포함했습니다.

[gahyuun]

오빠가 이번에 fetch-ax에 올린 PR이 머지된 이후에 fetch-ax 버전 업데이트가 필요할 것 같은데 이 부분은 제가 작업하면서 수정하겠습니다~

[yougyung]

코드량의 변화가 크네요.. 이에 에러핸들링 기능이 큰 몫을 한 것 같아요 수고하셨습니다. 👍
실제 fetch교체를 적용하니 개발중에는 고려하지 못했던 부분들을 확인할 수 있네요.

제안해주셨던 의견들에 대해서 개발을 진행하는 것에 찬성하는 입장입니다. 빈번하게 발생할 케이스라고 생각이 들어요. 2번의 경우에는 responseType을 지정한 경우에도 에러가 발생한 적이 있나요?

[yougyung]

axios 문법을 띄는 fetch-ax를 사용하는 만큼 data 속성명을 사용하면 개발 의도에 더 긴밀해질 수 있을 것 같은데, 속성명 교체에 대해 팀원분들은 어떻게 생각하시나요?

[seonghunYang]

저도 고려해봤는데, fetch-ax의 응답값 속성 이름이 'data'라서 겹치더라고요. 그리고 타입명도 저희가 'Body'로 설정해놔서 'body' 자체도 저는 괜찮은 것 같습니다.

[github-actions]

🚀storybook: https://65ccb85d5afe55a024495bc0-ftckjyrnga.chromatic.com/

[github-actions]

🚀storybook: https://65ccb85d5afe55a024495bc0-avcklafvre.chromatic.com/

[seonghunYang]

고생하셨습니다!

1. next 확장하는 속성을 사용하지 못하는 건 큰 에러네요,, 발견해주셔서 감사합니다
2. 개발 생산성이 향상되는 것에 동의하며 기능 추가가 되면 좋을 것 같습니다
3. 이 부분은 어차피 404 에러가 날거여서 크게 고려하지 못했네요 이 부분도 해결이 필요할 것 같습니다!

그리구 스토리북 크로마틱 실패뜨는거 확인 부탁드려요!

크로마틱 수정했습니다! 에러 코드 관련으로 깨지는 거라 임시방편으로 수정한 것이긴 한데, 좀 더 제대로 된 수정은 에러 코드 작업과 병행해야 할 것 같습니다.

[seonghunYang]

코드량의 변화가 크네요.. 이에 에러핸들링 기능이 큰 몫을 한 것 같아요 수고하셨습니다. 👍 실제 fetch교체를 적용하니 개발중에는 고려하지 못했던 부분들을 확인할 수 있네요.

제안해주셨던 의견들에 대해서 개발을 진행하는 것에 찬성하는 입장입니다. 빈번하게 발생할 케이스라고 생각이 들어요. 2번의 경우에는 responseType을 지정한 경우에도 에러가 발생한 적이 있나요?

responseType을 맞게 지정한 경우 에러가 발생한 적은 없습니다