Migration Fetch ax in my page

[gahyuun]

📌 작업 내용

구현 내용 및 작업 했던 내역

• my page에 fetch로 작업한 API를 fetch-ax로 교체했습니다
• content-type이 설정이 되지 않은 오류를 해결했습니다.

🤔 고민 했던 부분

• fetch-ax로 적용을 했는데 많은 line이 줄어든게 보이네요!!
• 과목 추가,과목 삭제, 과목 조회는 별다른 Custom Error가 존재하지 않기 때문에 에러 코드에 따른 분기처리는 따로 진행하지 않았습니다.

🔊 도움이 필요한 부분

• fetch-ax를 사용하면서 여러 문제점들을 발견했습니다.

1. requestInterceptor에서 Headers에 설정을 했을 때 Headers가 적절히 설정이 되지 않은 오류 발생

await instance.post(
      API_PATH.takenLectures,
      { lectureId },
      {
        responseType: 'text',
      },

위 코드와 같이 instance를 사용해서 과목 추가 api 요청을 진행했습니다. 이때 Content-Type은 instance에서 설정을 했기 때문에 별도의 설정을 하지 않았는데 요청 시 415 Error가 발생했고 확인을 해보니 instance의 requestInterceptor에 headers가 제대로 설정이 되지 않은 것을 확인했습니다. 정확한 원인을 파악하지는 못했으나, config.headers를 일반 객체가 아닌 Headers 객체로 변경이 되었을때 해결이 됐습니다. 혹시 이 원인 아시나요 ,,
이 부분에 대해서는 responseType, Invalid URL Error 해결하면서 같이 파악해보겠습니다. 🥹
Myongji-Graduate/fetch-ax#19

[github-actions]

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

[seonghunYang]

• 수고하셨습니다. Header 문제는 HeaderInit 타입에 Headers, Record, string[][] 모두 포함되어 있어 발생하는 것 같습니다. 이 문제의 본질과 해결 방법에 대해 고민해봤는데요, 우리 로직상 RequestInterceptor에서 header가 반드시 Headers 타입이라면 Headers 타입으로 보내주도록 수정하는 것이 문제를 예방하는 데 효과적일 것 같습니다.

[gahyuun]

• 수고하셨습니다. Header 문제는 HeaderInit 타입에 Headers, Record, string[][] 모두 포함되어 있어 발생하는 것 같습니다. 이 문제의 본질과 해결 방법에 대해 고민해봤는데요, 우리 로직상 RequestInterceptor에서 header가 반드시 Headers 타입이라면 Headers 타입으로 보내주도록 수정하는 것이 문제를 예방하는 데 효과적일 것 같습니다.

그러면 오빠는 fetch-ax 코드를 수정하지 않고 지금처럼 requestInterceptor만을 수정하는게 낫다고 말씀하시는게 맞을까요!?
저는 fetch-ax 코드를 수정해야한다고 생각합니다..!

// 졸부 코드
  requestInterceptor: (config) => {
 // 
    return config;
  },
// fetch-ax 코드 
  const requestHeaders = new Headers();
  if (defaultOptions?.headers) {
    new Headers(defaultOptions.headers).forEach((value, key) => {
      requestHeaders.set(key, value);
    });
  }
  if (requestInit?.headers) {
    new Headers(requestInit.headers).forEach((value, key) => {
      requestHeaders.set(key, value);
    });
  }

requestInterceptor에서 매개변수로 넘겨주는 config의 header는 Headers 타입이므로 ( type HeadersInit = [string, string][] | Record<string, string> | Headers; ) 일반 객체처럼 구조 분해할당을 했을 때 문제가 발생했습니다

그런데 사용자 입장에서는 instance시 default option으로 Record<string,string> 타입으로 설정했으면 config.headers로 Headers 타입이 아닌 Record<string,string> 타입으로 예상하지 않을까 싶습니다. 사실 저도 그렇게 예상했고 무작정 415 Error가 떴으니까,, fetch-ax 코드를 까보고 알아내는데 시간이 걸렸습니다.

결론적으로 저는 사용자가 default Options 시 설정했던 헤더의 타입 기반으로 config.headers에 넘겨주는게 좋지 않을까 싶은데 이 부분에 대해서 어떻게 생각하시나요?!
ex)
Record<string,string> 타입으로 header 설정했다면 config.headers의 타입은 Record<string,string>
Headers 타입으로 header 설정했다면 config.headers의 타입은 Headers

