Skip to content
This repository has been archived by the owner on Oct 10, 2024. It is now read-only.

feat: update fetch configuration logic #66

Conversation

sublimator
Copy link
Contributor

@sublimator sublimator commented Apr 30, 2024

Squashed version of #65 which was an exploration branch for a new version, including non-relevant commits

Closes:

Update fetch detection logic (See: #63 (comment)):

  • Add client._fetch hook point for tests rather than using a global
  • Use top level await, per here and here (this won't work in commonjs)
  • Use native fetch when available on node
  • Support service workers such as manifest version 3 Web Extensions

// We can't use a top level await if eventually this is to be converted
// to typescript and compiled to commonjs, or similarly using babel.
const configuredFetch = Promise.resolve(
globalThis.fetch ?? import('node-fetch').then((m) => m.default));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some credit to fuegoio here:
#65 (review)

@lerela lerela merged commit 06dabd3 into mistralai:main Apr 30, 2024
3 checks passed
This was referenced May 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using node-fetch makes some Node.js libraries incompatible with this library
3 participants