Http request with BodyInit

[a-khakimov]

I made a small draft that solves my problem #266. I suggest making such a change to support http requests with a body for other types, not only text.

[davesmith00000]

Hi @a-khakimov - sorry for the slow reply. I've been stuck on a long piece of work in another repo. Will check it out as soon as I get back to Tyrian. 👍

[a-khakimov]

Hi @davesmith00000! Just wanted to check in regarding PR and see if you’ve had a chance to take a look. Thanks, and appreciate your time!

[davesmith00000]

Good morning @a-khakimov,
Just taking a look, thanks for the nudge. Can you do a quick rebase (or fork sync) against main please? It's just to pull in the CI config changes so we can run another build.

[davesmith00000]

This might be ok, but I'm a bit worried about using BodyInit as a catch-all type. I am entirely open to counter arguments. 🙂

I haven't tried this so this is speculation, but from reading around I think we need something like this (though this seems cumbersome):

enum Body derives CanEqual:
  case Empty
  case PlainText(contentType: String, body: String)
  case File(contentType: String, body: org.scalajs.dom.File)
  case Blob(contentType: String, body: org.scalajs.dom.Blob)
  case FormData(contentType: String, body: org.scalajs.dom.FormData)
  case ArrayBuffer(contentType: String, body: org.scalajs.dom.ArrayBuffer)
  case TypedArray(contentType: String, body: org.scalajs.dom.TypedArray)
  case URLSearchParams(contentType: String, body: org.scalajs.dom.URLSearchParams)
  case ReadableStream(contentType: String, body: org.scalajs.dom.ReadableStream)

It also feels like we could do with an enum of content types, and then we could provide helper methods so that you could do things like:

def bodyImagePng(data: Blob): Body =
  Body.Blob(ContentType.imagePng, data)

Or something. What do you think?

[hobnob]

A list of content types is a big 'ol list - maybe just types we support? I suspect for now you probably just want PlainText, File, and Blob to start.. dealing with application/json and application/xml are problems all on their own for another time 🙂

[davesmith00000]

Possibly crossed wires (or I'm just wrong, wouldn't be the first time 😁 ), we're not talking mime-types, we're talking bodyinit types - I think that's the whole list ☝️.

References:
https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/send#parameters
https://fetch.spec.whatwg.org/#bodyinit-unions

An XMLHttpRequestBodyInit, which per the Fetch spec can be a Blob, an ArrayBuffer, a TypedArray, a DataView, a FormData, a URLSearchParams, or a string.

Not...sure...where I got ReadableStream from... 🤔

Does that change anything for you?

[davesmith00000]

I'm approving this because I've been unintentionally sitting on it. It is an improvement that solves a problem, thank you for the work and I'm very sorry it's taken such a long time to approve.

I still think there's more to do here, but I've left a placeholder issue that points back to here for some future person to look at:
#290

I need to figure out how to rerun this build to make sure it's passing before I merge.

Thanks again!

Update: Can't figure out how to re-run it but I've run it locally and it seems ok.