[박근백] fetch 라이브러리

Geunbaek commented 1 month ago

프로젝트 이름

fetch 라이브러리

개요

fetch를 조금 더 편하게 사용할 수 있는 래퍼: post, get 등으로 명시. get(url).json(), post(url).json() 등과 같이 타입을 지원하는 json 메서드.

저장소 주소

https://github.com/Geunbaek/my-fetch

npm 주소

퍼블리싱이 완료된 후 이슈를 업데이트하여 npm 주소를 남겨주세요

체크리스트

[x] 개인 저장소 생성
[x] README.md 파일 작성 (다음 항목 포함)
- [x] 프로젝트 개요
- [x] 설치 방법 / 사용 방법
- [x] 라이선스 표기
[x] 기능 구현
[x] 테스트 추가 (라인 커버리지 60% 이상)
[ ] 사용 예제 프로젝트 추가 (examples 폴더 아래에 2개 이상)
[ ] npm 퍼블리싱

taggon commented 1 month ago

헤더를 전달할 방법 등도 고려되면 좋을 것 같습니다. jQuery에 있는 $.ajax 또는 axios 등의 API를 보면 도입할만한 부분이 있을 것입니다. 👍

taggon commented 1 month ago

👍 msw를 사용한 테스트: 실제 사용과 가까워서 좋습니다.
테스트 설명은 "무엇을 테스트한다"고 쓰는 게 아니라 "이런 동작을 기대한다"고 쓰는 게 좋습니다. 근백님이 작성한 모듈을 아무런 사전 지식이 없는 개발자가 보고도 이해할 수 있도록이요. 예를 들어, request with body가 아니라 body 옵션은 전달한 그대로 서버에 전달된다 정도로 하면 어떨까요? 서버에서 400 에러를 반환하면 status가 400인 FetchIoError가 던져진다.도 한 가지 예입니다.
GET/POST 테스트에 query string에 대한 테스트가 없는데 필요하지 않을까요?
POST는 encType을 설정할 수 있나요?
POST/PUT/PATCH의 body는 객체만 전달할 수 있는 걸까요?
응답 데이터는 json만 가능한가요? 서버에서 plain text 타입을 반환하면 어떻게 되나요?
formatParams는 URLSearchParams를 사용해서 재작성 해보세요.
싱글톤으로 작성한 이유가 있나요? 클래스로 작성한 이유는 뭘까요?
추상 클래스는 어떻게 활용할 계획이었을까요?
README 설명이 불친절합니다. 예를 들어 options에는 어떤 값이 포함되나요?
다른 파일과 공유하는 것도 아닌 듯 한데, 타입을 굳이 별도 파일로 분리한 이유가 있을까요?

Geunbaek commented 1 month ago

@taggon 피드백 정말 감사합니다 !

테스트 설명은 "무엇을 테스트한다"고 쓰는 게 아니라 "이런 동작을 기대한다"고 쓰는 게 좋습니다. 근백님이 작성한 모듈을 아무런 사전 지식이 없는 개발자가 보고도 이해할 수 있도록이요. 예를 들어, request with body가 아니라 body 옵션은 전달한 그대로 서버에 전달된다 정도로 하면 어떨까요? 서버에서 400 에러를 반환하면 status가 400인 FetchIoError가 던져진다.도 한 가지 예입니다.

최대한 반영해서 다시 작성 후 다시 피드백 요청 드리겠습니다.

GET/POST 테스트에 query string에 대한 테스트가 없는데 필요하지 않을까요?

주로 GET 메서드에서 query string 이 사용된다고 생각하여 GET 메서드에만 쿼리 스트링에 대한 테스트를 작성했는데 다른 가능한 테스트들를 모두 작성하는 것이 좋을것 같습니다 !

POST는 encType을 설정할 수 있나요? POST/PUT/PATCH의 body는 객체만 전달할 수 있는 걸까요?

header의 속성으로 추가 하는 것만 생각하고 body 로 들어가는 formData는 고려 못했네요. 감사합니다 ! 이부분도 수정 및 테스트 코드 추가해보겠습니다!

