crowdin-api-client-js
crowdin-api-client-js copied to clipboard
crowdin
Proposal: switch from axios to undici
Node.js has recently implemented the fetch API into it as an experimental flag (ref: https://github.com/nodejs/node/pull/41749) which makes use of node's undici package and sets fetch and other features from it as global variables. As this is expected to come out officially in node 18 later this year, I recommend that we switch from axios to undici for all fetch requests. If the maintainers agree with this change I would be willing to open the PR that introduces that.
I am not fully sure how safe it would be to switch to fetch in Node. It seems like it's still experimental feature. At least I do see in PR that in order to have fetch globally you need to add --experimental-fetch. And also we already have an option to use fetch. https://github.com/crowdin/crowdin-api-client-js#list-of-projects-with-fetch-api . So maybe let's just keep eye on it and eventually we can fully remove axios and only use fetch.
@yevheniyJ just to clarify, my proposal was to first switch to undici, and then remove the dep altogether and use node's implementation when that comes to stable node. I have noticed, however, that even the undici package uses experimental modules within node that, despite not needing a flag to be activated, they still have a warning saying they can change at any time, so I agree that it's best to keep this here and revisit once node 18 comes around.
Also, since you mentioned the ability to use fetch in the client, I'm pretty sure that's for DOM users only, right? Seeing as there is no other dependency that would only make sense
@ImRodry fetch was introduced to work in plugin-based environment where axios was not suitable. But actually the API signature of fetch in Node is equivalent to the fetch that is currently implemented in the client. So it means that you already can use fetch in Node if fetch is globally declared. And we just need to remove later on axios and leave only fetch.
At the moment, without any experimental flags, fetch is only available in a browser environment, not in Node, so I'd assume that's only usable there, right?
Yes or if you really want to use fetch in Node you then need to turn on that feature on you end and configure client to use fetch.
Node v18 has now been released and fetch is declared globally without any flags. Can we switch to node's version of fetch now and release a new major version along with #137?
Node v18 has now been released and fetch is declared globally without any flags. Can we switch to node's version of fetch now and release a new major version along with #137?
We'll think about this possibility.
May I know why this was closed @andrii-bodnar? Undici’s fetch is noow stable on the latest version of Node 18.
@ImRodry we've recently updated Axios to the latest version and improved Error handling. #226. It's based on the Axios exceptions.
We definitely not going to migrate from Axios in the nearest future. When something changes we will reopen this issue.
That does not answer my question nor does it satisfy the original issue here. The goal here is to remove the dependency in favor of the built-in library that we’re all using already. Of course this would require a major version bump but it’s still something you should be doing.