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

Feature/use fetch #40

Closed
wants to merge 3 commits into from
Closed

Conversation

mmailaender
Copy link

Ticket(s): 36

Problem

Axios has limited platform support.

Solution

Replace it with fetch()

With native fetch(), we can support almost all JS runtimes and platforms.

  WebAPI Node 18 Node 17> Deno Bun
Browser fetch() N/A N/A N/A N/A
Cloudflare Workers fetch() N/A N/A TODO N/A
Vercel Edge (Cloudflare Workers) fetch() N/A N/A TODO N/A
Vercel Serverless functions N/A fetch() Isomorphic Unfetch / Cross-fetch fetch() fetch()
Cloud Functions N/A fetch() Isomorphic Unfetch / Cross-fetch fetch() fetch()
Azure Functions N/A fetch() Isomorphic Unfetch / Cross-fetch fetch() fetch()
AWS Lambda N/A fetch() Isomorphic Unfetch / Cross-fetch fetch() fetch()
Netlify Functions (AWS Lambda) N/A fetch() Isomorphic Unfetch / Cross-fetch fetch() fetch()
Web Assembly TODO TODO TODO TODO TODO

Except Node and Bun I see atm some downsides:

  1. The new fetch() native implementation available from 17.5 is not yet supporting HTTP/2
  2. Bun is also not yet supporting HTTP/2
  3. Node 16.x has no native fetch() support, and will be maintained until 2023-09-11. Taking your requirement into consideration @ptpaterson to support it, the question that I see is if it could be a temporary solution with one of the following options:
    a. forked Cross-Fetch version*,
    b. Isomorphic Unfetch*,
    c. dedicated node-fetch package*,
    d. dedicated Axios package*,
    e. ...?

* all of them lack support for HTTP/2

Progress:

  • Replace Axios with Fetch
  • Rewrite Tests to work with Fetch
  • With fetch, we have specified HTTP codes for mocks. The codes need to be verified if they match the server specification.
  • Test with HTTP/2
  • Run all tests on all platforms and JS runtimes
  • [] Test Isomorphic Unfetch
  • Test Cross-Fetch

Result

Out of scope

Testing

@cleve-fauna
Copy link
Contributor

@mmailaender Thanks for the patch here and the great feedback.

Another requirement we have is to support http/2. Unfortunately, from what I understand node-fetch (which underlies cross-fetch in node) does not support http/2 node-fetch/node-fetch#342.

Thus, we are working internally to branch between fetch and http/2 depending on the context of usage or the user's configuration.

What we will aim for here is:

  1. Sane zero-touch defaults. If we detect your app is best with fetch() we'll default to that without user configuration required. If we detect your app is best with http2 we will go with that - TBD on what exact library that will be - don't want to pen any commitments here :).
  2. Minimize dependencies we force onto the user's. Axios clearly suffers from this. Correct this error.
  3. Ability for user's to configure to the http layer of their choice if they do not want the defaults.

We are pursuing that development work inside Fauna now. So expect to see PRs with this motion in mind in the coming weeks.

That being said, if you need this commit here to start using the language in Cloudflare or another platform please keep this branch open and run with it!

Thanks so much for your feedback!

@cleve-fauna
Copy link
Contributor

Hey @mmailaender we just merged #66 which replaces the driver's default HTTPClient with fetch from Axios.

As such, I'm going to close this PR.

Please give it a go and let us know what you think!

Thanks again for being engaged and taking the initiative on this.

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

Successfully merging this pull request may close these issues.

3 participants