[seonghunYang]

config.headers

음, 전달 과정에서 오해가 있었던 것 같습니다. 저도 fetch-ax를 수정해야 한다는 입장입니다. 다만, 제가 앞서 말씀드린 것은 fetch-ax의 현재 로직을 수정하지 않을 경우에 대한 것입니다. 현재 로직상으로 보면 RequestInterceptor에서 config의 header 타입이 반드시 Headers인데, 현재 HeaderInit으로 되어 있어 문제가 발생하는 것이라고 생각해 일단 타입을 수정하는 것이 가장 간단한 해결책이라고 생각되어 말씀드렸습니다.

저도 가능하다면 default Options 설정 시 지정한 헤더의 타입을 기반으로 config.headers를 넘겨주는 게 좋다고 생각합니다. 다만, 인스턴스 생성 시 헤더 설정 타입과 요청 호출 시 헤더 타입이 다른 경우에는 어떻게 해야 할까요? 이런 고민 때문에 Headers 타입으로 통일했다고 생각했습니다. 따라서 이러한 문제 때문에 인터셉터에서는 Header의 타입을 Headers로 고정하는 것이 적절한 방법이라고 판단하여 의견을 제시했습니다. 로직 상에서 분리가 가능하다면 사용자가 지정한 헤더의 타입을 기반으로 config.headers를 넘겨주는 것도 저는 좋다고 생각합니다.

[yougyung]

수고 많으셨습니다!
먼저 관련 레퍼런스&테스트를 진행하다 리뷰가 늦어지게 되어 죄송합니다ㅠㅠ
저도 fetch-ax의 수정이 필요할 것으로 예상되는데요. new Header를 통한 생성보다는 { Authorization: Bearer ...}; 형식을 통해 header를 생성하는 것이 일반적이라고 생각해 가능한 HeadersInit을 유지하며 로직을 변경하는 방향으로 고민을 해봤습니다. axios도 [key: string]: any;type을 취급하고 있었고요.
test를 해보니 일반 객체와 같이 전달한 경우에 header가 아래와 같이 전달되고 있었고,

{
  Authorization: 'Bearer ',
  [Symbol(map)]: [Object: null prototype] {
  'content-type': [ 'application/json' ],
  ab: [ 's' ]
}
}

new Header를 통해 생성한 경우 아래와 같이 전달되고 있었습니다.

Headers {
    [Symbol(map)]: [Object: null prototype] {
    'content-type': [ 'application/json' ],
    ab: [ 's' ],
    Authorization: [ 'Bearer ' ]
  }
}

그래서 requestInterceptor함수를 실행한 이후에 다시 new Header를 진행하는 방식을 취해봤는데, 입력 타입에 상관없이 2번 응답을 구할 수 있습니다. 이와 같이 로직을 변경해보는 것은 어떨까요? 아래는 예시로 작성해본 코드입니다.

    requestArgs = requestInit.requestInterceptor(requestArgs);// 기존코드
    //추가된 코드
    const headers = new Headers(requestArgs.headers);
    headers.forEach((value, key) => {
      requestHeaders.set(key, value);
    });
    requestArgs.headers = requestHeaders;

[yougyung]

추가적으로 ‘’로 전달받는 응답에대해서 responseType: 'text',로 사용자가 넣어주는 것이 예측하기 어려울 수 있다는 생각이 들었어요. 해당 케이스가 Post, patch, delete 등에서 빈번할 것으로 예상되어서 fetch-ax에서 문자열 빈값에 대한 예외처리를 진행하는 것이 어떨까? 생각이 들었는데 팀원분들의 의견이 궁금해요!

[gahyuun]

