nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
104.51k stars 28.19k forks source link

Implement window.fetch into core #19393

Closed thecodingdude closed 2 years ago

thecodingdude commented 6 years ago

Edit: Fetch is available through the --experimental-fetch flag in Node. This is still very new :]


Edit: please note that this issue is pretty old and a lot of the information in the first few comments isn't up to date - for current status please see https://github.com/nodejs/node/issues/19393#issuecomment-776529831 .


https://github.com/bitinn/node-fetch

It would make sense if window.fetch was implemented into core. it seems to be a stable enough API that would make a good candidate for inclusion. Not sure what the process is from here but thought I'd raise an issue :)

jasnell commented 6 years ago

this has come up from time to time but has not had much traction just yet. Let's see what folks think tho :-)

devsnek commented 6 years ago

bradley and i were discussing this in a roundabout way on the subject of importing from urls. if that feature was introduced (and i think in general we do want it) we would need to implement this: https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-single-module-script which uses the fetch spec. as another note if this was added in core i would want to pull in an existing c++ implementation from one of the browsers. however at a bare minimum node will definitely be adding Request and Response objects, it just might not add a function called fetch

mscdex commented 6 years ago

-1 this kind of higher-level functionality is best left to userland

guybedford commented 6 years ago

This would definitely be useful in simple cross-platform APIs and I think is what a lot of people use node-fetch for already. Also it would be nice if HTTP/1 v HTTP/2 negotiation can be handled automatically like in browsers as well.

styfle commented 6 years ago

I would love this! ❤️

Isomorphic JS is one of the big reasons people who start with JS on the front end, eventually pick up Node.js on the backend.

You get to run the exact same function in the browser and the server and the one place of contention I keep finding is window.fetch.

Some of the code that runs on the server and client needs to make HTTP requests to another server (think microservices with server side rendering and client side rendering).

One case for bringing it into core is that making HTTP requests (client) is closely tied to responding to HTTP requests (server).

And we now have isomorphic URL parsing so now all we need is fetch! Let’s make fetch happen!

mikemaccana commented 6 years ago

Fetch is 'low level' according to it's author and major supporter hence missing a bunch of features like support for content types, JSON not being default, no query string encoding. Apps that need a high level quality HTTP client available in all JavaScript environments can continue using superagent.

devsnek commented 6 years ago

@mikemaccana the fetch we are talking about is https://fetch.spec.whatwg.org/ and i don't think its appropriate to be plugging other http libraries

mikemaccana commented 6 years ago

@devsnek Yes I know, that's the one I was specifically referring to. I don't have any particular enjoyment of superagent asides from it being:

If fetch supported these I'd suggest it be included in node. To repeat: I've asked fetch's author and main proponent, Jake Archibald, why it doesn't support these things and it's stated that fetch is designed to be a low level API and things like sensible defaults can/should be added by higher level APIs. As a result I see no reason to use fetch in node, or in the browser.

joyeecheung commented 6 years ago

@mikemaccana

These are actually desirable properties for the argument of including fetch in Node.js core, if we want to keep the core low-level and small. I believe we will need to eventually provide a promisified API for http/http2 anyway, and fetch as an existing spec for a similar set of low-level functionality as our http is something worth considering. In my experience the missing pieces in fetch feels pretty similar to the missing pieces in Node.js's http, the major difference is that you have a spec'ed API to count on.

Also I kind of doubt the "more popular than fetch" part, I don't think there are as many people using super-agent in the browser as in Node, given that fetch simply exists in the browser (modulo situations needing polyfills)?

Although, before we start implementing fetch, I believe we will need to introduce the stream API in the browser? Is introducing yet another stream in Node.js on the table?

The one time I've been forced to resort to XHR in the browser was when I needed progress, although I think with the browser's ReadableStreams it's possible to do the same thing with an API that's a bit awkward (res.body.read().byteLength)- if I'd implement progress in Node.js I think I'll need to use chunk.byteLength from the chunk being emitted in the data event, which is where the difference between the two streams start to matter.

