-
Notifications
You must be signed in to change notification settings - Fork 1
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
[PROP]: Use node-fetch as standard server-side request module #3
Comments
We are already using Axios as the provider of our request.js implementation. The reason that I can remember (that we briefly discussed recently) was that Axios provides a progress interface. I think @codebender828 might have additional comments on this because if I remember correctly he was originally quite strongly on team Axios. |
Fetch, being a native browser successor of the decades-old xhr interface, is not only good for its size (no wrappers needed), features exclusive to it like the cache api and req aborting, but also as of this year now supports req/res low level upload streams, whereas xhr actually needs to buffer the response to memory. Full support is beginning to roll out across all browsers (and this is easy to detect support for and fallback) |
This proposal is also scoped to server-side javascript, which can have different requirements from client side HTTP requesting. The first priority is having a single consistent request interface to use across a single system (so one repo absolutely should not be using multiple request modules), but there can be benefits in developer overhead in also standardizing the request interface across systems as well |
On the topic of request abortingPoint 1. Axios can also do request aborting and we are doing this already in request.js. On the topic of cachePoint 2 As for cache, I'm actually not that familiar with cache policies in fetch. I assume it is doing something more than just modifying the cache-control header? Because ofc on Axios we can also set headers in the request. In addition to this there seems to be some kind of adapter for Axios: However even if the adapter solves all the issues, there seems to be a issue with performance in node which would require us to do some additional configuration:
Question 1: Also, are you certain node-fetch gives the same functionalities as browser fetch?
Question 2: Also in which cases would we need cache for requests in server side code? We are not requesting a lot of static data from the server? On the topic of progressQuestion 3: Do we see ourselves needing progress on the serverside? Others commentsWhichever we end up choosing, maybe we should create a request wrapper on the server side similar to that of webapp. This way we don't have to marry ourselves into any specific implementation |
I mean specifically the Cache API
I said "API-Consistent", which it is. If a developer is trying to use fetch options like "keepalive", "referrer", or "credentials" , they most likely don't know what they mean or do.
No, these are responding to its viability on the client. See the original issue text for serverside implementation reasoning. Note: this wasn't about progress though, it was about streams, which is much broader and much more powerful than just general http req progress indicators buffered to memory.
That's a good idea, but we should decide on a module now. I would want to commit to fetch (node-fetch on server) for the reasons above as well as the speed of development happening on the fetch api and the probable deprecation of xhr in the future, or at least the lack of development on it now other than general maintenance. |
Why node-fetch?
Why Not ____?
request
is deprecatedisomorphic fetch
uses node-fetch under the hood, acting more like a client polyfill with ss supportaxios
is a good module as well, but it would need to be implemented on client (11kb) to be API-consistent with client request moduleThe text was updated successfully, but these errors were encountered: