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
feat!: Upgrade to node-fetch
v3
#617
base: main
Are you sure you want to change the base?
Conversation
Warning: This pull request is touching the following templated files:
|
We'll merge this once we're ready to drop Node 14/16 (which is WIP) |
General question / comment I know that both |
I was a bit torn on this decision; technically it would be experimental, however the API surface hasn't really changed since its introduction. The catch is the Outside from that, internally we would need to use a beta version of nock, which includes unidici support - it seems to work fine in testing. |
Ah I hadn't even considered nock not supporting it. I wonder if by the time we are ready to move to Node 18 nock will be out of the beta phase. |
Created a proof-of-concept for migrating to |
Object.assign(res, {config}); | ||
|
||
// best effort for `node-fetch`; `.data` is not writable... | ||
return new Proxy(res, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's gotta be a cleaner way to do this... hmm. We want to expose all of res
's types so that customers can take advantage of new properties as they're available
Node 18 & 20 pass in this commit 👌🏽: |
Fixes #429
Fixes #508
🦕