feat!: Custom fetch support, remove node-fetch, ESM+CJS dual build, migrate to vitest, TS fixes, test improvements #162
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Apologies for this cursed PR - it snowballed from trying to get the fix added by #161 to typecheck.
TL;DR changes in this PR:
fetchimplementation@ts-ignoresnode-http only supports
signalin v3, which only supports ESM. Instead of bothering with node-http breaking changes and incompatibility between node-http and built-in fetch types, I removed it altogether in favour of built-infetch. This also simplifies tests by explicitly passing in a mockedfetchimplementation and not needing to mock globals, which doesn't play nice with TS.By depending on built-in fetch and migrating to ESM, we're now requiring Node >= 18, which is enforced in
enginesinpackage.json. The bulk of the changes to imports are for making file extensions explicit, which are mandated by ESM.Building CJS in addition to ESM lets us continue supporting use of
require, which we're still using in our docs. The dual-build ESM+CJS approach is the same that https://github.com/auth0/node-auth0 is using, which I used as a reference.There are no user-facing API breaking changes in this PR, but the removal of node-fetch and requirement of Node >= 18 might warrant a major version bump. I would like to release this as a v4 beta and get some real-world testing.
There's 1 minor bugfix in
sdk/polling_manager.ts. We were fetching the environment document twice immediately on startup. This extra call has been removed.We're also now down to just 1
@ts-ignorein the whole project.