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

Discussion: use with fetch API #341

Closed
jamesarosen opened this issue Nov 8, 2022 · 5 comments
Closed

Discussion: use with fetch API #341

jamesarosen opened this issue Nov 8, 2022 · 5 comments

Comments

@jamesarosen
Copy link

jamesarosen commented Nov 8, 2022

Environment

Node version: 18.x
typed-rest-client version: 1.8.11

Issue Description

Popular libraries like make-fetch-happen and cache-control-fetch rely on the fetch API. This API is new in Node 18, but has good polyfill support through node-fetch or minipass-fetch.

It is currently possible to override RestClient.client.requestRawWithCallback to use fetch, but the method is complex and the interface between fetch/Request/Response and http/ClientRequest/IncomingMessage is nuanced. I'd love to see a standard (and well-tested!) community solution for this so we can all get the implementation right.

@jamesarosen
Copy link
Author

In the meantime, scarlett is a possible alternative for folks who want a typed fetch-based REST library.

@DmitriiBobreshev
Copy link
Contributor

DmitriiBobreshev commented Nov 9, 2022

Hi @jamesarosen, thank's for the suggestions, we will take a look what is the best way to implement the solution when we will have enough capacity, but any thoughts, discussions and PRs are welcome!

@jamesarosen
Copy link
Author

jamesarosen commented Nov 9, 2022

I think the simplest thing would be to add an optional FetchBasedHttpClient to this package. Clients would opt into it with:

import { RestClient, FetchBasedHttpClient } from "typed-rest-client";

const myApi = new RestClient({ client: new FetchBasedHttpClient() });

That would require adding an optional package dependency on either node-fetch or minipass-fetch. The RestClient constructor doesn't currently accept a client, but it certainly could, especially since client is part of the class's public API.

Another alternative would be a separate package like microsoft/typed-fetch-client. That seems like more overhead than is necessary.

@github-actions
Copy link

github-actions bot commented Feb 7, 2023

This issue has had no activity in 90 days. Please comment if it is not actually stale

@github-actions
Copy link

This issue has had no activity in 90 days. Please comment if it is not actually stale

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

No branches or pull requests

3 participants