Also, the fetch spec does not seem to include timeout or agents (yet?) at the moment, there might be more functionalities missing compared to our existing http API. For reference, node-fetch seems to implement these non-standard options. Again, not sure if our implementation should implement non-standard functionalities even if they supply the missing pieces compared to the old API.

joyeecheung commented 6 years ago

also cc @TimothyGu you might be interested?

joyeecheung commented 6 years ago

@thecodingdude

entirely disagree; Node uses v8, and by extension, should implement as many as v8 features as possible that make sense. fetch is one of those where developers wouldn't need to npm install request or node-fetch which are very popular libraries so this functionality warrants being in core.

Technically fetch is not a v8 feature though, it's an API spec'ed by WHATWG, whereas v8 implements ECMA-262, a spec by ECMA TC39 - if v8 implemented fetch then there would not be this feature request, because we basically just expose what v8 exposes.

BTW: I don't think we are talking about pulling npm packages into the core? Rather, we are talking about implementing the spec on our own and pulling in the WPT to test compliance, possibly with a bunch of code written in C++, much like what we did for WHATWG URL.

mikemaccana commented 6 years ago

@thecodingdude the continued popularity of non-fetch libraries isn't a personal opinion, it is a fact - superagent had 1.6 million downloads this week . Nor is the lack of reasonable high-level defaults in fetch: again (again) that is acknowledged by fetch's author. Please don't reframe verifiable objective technical facts as irrelevant subjective opinions because you do not like them.

please don't plug other libraries here, they are irrelevant to our discussion.

developers wouldn't need to npm install request or node-fetch

😂👍

bnoordhuis commented 6 years ago

I'm personally -1 for two reasons:

  1. Having two different APIs (fetch() and http.request()) for doing the same thing is uncohesive.

  2. fetch() is not flexible enough to support things like proxying, custom TLS certificates, etc. It's handled transparently by the browser but that won't work in node.js.

currently, 84 people agree

I wouldn't put too much stock in that. GH polls are easily gamed through Twitter brigading and whatnot. I'd take them more seriously if it cost a dollar to cast an upvote.

mikemaccana commented 6 years ago

thecodingdude I have commented with a bullet pointed list of technical reasons why fetch is unsuitable as a general purpose HTTP client. I am aware you "do not care how/good bad fetch is". I think this is wrong: node should pick and adopt quality APIs. That you're personally uninterested in fetch's quality does not make it irrelevant to the discussion. The browser vendors were comparing fetch to window.xhr, node is not.

Jamesernator commented 6 years ago

@mikemaccana Why does fetch need to have high level features like "HTTP verbs as method names" for it to be included in core? You can trivially write your own methods that do this, in addition to the other high level features you've mentioned.

The nice thing about having standard APIs shared across both the browser and Node is that by writing the high-level API to target the low-level one (in this case fetch) you can get consistent behaviour much more easily than having to make your API work around differences between different APIs in node and the browser.

devsnek commented 6 years ago

@thecodingdude any reason this was closed? discussion still seems to be moving

thecodingdude commented 6 years ago

@devsnek yeah, I am not satisfied with the way this discussion and issue went, and I realise that github issues are simply not the appropriate forum for discussing whether features should land into core. I much prefer the node-eps since that was a proper document outlining the proposal. My intention was to gather the thoughts of the core team, and whomever else is involved in merging features into core (planning, coding, testing etc). Instead the discussion here so far is basically meaningless. It should be about feasibility, what would be required, and when such a feature could be merged.

Go take a look at this issue and try and follow the avalanche of conversation about websockets in core. There's too much noise and not a straightforward approach to proposing new features. I think PHP's rfc system is a good example of getting new features added. Clear, concise, accountable.

I'm down for discussing an rfc and its merits in a separate issue but it's clear this isn't going anywhere, with little to no interaction from TSC or anyone else that matters so there's no point leaving it up just to spam people's inboxes.

I'll refrain from making such issues in the future, my bad. cc @jasnell

mikemaccana commented 6 years ago

@Jamesernator Why does fetch need to have high level features like "HTTP verbs as method names" for it to be included in core? You can trivially write your own methods that do this, in addition to the other high level features you've mentioned

Sure. I, and everyone else that needs to use query strings, can write a method to encode query strings. Better yet: let's include a good quality, batteries included HTTP client in node where everyone doesn't need to fix it.

@thecodingdude looks like you deleted your comment, but to answer anyway: HTTP2 is necessary because it is required to speak the current version of HTTP. fetch is not necessary, because it is but one of many HTTP clients, and not a very good one, for the reasons discussed.

Jamesernator commented 6 years ago

@mikemaccana But the reason for including fetch in Node core isn't about batteries included, it's so that you can write code that targets both the browser and Node at once. That's why Node also supports the URL constructor (and it's even now global like in the browser in node 10) and the docs even refer to the old url.Url objects as the legacy url API.

This is a good thing, as it means some module that uses dynamic import for instance they don't need to include any library to be able to load resources relative to it e.g.:

...

export default async function isWordForLanguage(language="english") {
    const wordDataUrl = new URL(`./resources/${ language }.csv`, import.meta.url)
    const words = await loadSomehow(wordDataUrl)
    return makeIsWord(words)
}

Now that code has no platform specific behavior (assuming a loadSomehow method which could potentially be fetch). The benefits to this should be fairly obvious:

I think it's a pointless endeavour to create another http library in Node if it isn't for interoperability. If Node creates another one that isn't in the browser then you simply get the same situation you have now, if you want browser inter-operable code you another library built on top of it just to get interoperability with browsers.

If you think it's important for browsers to support high-level APIs out of the box then it's probably worth supporting the efforts to make high level APIs part of the browser and make clear your goals what you want to see out of high-level APIs. I'm sure many developers would appreciate getting high-level API specifications.

Even better would be if there was a way to get Node and Browsers to collaborate on high-level features, I'm not sure how this might work but if it could be done then it'd be a lot easier to propose high-level APIs that target all environments instead of being silo-ed into Node or the browsers.

mikemaccana commented 6 years ago

Thanks @Jamesernator - it's nice to know there's an effort to resolve this on a larger scale. Thanks also for being polite and actually reading my comments before responding.

guybedford commented 6 years ago

I tend to think that were NodeJS to have been developed today, there's no doubt that fetch would have been implemented in the name of aligning with browser APIs. Yes there are some really interesting edge cases, but @TimothyGu has handled these really well in the node-fetch integration.

@thecodingdude do you mind if I reopen? We potentially could still have things line up here - I don't think the detailed discussion has run its course fully quite yet and I think there's still fruitful discussion to be had personally.

@bnoordhuis node-fetch handles the differences here by supporting the NodeJS-specific agent option that delegates features like proxy https://github.com/bitinn/node-fetch#options. It breaks the universal API, but could also be done conditionally quite easily, so I tend to think it seems a fairly good compromise with this stuff. It could also be decided that such a divergence is a bad idea as well though certainly.

The key thing though is that this fetch API is purposely restricted - HTTP and HTTP/2 APIs still remain fundamental, this could just provide the easy universal experience. There's nothing wrong with having two APIs to do the same thing, when one builds on top of the other to provide a compelling user experience.

thecodingdude commented 6 years ago

@guybedford feel free to make a new issue discussing this further if that's the direction that is most suitable. I'd much prefer technical discussions about the implementation itself, and not what fetch does/doesn't do vs alternatives which is why I closed the issue in the first place.

Quite honestly, I don't see why this discussion needs to happen on github - at the least is should be locked to collaborators who are going to be working on the code and discussions focused entirely on implementing fetch, identical to the browser where possible, and nothing more and nothing less.

guybedford commented 6 years ago

@thecodingdude I'm still not quite sure I follow why you need a new issue here. Discussing the problem boundaries should surely be the first step for any new feature, and allowance should be made for a wider discussion at the start as well. 16 comments hardly seems off the rails as well.

Certainly the technicalities of whether it should be JS or C++, or where to draw the line on pooling and proxies are important, but things have to run their own course.

