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

BREAKING CHANGE: drop support for XMLHttpRequest and remove cross-fetch #144

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

Conversation

talentlessguy
Copy link

@talentlessguy talentlessguy commented Jun 28, 2024

This PR removes support for XMLHttpRequest as it is not used widely anymore for fetching things. Fetch API no longer requires a polyfill on Node.js, and is supported by all major browsers and runtimes.

See compatibility table: https://developer.mozilla.org/en-US/docs/Web/API/fetch#browser_compatibility
and: https://caniuse.com/fetch

For environments that don't support fetch (such as IE11) - they should install a fetch polyfill in their projects.

It also removes dependency on cross-fetch as it is not needed anymore. Node.js supports fetch natively since 18.x. I've added an engines.node field to notify that minimum node 18 is required.

Since it's a breaking change, in my opinion a semver major bump is required.

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • run tests npm run test
  • tests are included
  • commit message and code follows the Developer's Certification of Origin

Checklist (for documentation change)

  • only relevant documentation part is changed (make a diff before you submit the PR)
  • motivation/reason is provided
  • commit message and code follows the Developer's Certification of Origin

@adrai
Copy link
Member

adrai commented Jun 28, 2024

XMLHttpRequest is only used as fallback... if no fetch is available...
Also cross-fetch is only used if fetch is not available...
At runtime there should be no difference on modern environments of having or not having the XMLHttpRequest and/or cross-fetch "logic" in i18next-http-backend or not...
So what hurts in keeping i18next-http-backend like is it now for a longer while?

@talentlessguy
Copy link
Author

@adrai unnecessary code being downloaded by browsers and package managers. Other packages might also import cross-fetch but with another version, resulting in dependency duplication.

@adrai
Copy link
Member

adrai commented Jun 28, 2024

Do you have an actual issue with the current i18next-http-backend version?
Do you need to have this merged soon? Because of what?

@talentlessguy
Copy link
Author

talentlessguy commented Jun 28, 2024

@adrai there's no urgency of it getting merged, I just submitted a PR as a general ecosystem cleanup. That's done so that modern environments aren't slowed down by unnecessary polyfills.

@adrai
Copy link
Member

adrai commented Jun 28, 2024

My concern is, I know there are users out there having still the need for this fallbacks... if I merge this now, there risk is high there will be more github issues, etc... asking for bringing back support etc...
Would you be ready to respond to those github issues?

@adrai
Copy link
Member

adrai commented Jun 28, 2024

@adrai there's no urgency of it getting merged, I just submitted a PR as a general ecosystem cleanup. That's done so that modern environments aren't slowed down by unnecessary polyfills.

ok, then let's keep this PR open for a while...

@talentlessguy
Copy link
Author

talentlessguy commented Jun 28, 2024

@adrai

  1. in case of fallbacks, they should pin a version to the previous major. This is very simple, just doing a ^major... works. Stuff might break only if it's imported without specifying a version, which is a risk of it's own anyway. Any further bugfixes can be backported to previous major if needed, in case there's an urgent security bug.
  2. I labeled it as "BREAKING CHANGE" specifically for preserving compat, so that other projects who don't need all these fallbacks and polyfills (which are the majoeity) don't have to download/pull the code.
  3. yes, I'm ready to fix it if there's any issues arising with it.

@adrai
Copy link
Member

adrai commented Jun 28, 2024

will come back to this in 6-7 months...

@talentlessguy
Copy link
Author

talentlessguy commented Jun 28, 2024

@adrai thanks for reaching out and taking time to look at the PR! I'll ping in 6 months in case of anything

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants