Open talentlessguy opened 2 months ago
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?
@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.
Do you have an actual issue with the current i18next-http-backend version? Do you need to have this merged soon? Because of what?
@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.
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 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...
@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.
will come back to this in 6-7 months...
@adrai thanks for reaching out and taking time to look at the PR! I'll ping in 6 months in case of anything
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 supportsfetch
natively since 18.x. I've added anengines.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
npm run test
Checklist (for documentation change)