Add module jsfetch

[juancarlospaco]

• Add module jsfetch for fetch support for JavaScript target.
• https://developer.mozilla.org/docs/Web/API/Fetch_API
• Default arguments of the API are also default arguments on this proc. 1-1 copy of the standard.
• Documentation with examples. Tiny code and runs very fast.

import jsfetch
let myPromise = fetch("http://example.io".cstring, HttpPost, body = "some data".cstring)

/* Generated by the Nim Compiler v1.0.2 */
/*   (c) 2019 Andreas Rumpf */
var framePtr = null;
var excHandler = 0;
var lastJSError = null;
if (typeof Int8Array === 'undefined') Int8Array = Array;
if (typeof Int16Array === 'undefined') Int16Array = Array;
if (typeof Int32Array === 'undefined') Int32Array = Array;
if (typeof Uint8Array === 'undefined') Uint8Array = Array;
if (typeof Uint16Array === 'undefined') Uint16Array = Array;
if (typeof Uint32Array === 'undefined') Uint32Array = Array;
if (typeof Float32Array === 'undefined') Float32Array = Array;
if (typeof Float64Array === 'undefined') Float64Array = Array;
var nim_program_result = 0;
var global_raise_hook_18618 = [null];
var local_raise_hook_18623 = [null];
var out_of_mem_hook_18626 = [null];
  if (!Math.trunc) {
    Math.trunc = function(v) {
      v = +v;
      if (!isFinite(v)) return v;
      return (v - v % 1)   ||   (v < 0 ? -0 : v === 0 ? v : 0);
    };
  }


var my_promise_191256 = (fetch("http://example.io", {
    method: ["HEAD", "GET", "POST", "PUT", "DELETE", "GET", "GET", "GET", "PATCH"][2],
    body: ("some data" || undefined),
    integrity: ("" || undefined),
    referrer: ("client" || undefined),
    mode: ["cors", "no-cors", "same-origin"][1],
    credentials: ["include", "same-origin", "omit"][0],
    cache: ["default", "no-store", "reload", "no-cache", "force-cache"][0],
    redirect: ["follow", "error", "manual"][0],
    referrerPolicy: ["no-referrer", "no-referrer-when-downgrade", "origin", "origin-when-cross-origin", "unsafe-url"][2],
    keepalive: false
}));

[Araq]

This should be a Nimble package.

[andreaferretti]

I wouldn't include fetch, which is JS specific. httpclient has the advantage of working on all environments, hence it is what we should encourage

[juancarlospaco]

The JS backend is JS specific, like literally. 🤷‍♀️

[andreaferretti]

Of course, but I think we should strive to provide a stdlib that works both native and in JS. This includes things like the json, httpclient and times modules.

Users that compile to Javascript could use JSON, fetch and Date, but if we encourage to do so, the experience of using Nim/JS becomes distinct from using Nim/C

[juancarlospaco]

That was exactly the idea...

[andreaferretti]

Sorry, I had assumed that httpclient was working under JS. If not, that functionality should definitely be provided, relying on fetch/XMLHttpRequest under the hood.

[andreaferretti]

My suggestion is to include jsfetch and later implement the API of httpclient using fetch for the javascript target

[juancarlospaco]

ReSync'd with Devel.
This is not perfect, but is like 90% of the base we need to start building on top some kind of httpclient.

[ghost]

Why close?

[timotheecour]

ya @juancarlospaco could you re-open or explain why you closed?

[andreaferretti]

My guess would be because this was made on Oct 29, rebased on Nov 29, asked for feedback on Dec 31 and languished ever since

[timotheecour]

Ya I have a lot of PR's in similar state, eg #11992 with 18 upvotes and no review after 8 months... but mostly PR's that are left to languish for weeks or months despite all review comments being addressed, CI being green, and rebased against devel.

Maybe we should consider some of the options I suggested in https://forum.nim-lang.org/t/4285#26643 (how to increase velocity for merging PRs?) as velocity is still very much a problem, and is discouraging contributors and contributions in a language that desperately needs more user base and contributions.