이 부분에 대해서는 responseType, Invalid URL Error 해결하면서 같이 파악해보겠습니다. 🥹
PR 코멘트에 나와있듯이 Headers와 같이 수정할 예정입니다~!

[gahyuun]

const headers = new Headers(requestArgs.headers);
    headers.forEach((value, key) => {
      requestHeaders.set(key, value);
    });
    requestArgs.headers = requestHeaders;

말씀하셨던, "requestInterceptor이후 위 코드 추가" 에 대한 방법을 적용해보았습니다! 다만 편리성을 위해 fetch-ax에 적용하는게 아닌 interceptor에 적용을 하여서 Content-Type이 잘 존재하는지만 체크를 하였습니다.

export const instance = fetchAX.create({
  headers: {
    'Content-Type': 'application/json',
  },
  responseRejectedInterceptor: (error) => {
    fetchAxErrorHandler(error);
  },
  requestInterceptor: (config) => {
    const accessToken = cookies().get('accessToken')?.value;

    config.headers = {
      ...config.headers,
      Authorization: `Bearer ${accessToken}`,
    };
    const requestHeaders = new Headers();
    const headers = new Headers(config.headers);
    headers.forEach((value, key) => {
      requestHeaders.set(key, value);
    });

    console.log(requestHeaders.get('Content-Type'));
    return config;
  },
});

config.headers는 일반 객체 변형으로 인해 Content-Type이 날라간 상태이고 해당 config.headers를 기반으로 new Headers를 진행하고 requestHeaders에 set을 해보았습니다. 하지만 이러한 형태에도 requestHeaders.get('Content-Type') 시 null이 나오는 상황인데 혹시 제가 테스트를 잘못 진행했을까요?

[gahyuun]

@seonghunYang @yougyung
일단 제가 생각한 결론은 "config.headers를 Headers 타입으로 강제하기" 입니다

Headers 타입으로 header 설정했다면 config.headers의 타입은 Headers 처럼 사용자가 설정한 헤더 타입으로 매개변수를 지정하는 것입니다
다른 인터페이스를 참고했을때

1. axios
   위에서 언니가 axios는 [key: string]: any 해당 타입으로 넘겨준다고 했는데, axios는 XMLHttpRequest 기반이고 XMLHttpRequest에서 setRequestHeader 함수를 사용하여 Headers 객체를 사용하는게 아닌 직접 설정하는 것으로 알고 있습니다.
2. ofetch
   ofetch는 Headers 타입으로 강제하는 것 같습니다. 따라서 ofetch onRequest에서 headers에 일반 객체를 설정할 수 없습니다.

이렇게 Headers 타입으로 강제하게 되면 코드 구현에서 바로 오류가 생기니까 예상치 못한 결과 발생을 줄일 수 있기 때문에 해당 방식이 가장 적합하다고 생각했습니다.
물론 제가 처음에 제안한 방식인 사용자가 설정한 헤더 타입으로 매개변수를 넘겨주는게 더 유연하지 않을까 생각했습니다.
하지만 오빠가 말한 인스턴스 생성 시 헤더 설정 타입과 요청 호출 시 헤더 타입이 다른 경우에 대해서 호출부 타입이 인스턴스 타입을 덮어씌우도록 구현하려고 했는데, 사용자 입장에서 충분히 예상 가능하다고 확신이 들지 않고 유연성보다 해당 단점이 더 크게 느껴져 Headers 타입으로 강제하는 방식을 생각했습니다. 다른 분들은 어떻게 생각하시나용?

위 방식대로 진행하게 되면
fetch-ax에서 현재 config.headers의 HeadersInit 타입을 Headers 타입으로 수정을 진행할 것 같습니다~!

[yougyung]

config.headers는 일반 객체 변형으로 인해 Content-Type이 날라간 상태이고 해당 config.headers를 기반으로 new Headers를 진행하고 requestHeaders에 set을 해보았습니다. 하지만 이러한 형태에도 requestHeaders.get('Content-Type') 시 null이 나오는 상황인데 혹시 제가 테스트를 잘못 진행했을까요?

