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

feat: drop uneeded polyfills #163

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

talentlessguy
Copy link

Removes polyfills that are no longer needed.

AbortController has been supported since Node v14, which reached it's EOL in 2021.

fetch API has been supported since Node v18, which will reach it's EOL in 30.04.2025 and has been there for 3 years, so it is safe to drop the dependency.

@ospfranco
Copy link
Collaborator

This needs to work in other environments other than node. Like rn. Did you test it there?

@talentlessguy
Copy link
Author

talentlessguy commented Jan 19, 2025

I haven't tested manually but fetch and AC are both supported in React Native

see: https://reactnative.dev/docs/network

All edge runtimes (such as Cloudflare, AWS and Vercel Edge) support both as well

@talentlessguy
Copy link
Author

If needed I can manually test on RN, though I don't think it's required, given that fetch is officially supported in RN

@ospfranco
Copy link
Collaborator

I will keep this open... tbh there is no harm in having the polyfills is there?

@talentlessguy
Copy link
Author

@ospfranco the harm is unnecessary bloat, duplication (polyfills are usually loaded by a bundler already, and other libraries also load fetch polyfill of a different version) and increased supply chain risks

@ospfranco
Copy link
Collaborator

I don't know... someone added the polyfill for the abort controller because AWS lambdas... or cloudflare... don't remember anymore, it was annoying enough to get it right the first time. I'll leave the PR open, if enough people vote it up then I'll merge it.

@talentlessguy
Copy link
Author

@ospfranco didn't mean to sound rude or anything, based on my knowledge all platforms support both fetch and AC now. Either way that person should have enabled AC polyfill in their own bundle, not in the library, but that's my personal opinion.

Speaking of AC support in lambdas,

AWS Lambda supports it since v3 (most used version), see https://aws.amazon.com/blogs/developer/abortcontroller-in-modular-aws-sdk-for-javascript, the blog post is from 2020

Cloudflare supports it as well, see https://developers.cloudflare.com/workers/runtime-apis/web-standards/#abortcontroller-and-abortsignal

hopefully that helps

@ospfranco
Copy link
Collaborator

Didn't take it the wrong way. I just don't want to add more work on my plate for something that is working. Thanks for the PR!

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