뭔가 코드를 작성할때에는 이것저것 고려한다고 고려한것 같은데 피드백을 받고 보니 너무 관련 자료수집을 안하고 코드를 작성한것 같네요.. 조금 더 보완해 보겠습니다.

응답 데이터는 json만 가능한가요? 서버에서 plain text 타입을 반환하면 어떻게 되나요?

응답데이터는 응답 객체의 contentType 에 따라 다르게 데이터를 변환해서 반환하고 있습니다.

formatParams는 URLSearchParams를 사용해서 재작성 해보세요.

네! 감사합니다.

싱글톤으로 작성한 이유가 있나요? 클래스로 작성한 이유는 뭘까요?

제가 라이브러리를 만들때 가장 많이 참고했던 라이브러리가 axios 입니다. 최대한 axios와 비슷한 문법으로 작성하여 사용자들이 쉽게 접근할수 있도록 하려고 했습니다. 그러다 보니 싱글톤으로 작성하게 되었습니다.

클래스로 작성한 이유는 따로 없는 것 같습니다. 어떤 객체가 필드를 가지고, 메서드를 가지는 경우를 주로 클래스로 자주 작성을 해왔기에 습관적으로 클래스로 작성했던 것 같습니다.

javascript 에서는 클래스로 구현하는 것과 함수로 구현하는 것에 있서 상속과 같은 것을 제외한다면 동일하게 동작하도록 작성할 수 있을 것 같은데 혹시 태곤님은 어떠한 기준으로 이걸 나누는지 말씀해주실 수 있나요?

추상 클래스는 어떻게 활용할 계획이었을까요?

추상 클래스는 interface 와 비슷한 역할로 생각하고 추상클래스를 사용했습니다. 그런데 태곤님의 피드백을 보고 찾아보니 다른 역할을 하는 것 같더라구요. 이부분은 좀 더 학습해보겠습니다 !

README 설명이 불친절합니다.

보완해 보겠습니다 !

다른 파일과 공유하는 것도 아닌 듯 한데, 타입을 굳이 별도 파일로 분리한 이유가 있을까요?

이건 지난번에도 비슷한 피드백을 받은 것 같은데요. 하나의 파일 내에서만 쓰이더라도 각각을 다 분리하는 습관이 있더라구요. 과거에 하나의 파일에 몰아뒀다가 다른 파일에서 사용하는 경우가 있더라도 그걸 작성해둔것을 미처 생각하지 못하고 다시 작성했던 경험이 있는데 그 경험에 의한 것인 것 같습니다.

혹시 태곤님은 하나의 파일 내에서 쓰이던 함수가 다른곳에서 쓰일때 어떤식으로 리팩토링하시나요?

taggon commented 1 month ago

README 파일에 설명이 부족합니다. 각 옵션이 어떤 역할을 하는 것인지 기술되어 있지 않습니다.
테스트에서도 옵션을 테스트하고 있지 않습니다. 메서드별로만 테스트가 작성되어 있고 예를 들어 타임아웃 동작조차 테스트가 되어 있지 않습니다. 테스트를 더 보강하셔야 합니다.
JSDoc 형태로 fetchIo의 메서드와 옵션에 설명을 추가해보는 건 어떨까요
GET 요청에도 body를 방지하지 않고 있는데 의도한 걸까요?
try - catch 범위는 딱 필요한 만큼만 잡으시면 디버깅에 유리합니다. 파라미터와 body를 작성하는 부분도 try-catch가 필요할까요?
에러 처리 부분을 보면 모든 에러가 FetchIoError 형식을 따른다고 가정하고 있는데 실제로는 그렇지 않습니다. 아래 URL에서 보듯이 fetch에서 발생할 수 있는 예외만 해도 많이 다릅니다. https://developer.mozilla.org/en-US/docs/Web/API/Window/fetch#exceptions FetchIoError는 타입이 아니라 Error를 상속한 클래스로 작성하고 instanceof를 사용해서 구분하여 처리해야 합니다. 다른 건 몰라도 에러를 plain object로 처리하면 안되요.
fetchIo의 메서드들은 currying하면 더 단순하게 만들 수 있지 않을까요? 아래와 같은 느낌으로 사용하면 될 것 같습니다.
```
get: requestWith('get'), 
...
```

FC-InnerCircle / fe1-library