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

chore: don't bundle undici #3475

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

benmccann
Copy link
Contributor

undici is already provided by Node.js, so we don't need to bundle our own version of it. Use the node-native-fetch library, which takes care of this and has battle-tested proxy-handling code.

before:
2107kB dist/index.js
62484 lines

after:
1812kB dist/index.js
36063 lines

@peter-evans
Copy link
Owner

@benmccann Thank you! This is great.

I'm wondering if I still need const proxy = getProxyForUrl(options.baseUrl) for the custom proxy options. It looks like node-fetch-native supports parsing the proxy env vars. Perhaps I can get away with just using fetch instead of createFetch.

@peter-evans
Copy link
Owner

Confirmed this passes the test suite: #78 (comment)

@peter-evans peter-evans merged commit 2aadff0 into peter-evans:main Nov 4, 2024
5 checks passed
@peter-evans
Copy link
Owner

Nice! 🙌

image

#3478

@peter-evans
Copy link
Owner

I'm wondering if I still need const proxy = getProxyForUrl(options.baseUrl) for the custom proxy options. It looks like node-fetch-native supports parsing the proxy env vars. Perhaps I can get away with just using fetch instead of createFetch.

Seem to work and pass my proxy tests: #3483

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.

2 participants