bnoordhuis commented 6 years ago

There's nothing wrong with having two APIs to do the same thing, when one builds on top of the other to provide a compelling user experience.

But it's not 'on top of' - they do the same thing, just with different interfaces.

Arguments in favor of fetch() boil down to 'convenience' - not a good reason for inclusion in core.

guybedford commented 6 years ago

Thanks @bnoordhuis these are important points as to where the line is in Node, and I must admit I'm not familiar with these sorts of discussions historically, but so long as Node strives to provide core utilities, this seems like an important one to include to me.

But it's not 'on top of' - they do the same thing, just with different interfaces.

Currently node-fetch is built on top of the http module, I'd personally prefer such a JS-based implementation here, working from exactly what @TimothyGu has already built as one of the most popular libraries. The pattern of having high and low level APIs for the same thing is an established one.

Arguments in favor of fetch() boil down to 'convenience' - not a good reason for inclusion in core.

Convenience is a hugely important argument, the question is by how much to justify the maintenance costs. Node doesn't just support modules and native bindings - it provides server utilities and universal APIs where appropriate. As I've said, fetch seems to me a critical part of the universal toolbox these days.

I'd even push for fetch to transparently integrate HTTP/2 in future, allowing such a feature to not only build on top of the existing builtins, but also to provide a unification that we don't currently have. I know there are complexities to be tackled here, and it's not a given, but it feels like it would be huge win for Node and its users.

bnoordhuis commented 6 years ago

Currently node-fetch is built on top of the http module

That's an implementation detail. Looking at it from a user's perspective it's different ways of doing the same thing, neither one clearly superior.

Convenience is a hugely important argument, the question is by how much to justify the maintenance costs.

The public interface of built-in modules cannot be changed easily, sometimes not at all. Best case it's still a years long affair. Modules on npm don't have that issue.

fetch() has going for it that it's a standard, except we'd have to extend it to make it usable (e.g., that agent option you mentioned.)

guybedford commented 6 years ago

It seems like we would need to assess if the agent approach used in node-fetch would be suitable, and how stable this API or another similar around this could be considered to be. Perhaps @TimothyGu can share thoughts re the stability of these cases to ensure backwards compat by default.

TimothyGu commented 6 years ago

I am conflicted on this issue. As a perennial supporter of the small core, I point to the flexibility in making breaking changes being a userland module allows. However, node-fetch (and the Fetch API) has been remarkably stable: node-fetch only had one breaking release in the past few years, and the currently scheduled breaking changes are all logistical (bumping the supported versions of Node.js and a third-party package).

On the other hand, I feel it is would be a much improved experience for users if they do not have to install a package for the very basic purpose of fetching a file in a script, with an interface they could be familiar with. No, http.request does not count as one solution, since one would have to put more work into supporting redirects, compression, and now transparent HTTP/2.

Regarding the functionality gap @bnoordhuis mentioned:

fetch() has going for it that it's a standard, except we'd have to extend it to make it usable (e.g., that agent option you mentioned.)

In my opinion, if node-fetch has all of its extensions (grep for "node-fetch extension" in the README) removed, it would still be a perfectly useful tool, especially for single-file scripts. Telling the user "fetch() is offered only for convenience, and for more features/customizations (like the agent option), use node-fetch/request/etc." is perfectly viable solution.

In the end, I would be happy to see fetch() and peripheral APIs be a part of core.

jakearchibald commented 6 years ago

For node to support fetch we may need to make a couple of spec changes to allow it to bypass things like CORS, but it sounds doable.

However, I don't think it's worth considering unless/until node implements https://streams.spec.whatwg.org/, which I'd love to see! Edit: Although, browsers shipped fetch before streams landed, so I guess node could do the same.

jakearchibald commented 6 years ago

@mikemaccana

Fetch is 'low level' according to it's author

It's low-level for the web. But in some ways that makes it high-level for node due to its relatively relaxed security requirements. Although this could be tacked pretty easily in the spec.

and major supporter hence missing a bunch of features like support for content types

Fetch supports the Content-Type header.

JSON not being default