a few points:

• increasing velocity doesn't mean lowering quality of reviews; making a PR languish for 1 more month won't help with review quality
• throughput can be increased by more delegation + encouraging more ppl reviewing, and doing prompt reviews
• tooling can help (increasing CI speed eg with [WIP] fix #6999 (allow [nimDocOnly] in commit msg); fix #13477 (flaky nodejs install); log PR url (+other info) in runCI logs #13556 for [nimDocOnly] in commit msg and other means), implementing code coverage
• maybe a soft-process for queuing PR's to review

I'm excluding from this discussion PR's that are WIP, or with un-addressed review comments, or broken tests (excluding flaky ones). These are not "review-ready", but early feedback is still encouraged for those as early feedback saves time for both reviewer and PR author.

On a positive note, I consider important_packages.nim a success that mitigates risks of regressions and maintains a sane ecosystem amongst packages. We should build on that.

very non scientific stats

number of PR's merged over last 30 days, see https://github.com/nim-lang/Nim/pulse/monthly:

• nim: 126
• D (dmd+phobos+druntime): 181
• cpython: 411
• rust: 453

[Araq]

Well this should have been a Nimble package. Too much stuff is added to the stdlib!

[andreaferretti]

Well this should have been a Nimble package. Too much stuff is added to the stdlib!

The problem is that this is needed to support asynchttpclient in the JS target. Since asynchttpclient is already in the standard library, I see no reason to omit this module

[Araq]

The problem is that this is needed to support asynchttpclient in the JS target. Since asynchttpclient is already in the standard library, I see no reason to omit this module

I wasn't aware.

[juancarlospaco]

• figure whether these modules should go under std/jsfetch or std/js/jsfetch etc (related to discussion started in nim-lang/RFCs#344 (comment) regarding std/wintypes vs std/windows/wintypes)

I am neutral.
But jsbigints is already on std/, so you mean to move all of them?.

[timotheecour]

But jsbigints is already on std/, so you mean to move all of them?.

yes, since 1.6 wasn't released yet

[ringabout]

Thanks for your work :)

[dom96]

Well this should have been a Nimble package. Too much stuff is added to the stdlib!

The problem is that this is needed to support asynchttpclient in the JS target. Since asynchttpclient is already in the standard library, I see no reason to omit this module

@Araq @andreaferretti I don't see why that justifies putting this straight into the stdlib. You can easily create a Nimble package that implements the httpclient API and then import via:

when defined(js):
  import jshttpclient
else:
  import httpclient

In fact, you could even export the right modules from the package to avoid the need for each user to write these when's. I'm not saying this should never be in the stdlib, but I would say that to make iteration faster and ensure stability all new modules should first start out as Nimble packages unless there is a very good reason to put it in the stdlib, which this IMO isn't.

[andreaferretti]

Well, I assume most applications of Nim compiled to JS would be to make some kind of webapp. And I am pretty sure that most webapps need to contact some API endpoint via HTTP. Leaving out the HTTP client for the JS target seems silly to me. I think it would be worth to have a working, compatible implementation of httpclient for JS, and that would be inevitably based on the HTTP functionality of the underlying platform, which today is based on fetch

[dom96]

I'm not saying we should never have this in the stdlib, I'm saying we should start this out as a Nimble package, once it becomes stable move it to the stdlib, otherwise we are just asking for problems because our hands are tied if we want to improve the API. Can you explain why we cannot start this out as a Nimble package first?

[juancarlospaco]

No one is going to " improve this API ",
no need to, this directly follows the official standard,
the jshttpclient that builds on top can have the API improved freely as needed.

[dom96]

Fair enough, you're right that the API is unlikely to change for this module. But my assumption is that this won't hold for other modules and it would be best to give them a chance to bake before being included in the stdlib.

Once again, I would say this should hold for any new stdlib inclusions. I will again ask, what is the reason this cannot start life as a Nimble package so that it can be iterated on quickly and tested by the wider community much faster?

[juancarlospaco]

Anyone interested feel free to review and describe possible API designs there: #17373 (comment)