제가 작업한 환경과 조금 다른 점은 요청객체requestHeaders생성 시, 기존 headers가 반영되어져있지않은 것 같습니다! 아래 코드로 테스트를 진행해보시면 출력되는 것을 확인할 수 있습니다!

더불어 제안해주신 결론 중에, 개발과정에서 잠재적오류파악이 용이하다는 점은 Headers타입만이 갖는 장점이라고 생각해요! 반면 axios와 비슷한 인터페이스를 취하지못한다는 점을 단점으로 갖는다고 생각합니다. 위 방식으로 만일 문제가 해결된다고 해도 여전히 해당 결론이 적합하다고 생각하시는지 의견 궁금해요.

  requestInterceptor: (config) => {
    const accessToken = cookies().get('accessToken')?.value;
    const requestHeaders = new Headers(config.headers);
    config.headers = {
      ...config.headers,
      Authorization: `Bearer ${accessToken}`,
    };
    const headers = new Headers(config.headers);
    headers.forEach((value, key) => {
      requestHeaders.set(key, value);
    });

    console.log(requestHeaders.get('Content-Type'));
    return config;
  },

[gahyuun]

config.headers는 일반 객체 변형으로 인해 Content-Type이 날라간 상태이고 해당 config.headers를 기반으로 new Headers를 진행하고 requestHeaders에 set을 해보았습니다. 하지만 이러한 형태에도 requestHeaders.get('Content-Type') 시 null이 나오는 상황인데 혹시 제가 테스트를 잘못 진행했을까요?

제가 작업한 환경과 조금 다른 점은 요청객체requestHeaders생성 시, 기존 headers가 반영되어져있지않은 것 같습니다! 아래 코드로 테스트를 진행해보시면 출력되는 것을 확인할 수 있습니다!

더불어 제안해주신 결론 중에, 개발과정에서 잠재적오류파악이 용이하다는 점은 Headers타입만이 갖는 장점이라고 생각해요! 반면 axios와 비슷한 인터페이스를 취하지못한다는 점을 단점으로 갖는다고 생각합니다. 위 방식으로 만일 문제가 해결된다고 해도 여전히 해당 결론이 적합하다고 생각하시는지 의견 궁금해요.

  requestInterceptor: (config) => {
    const accessToken = cookies().get('accessToken')?.value;
    const requestHeaders = new Headers(config.headers);
    config.headers = {
      ...config.headers,
      Authorization: `Bearer ${accessToken}`,
    };
    const headers = new Headers(config.headers);
    headers.forEach((value, key) => {
      requestHeaders.set(key, value);
    });

    console.log(requestHeaders.get('Content-Type'));
    return config;
  },

말씀해주신 코드 잘 돌아가는 거 확인했습니다!
하지만 저는 config.headers의 타입을 Headers 타입이나 Record<string,string> 타입으로 강제하는 것이 더 적절하지 않을까? 라는 생각을 했어요
말씀하신 해결 방안은 "Headers가 구조 분해 할당이 안되는 문제를 해결" 해준다고 생각하는데, 본질적으로는 "config.headers가 어떤 타입인지 명시해주지 않은 것"이 문제이고 타입을 명시함으로써 사용자가 객체에 맞게 사용하도록 하는 게 더 좋지 않을까 생각합니다
또한 Headers 타입으로 강제하게 되면 axios와 비슷한 인터페이스를 취하지 못하는 단점이 있다고 하셨는데 XMLHttpRequest에는 Headers 객체를 사용하지 않는 점을 고려하면 어쩔 수 없는 부분이기도 하지만 이런 점을 고려하면 config.headers를 Record<string,string> 타입으로 강제하는 게 더 나을 것 같네요

[yougyung]

config.headers의 타입 범주가 넓고, 특히 Headers와 일반객체의 각 내부 속성이 혼용되지 않는다는 차원에서 입출력타입 모두 interceptor내에서 만큼은 타입 강제가 필요하겠네요. 하나로 통일하는 목적만 충족시키면 되지만, 저도 이용이 자유로운 Record<string,string>타입이 더욱 적절할 것으로 예상되요!