For what? Response bodies? Having a type that changes depending on a header from the sender sounds risky to me. Eg, you could easily end up with code that works great when the response pops out as an object, but if an attacker could manipulate headers to change that into a binary format, bad things could happen.

no query string encoding

This isn't true. The fetch API supports URL objects which support searchParams: https://url.spec.whatwg.org/#interface-urlsearchparams. These can be used as request bodies too.

'fetching with method POST' which is a somewhat odd mental model

It's… how HTTP works. The method name is a string.

I've asked fetch's author and main proponent, Jake Archibald

I'm not fetch's author, although I have contributed. I don't know what qualifies me as the main proponent either.

The list of "dislikes" seems pretty weak to me. A better way to prove this could be with code. Being aggressive & uninformed is a bad combo, try to shake at least one of these.

@joyeecheung

The one time I've been forced to resort to XHR in the browser was when I needed progress, although I think with the browser's ReadableStreams it's possible to do the same thing with an API that's a bit awkward

It's currently:

const response = await fetch(url);
const total = Number(response.headers.get('content-length'));
let bytes = 0;
for await (const chunk of iterateStream(response.body)) {
  bytes += chunk.length;
  console.log(bytes, ' - ', total);
}

I kinda like that this is explicit that you're trusting the Content-Length header for the total. Although I'd like to introduce some kind of observer object to make this easier. And you won't need iterateStream once ReadableStream implements Symbol.asyncIterator.

Also, the fetch spec does not seem to include timeout

You can do this with abort signals:

const controller = new AbortController();
const { signal } = controller;
setTimeout(() => controller.abort(), 3000);
const response = await fetch(url, { signal });

This gives you the flexibility to timeout in more complex ways, eg if no bytes are sent for some kind of timeout:

const controller = new AbortController();
const { signal } = controller;
let timeoutId;
const response = await fetch(url, { signal });
const resetTimer = () => {
  clearTimeout(timeoutId);
  timeoutId = setTimeout(() => controller.abort(), 3000);
};
resetTimer();

for await (const chunk of iterateStream(response.body)) {
  resetTimer();
  doSomething(chunk);
}

or agents

Yeah, we don't have anything like this yet. We'd have to think about the security implications of this for the web. Or, we just reserve that option for node in the spec, so we'll never use something of the same name to mean something different.

addaleax commented 6 years ago

(I’ll re-open this because the discussion is obviously still ongoing and this issue hasn’t really come to a conclusion so far.)

mikemaccana commented 6 years ago

@jakearchibald oh dear, here we we go again.

Fetch supports the Content-Type header.

Yes. That's not the same thing a actually using a content type: if I accept JSON, then give me JSON. Don't ask every user to manually decode it.

JSON not being default for what for what? Response bodies?

Content Type and accepts.

The fetch API supports URL objects which support searchParam

Cool. Is this documented for users? The only fetch documentation I've ever seen has people manually encoding query strings and the link you just gave is to an implementation guide for browser developers.

It's… how HTTP works. The method name is a string.

Doesn't make it 'fetching a GET' or however you want to put it less awkward. More like someone really, really wanted to use the name 'fetch' instead of http.

I don't know what qualifies me as the main proponent either.

(Edit: mention of Jake's behavior on other social media platforms removed)

A better way to prove this could be with code.

😂. You contributed to a spec that doesn't do in 2018 what superagent did in 2012 and you want code, like nobody's ever written an HTTP client that doesn't suck before?

jasnell commented 6 years ago

This is not the place to discuss general opinions about fetch itself. Please keep the discuss focused on merits of implementing fetch in core.

benjamingr commented 6 years ago

I think this is one DOM API that'll be very hard to get right in core (due to header safelists etc) but easy to get well enough in userland.

jakearchibald commented 6 years ago

@benjamingr I don't think node would have to comply with the parts of fetch that are there for web security. It'd be nice to get changes into the spec that allow node to do that while still being compliant, as long as isn't too complicated.

mikemaccana commented 6 years ago

@jasnell my main concern is that adding fetch to core would still require users to either write a bunch of boilerplate code for common tasks, or install a high level library - as a result adding fetch would achieve very little for end users.

benjamingr commented 6 years ago

@jakearchibald you've always been very willing to take the extra step for the web platform - and as you know - I'm a fetch fan myself :) There are many things in fetch that are very different from Node's PoV as a client.

