Rework jshttpclient

[tandy-1000]

Building on from work in https://github.com/nim-lang/Nim/pull/17373/.

TODO:

• [x] Implement request options for jsfetch
• [x] jshttpclient should create a unified interface for xmlhttprequest and jsfetch
  • [x] Create unified request and response types
  • [x] Create interface to set options for requests but allow flexibility

[juancarlospaco]

Do not consider XMLHttpRequest and Firefox an actual Blocker, it is not our fault that recent years Firefox kinda lags behind Chromium in a few new features, it is Ok to make it just work in Chromium for now, it is not a Blocker to Merge. 🙂

I think Mozilla wasted too much resources in Rust and it shows...

[tandy-1000]

Do not consider XMLHttpRequest and Firefox an actual Blocker, it is not our fault that recent years Firefox kinda lags behind Chromium in a few new features, it is Ok to make it just work in Chromium for now, it is not a Blocker to Merge. slightly_smiling_face

I think Mozilla wasted too much resources in Rust and it shows...

I couldn't get it to work in chromium either unfortunately :(

[juancarlospaco]

I used this one and it works: https://github.com/juancarlospaco/nodejs/blob/main/src/nodejs/jsxmlhttprequest.nim

Otherwise post the full repro code that you are using.

[tandy-1000]

I used this one and it works: https://github.com/juancarlospaco/nodejs/blob/main/src/nodejs/jsxmlhttprequest.nim

Otherwise post the full repro code that you are using.

Hmm, interesting, I'll try that

[tandy-1000]

Ok my mistake, fusion/xmlhttprequest wasn't the issue, it was nodejs/jssynchttpclient .. I tested the latest commit and the before this PR, both don't seem to work

[juancarlospaco]

Make an issue with full repro code and error traceback from the console ?.

[tandy-1000]

Make an issue with full repro code and error traceback from the console ?.

https://github.com/juancarlospaco/nodejs/issues/5

[juancarlospaco]

Fixed, all green.

[juancarlospaco]

Do you think it is doable ?, it is too complicated ?.

If it is not doable, maybe Harpoon can be adapted to run in JS, it has unified interface compatible with hhtpclient. 🤔

[tandy-1000]

Do you think it is doable ?, it is too complicated ?.

If it is not doable, maybe Harpoon can be adapted to run in JS, it has unified interface compatible with hhtpclient. thinking

It is definitely doable, I haven't have time to take a look in the past few weeks.

Last time I worked on it I've been trying to figure out a good architecture - I can't really look at the Nim stdlib httpclient, because it sucks.

[tandy-1000]

I don't think setting headers is working on XMLHTTPRequest in my tests on firefox / chromium they show up as undefined..

[juancarlospaco]

Yeah, welcome to the land of buggy APIs of JavaScript. 🙂

[tandy-1000]

I also realised my generalisation of using fetchOptions for all requests (even those that don't require a body) does not work :( Maybe its worth making body in FetchOptions an Option?

[juancarlospaco]

I think that if it is not very stable, then jsasynchttpclient.nim and jshttpclient.nim need to be kept, also maybe some people used those modules already, and deleting the files may break their code, so restore jsasynchttpclient.nim and jshttpclient.nimat least for now please...

[tandy-1000]

Sure, I'll restore those.

[tandy-1000]

jsxmlhttprequest.nim

func setRequestHeader*(this: XMLHttpRequest; keyValuePairs: openArray[tuple[key, val: cstring]]) {.importjs:
  "(() => { const rqst = #; #.forEach((item) => rqst.$1(item[0], item[1])) })()".}
  ## Same as `setRequestHeader` but takes `openArray[tuple[key, val: cstring]]`.

I think this is the place with the bug

[juancarlospaco]

I think this is the place with the bug

Which bug ?, it was working last time I used it, can you fix it?.

[tandy-1000]

I think this is the place with the bug

Which bug ?, it was working last time I used it, can you fix it?.

When headers get added using that they show up like undefined: undefined, undefined undefined. I'll try.

[tandy-1000]

The last commit fixed things, so I will attempt using this new experimental/jshttpclient in some api bindings when we merge soon :)

[juancarlospaco]

When you consider is ready let me know, well review and merge...

[tandy-1000]

When you consider is ready let me know, well review and merge...

I think it is ready now that you can pass headers through, any other features you think might be useful?

[juancarlospaco]

ReSync with main branch plz, it wont let me merge because is not in sync...

git pull https://github.com/juancarlospaco/nodejs.git main
git commit -am "ReSync"
git push origin jshttpclient

... or something like that.

[tandy-1000]

I have no idea how to fix this git mess..