Skip to content
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

Http request with BodyInit #267

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions tyrian/src/main/scala/tyrian/http/Http.scala
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ object Http:
case (Body.PlainText(contentType, body), method) if !Set(Method.Get, Method.Head).contains(method) =>
headers.append("Content-Type", contentType)
requestInit.body = body
case (Body.File(contentType, body), method) if !Set(Method.Get, Method.Head).contains(method) =>
headers.append("Content-Type", contentType)
requestInit.body = body
case _ =>

requestInit.headers = headers
Expand Down
3 changes: 3 additions & 0 deletions tyrian/src/main/scala/tyrian/http/Models.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package tyrian.http

import org.scalajs.dom.BodyInit

/** An Error will be returned if something goes wrong with an HTTP request. */
enum HttpError:
/** A BadRequest means that the provide request was not valid for some reason.
Expand Down Expand Up @@ -41,6 +43,7 @@ enum Body derives CanEqual:
* the content of the body
*/
case PlainText(contentType: String, body: String) extends Body
case File(contentType: String, body: BodyInit) extends Body
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🙂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?


object Body:
def html(body: String): Body = Body.PlainText("text/html", body)
Expand Down
Loading