The API surface of fetch is actually quite large and browsers (and polyfills) don't really implement it. While fetch itself has been quite stable browsers haven't really:

Things that are geared towards ServiceWorker won't work (although I'm not sure if you saw Cloudflare's newish "server-side service-workers" thing which is really cool!).

Fetch is great, but I don't agree it's as stable or ready in browsers as people here mention - most of the polyfills blatantly ignore a lot of the edge cases.

Worst - we both know fetch is a lower level API than XHR that is quite more powerful, builds on better primitives and adds capabilities. None of the comments here focused on that - it sounds like people (not participants from core though) want this as a convenience rather than with the directed goal of web-node platform compatibility or exposing capabilities.

I think it would be interesting to make a concrete proposal evaluating all the changes it would require in Node and evaluate it then.

benjamingr commented 6 years ago

@thecodingdude

Quite honestly, I don't see why this discussion needs to happen on github - at the least is should be locked to collaborators who are going to be working on the code and discussions focused entirely on implementing fetch, identical to the browser where possible, and nothing more and nothing less.

We really dislike locking issues here and only do so when we're unable to moderate them. In this issue (while heated) people have made an effort to stay polite. We see everyone responding or reading here as potential contributors and I would like to invite anyone who is reading this and is not sure how to start participating more in node to reach out (my email is benjamingr at gmail) and we'll do our best to find you interesting ways to contribute.

So far this discussion has been going on pretty well with people raising arguments for both sides. Thanks for opening the issue and bringing it here.


@mikemaccana I request that you please consider self-moderating this comment

oh dear, here we we go again.

I've enjoyed your perspective other than that particular comment and I request that we keep a welcoming discussion environment here - as much as possible.

thecodingdude commented 6 years ago

@benjamingr and @addaleax please file a new issue in regards to this discussion. I never intended to open a can of worms which has seemingly happened and I dislike the general tone and attitude @mikemaccana has demonstrated throughout this issue with repeated offtopic remarks and general unhelpfulness. His contributor status should be reconsidered.

Heated discussions are not appropriate for Node and the request was quite simple: fetch into core. I would like to request you close/lock this issue and create a proposal if inclusion is viable. We do not need 50 more comments arguing too and fro as we end up going nowhere.

I think it would be interesting to make a concrete proposal evaluating all the changes it would require in Node and evaluate it then.

That pretty much ends the discussion. It'll be much easier to understand viability when there's a proposal (which is what node-eps was designed to do but sadly doesn't happen anymore).

mikemaccana commented 6 years ago

@thecodingdude similarly, I dislike that you're not tolerant of discussion of the merits of including fetch and wish to only limit the discussion to how it should be done, rather than whether it should be done - which is a reasonable concern whether you personally like it or not.

jakearchibald commented 6 years ago

@benjamingr

Fetch introduces a lot of classes (Request, Response, Headers, etc) that would exist alongside node's existing infrastructure for those things.

Yeah, I can't see how you'd implement fetch without also implementing those. Node implemented WHATWG url despite having its own methods, so I guess it'd be the same here. Question is if it's worth it in this case.

Browsers haven't really figured out .body.getReader() yet

I don't think this is true. The streams spec is pretty stable, with implementations in Chrome, Edge, & Safari. Firefox isn't there yet, but I hear they're finalising their implementation.

What type would it return in Node?

response.body would have to be a WHATWG readable stream (which is what I meant in https://github.com/nodejs/node/issues/19393#issuecomment-376443373).

What does .cancel on the body do?

This is standardised. https://streams.spec.whatwg.org/#rs-cancel covers the stream-specific parts, and the fetch spec reacts to cancelation in multiple places.

AbortController would mean discussing and settling how cancellation works in Node

Yeah, and AbortSignal is an EventTarget which I guess would need to be an EventEmitter in node. That's a compatibility issue we can't really avoid right now.

Things that are geared towards ServiceWorker won't work

Are there particular things you're thinking of? Fetch's references to service worker are pretty minor compared to CORS (which we'd need to find workarounds for).

Fetch is great, but I don't agree it's as stable or ready in browsers

I don't think that's fair to say. It's implemented in Chrome, Firefox, Edge, & Safari. Some implementations have been around for years. Ok, streams aren't part of the Firefox implementation yet, but it's the outlier.

I think the "should node implement?" question comes down to compatibility & suitability rather than stability. I don't have strong feeling here, but if you said "Node will implement either fetch without streams, or WHATWG streams but not fetch. Choose." I'd absolutely pick WHATWG streams.

I think it would be interesting to make a concrete proposal evaluating all the changes it would require in Node and evaluate it then.

+1 and the changes required in the fetch spec.

thecodingdude commented 6 years ago

@mikemaccana fetch is fetch - if you dislike the standard for any reason then take it up with WhatWG, this is not the right place to air your grievances with what it does/doesn't do. Whether this actually ends up in core depends entirely on the Node developers who will be lumbered with the responsibility of maintaining it for a number of years - this is where 'userspace' can be more practical because it's outside Nodes scope so if we're really requiring this much discussion then maybe it's not a suitable candidate to land in core.

Perhaps you'd be so kind to bug WhatWG with all the problems you've discussed here and get them to update the spec for you? You can file an issue with your concerns here.

benjamingr commented 6 years ago

@thecodingdude I kindly request that you consider your phrasing here. As a part of Node I've found the discussion (including Mike's most of the time) interesting. Please be respectful of different viewpoints and experiences and use welcoming and inclusive language.

We do a lot more discussion than this before introducing a feature like fetch, it is then brought up (as a concrete proposal) to the TSC who decide on whether or not it's worth taking into core and then we do more discussions on the implementation before landing it.

mikemaccana commented 6 years ago

@thecodingdude This isn't about personal grievances. Again I'm referring to the boilerplate code or libraries most node users will need to install in order to use fetch as an HTTP client. As @Jamesernator already mentioned, https://github.com/drufball/layered-apis/ is the place where discussion around high level APIs is already taking place (thanks James). I've said everything I need to here in previous comments. I feel that you continually mischaracterise these as somehow being personal grievances - if you stopped doing that, I wouldn't have to reply.

I appreciate there may be good reasons to include standard things even if they're suboptimal. Fair enough. But technical limits requiring most users to apply the same boilerplate are not personal grievances.

Edit: I have used technical means to stop the discussion between you and I.

Edit 2 to use more welcoming language per @benjamingr request.

benjamingr commented 6 years ago

@mikemaccana as it currently stands your comment violates our code of conduct. Please moderate it asap to meet our standards.

mikemaccana commented 6 years ago

@benjamingr Sorry, and done.

trygve-lie commented 6 years ago

Fetch introduces a lot of classes (Request, Response, Headers, etc) that would exist alongside node's existing infrastructure for those things.

These are classes Fetch would need, but there are also APIs which work with Fetch which I think developers would expect to be there if Fetch is implemented in core. I'm thinking of FormData and URLSearchParams which both are handy APIs when working with Form data and URL parameters. URLSearchParams are already in core.

If Fetch is to be implemented in core would one expect FormData to be implemented also?

I would find it a bit weird to have Fecth and URLSearchParams in core but not FormData.

benjamingr commented 6 years ago

I have gone ahead and deleted the last few comments. Actions of an individual on social media are not in scope for our issue tracker. I highly encourage you to edit your comments to keep the discussion on-topic and constructive.

addaleax commented 6 years ago

Just a question: If Node core gets a global fetch, does that mean that we could implement that by shipping node-fetch as a baked-in dependency, similar to what we do with node-inspect?

devsnek commented 6 years ago

@addaleax technically yes but I think one thing we want to do is keep the underlying http code separate, and I for one want to implement as much of it natively as possible