-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Node.js support #184
Comments
@kirill-konshin have you seen the |
Github's polyfill is the most popular and well known, so I thought maybe it is reasonable to make it isomorphic... I personally prefer single library approach so that I won't have to deal with bugs in 2-3 libraries which potentially may not catch up with the spec/bleeding edge implementations :) Your version is not UMD-wrapped (will not work with RequireJS at all) and it is not a one-file dependency as this repo (yours has own sub-dependencies, which means slightly trickier install for non-AMD users and front end developers in general). As I said earlier, those 5 items does not seem to be hard to do. |
You make some really good points that I hadn't thought about before because I don't really use RequireJS. I remember point 5 was considered but rejected because in order not to break backwards compatibility this repo would have to jump to v2.0.0… which didn't make sense… Once you know it's I can't speak for the GitHub team but much of what you're asking for seems to take this project away from what I think it is defined as:- a client side implementation of the whatwg fetch specification and nothing more. I don't think it would be very difficult to make a little Also having used variants of fetch on the server for over half a year in production (including initially one that used the node XMLHttpRequest polyfill, which is abandoned too*) I really must recommend node-fetch. It is far far far more robust and is actively maintained! * Here's a pull request I made to it in January. |
I was talking about https://github.com/pwnall/node-xhr2 which I've been using in production for about a year too :) but anyway, native NodeJS If another isomorphic library uses I think we've got 2 options:
Since they are sequential, I can make a PR for option 1 and then take a look on option 2. |
I think that it's OK to deviate a bit from the original course in order to make someone's life easier :) |
Hi, the request to make this library isomorphic comes in every now and then and we always refuse it. You should have searched past issues, like: #31 (comment) To reiterate our reasons for refusal:
|
Ok, than what about at least making it AMD-compatible? So that I can use the same interface like: var {fetch, Request, Headers, Response} = (typeof window !== 'undefined')
? require('fetch')
: require('node-fetch'); Right now |
Taking into account that currently in Chrome there is no way to track |
Chrome's Network tab displays all requests, including those made with <script>window.fetch = null</script>
<script src="fetch.js"></script> I don't recommend doing this, but it will work. |
The Fetch specification defines window as implementing the Our goal is to faithfully implement the polyfill in accordance with the specification so that it interoperates well with browsers' native fetch implementations. |
Sorry for slightly wrong description, here is what I meant: https://code.google.com/p/chromium/issues/detail?id=457484, Chrome does not show POST bodies as it does for XHR. The solution with deleting As an example, https://github.com/jakearchibald/es6-promise is an AMD compatible module, which can be loaded in scope and will not poison the global scope if not asked to do so. |
@kirill-konshin You can easily obtain a reference to this <script>
originalFetch = window.fetch
window.fetch = null
</script>
<script src="fetch.js"></script>
<script>
realPolyfillFetch9000 = window.fetch
window.fetch = originalFetch
</script> We're not responsible for the Chrome inspector bug and we will not change our code to work around your debbuging troubles. Charles Proxy is a great HTTP debugging tool that works regardless of browser inspector support. |
I am using Charles all the time. All that you said does not mean that you can't take the approach of |
We don't want to maintain modular support for fetch right now because we don't want to support people loading it in a modular fashion just yet (or never). |
Any justification why not? We're talking about ~20 lines of code at the end of file: function polyfill(obj){
if (!obj) obj = self; // fill window if nothing provided
obj.Header = Header;
obj.Body = Body;
obj.Request = Request;
obj.Response = Response;
obj.fetch = fetch;
return obj;
}
if (typeof define === 'function' && define['amd']) {
define(function() {
var res = polyfill({});
res.polyfill = polyfill;
return res;
});
} else {
polyfill();
} |
We don't use AMD at GitHub so we have no incentive to maintain support for AMD. It's just 20 lines of code from your perspective, but for us it means committing to AMD support and maintaining this boilerplate code by updating it with future AMD updates. Also, if we supported AMD then we would have to support similar loaders that people ask us about, which means adding even more boilerplate code or various So you see, from your perspective it makes perfect sense for us to add these 20 lines of code to our project, and from ours it doesn't so much. From our perspective it makes sense to continue maintaining this perfectly fine polyfill that does its job as intended. |
AMD boilerplate has not been changed for last 5 years and I doubt that anything will change in future according to RequireJS author. Almost every other loader is compatible either with CommonJS or with AMD. Just use full UMD declaration (+2 lines of code) instead of short one (which I posted) and you're good to go, this code will never change and will not require any Again, even |
Oh you mean this es6-promise which when adding loader support broke its functionality as a polyfill for months, which prevented us from upgrading it on github.com? |
According to semver it was a backward-incompatible change, this is why it is in 2.0. Stay with 1.0, if you use Bower or NPM it is easy. My proposal is fully backward compatible with what you have now. |
Fair point. However just reiterating on your previous arguments won't necessarily convince us any faster. I'd like to hear what my coworkers think. @dgraham some backup here please? 😅 |
I really doubt that such a small piece functionality, which by the way has value for your consumers, worth such a long discussion. I can make a PR and make sure all existing tests are passing. |
Life is tough ;) |
We receive a couple common requests, so I think it’s useful to summarize why we reject them. I spend lot of time considering these issues, and I don’t feel like I’m any closer to understanding why they recur. Here’s my perspective. BackgroundBefore looking at the requests, let’s cover some background. A polyfill is, by definition, a piece of JavaScript code that mimics a native web browser API that is still in development. It fills in missing functionality until the point where the browser supports the API natively, and then the polyfill is removed. Browser APIs are not modules—they are all attached to the A polyfill is a somewhat unique piece of code, in the sense that it is intended to not be used at all in most browsers. A majority of visitors do not run this fetch polyfill. Their browser natively supports both Polyfills are valuable not because of their reach, but because they allow us to use new browser features before all major browsers have shipped them natively. This is code that is intended to be deleted within 2-3 years, once all browsers have built-in support. Ok, so let’s discuss the two recurring requests. 1. Node.js supportThe dream of Write Once Run Anywhere is being repeated under the banner of “isomorphic” JavaScript. The goal is to support both web browser runtime environments and Node.js server environments with a single piece of code. This is admirable, but having lived this dream once before with Java, it is not a reasonable long-term strategy. It does not work. JavaScript must be optimized for its runtime: either a browser or a server process. Attempting to support both runtimes makes each of them worse. Some concrete examples follow. ComplexityFetch is a very well designed modeling of headers, requests, and responses, using asynchronous promises. This polyfill treats that design as a beautiful layer above an XMLHttpRequest implementation. We use XMLHttpRequest so you don’t have to! But a Node.js server does not have XMLHttpRequest—it doesn’t make sense in a server environment. Node is not constrained by a standards process or several competing implementations. Anyone is free to write an HTTP library for Node.js, and subsequently, there are many great HTTP packages. In order for fetch to support both a browser and a server we must introduce a plugin system where the HTTP subsystem may be swapped out for something else. We must then test the plugin system on all major browsers and then again with all popular HTTP libraries inside a Node.js server. This is complexity that we just don’t have time to support. Lowest common denominator featuresWhen designing for both a browser and a server, features that exist in only one runtime must not be used, and then we end up with something that works poorly in both environments. The bitinn/node-fetch package is a great implementation of a fetch-like API optimized for Node.js. Browsing through the code reveals many non-trivial code changes required to optimize for servers:
This list demonstrates why optimizing for the JavaScript runtime is important and extremely difficult to do in a single codebase. On the server, we get to use some of Node's best features even though web browsers don’t support them. 2. Module loader supportWe are occasionally asked to add support for one of several module loaders. These requests are usually accompanied by dramatic language like “poison the global scope”, meaning this line of code is considered offensive: JavaScript has four different module systems: AMD, Common.js, UMD, and standard ES6 modules. In the absence of a standard module system, the community built several of it own, which is great! For library authors, however, this is a disaster. Each of the competing module formats must be supported and tested with each new release. This is complexity that we are not interested in supporting for a polyfill. It is certainly reasonable to expect a library to support modules, but this isn’t a library. If it were as simple as adding the standardized SummaryWhen receiving these requests, we point people to supported Node.js versions of fetch or try to provide debugging tips related to the user’s browser tools. We won’t be providing direct support for either Node.js or modules. This polyfill provides excellent, well-tested, cross-browser Hopefully this long-winded answer provides context around how we’re thinking about these issues. <3 |
Excellent, @dgraham. Thanks 🙇 |
@dgraham thanks for the perfect explanation. This should be a well-advertised blog article posted everywhere 👍 |
why isn't it clearly documented to
Perhaps you should instead say in the Readme.md of this repo "For use in Node.js projects, use the |
node-fetch |
Do you have anything in your roadmap regarding Node.js support?
The purpose is to allow other cross-env libraries to use
fetch
as the client.From what I see now in
master
there are couple of things to be done:XMLHttpRequest
polyfill (for example https://github.com/pwnall/node-xhr2)fetch()
function instead of always writing to globalpolyfill()
method just like ines6-promise
for AMD usersfetch
, current one is unmaintained for 2 yearsIs it worth investigation and making a PR? Does not seem to be too much effort.
The text was updated successfully, but these errors were encountered: