search

TheSpyder / rescript-webapi

ReScript bindings to the DOM and other Web APIs
http://tinymce.github.io/rescript-webapi/api/Webapi/
Other
151 stars 37 forks source link

Request and Response constructors? #63

Closed tom-sherman closed 2 years ago

tom-sherman commented 2 years ago

Are these purposefully omitted or would you accept PRs for them?

I'm currently working on some Remix and was looking to depend on this package for the webapi types.

TheSpyder commented 2 years ago

I believe it does have some bindings in this area, we pulled in bs-fetch as Webapi.Fetch:

https://github.com/tinymce/rescript-webapi/blob/f1194750136d1ac7b674ef1f21428f63213e2bbd/src/Webapi/Webapi__Fetch.res#L349-L362

We have #30 logged to update the fetch API in a more modern ReScript style after 1.0. For now we're trying to be as compatible as possible to help existing bs-webapi users to migrate.

If this doesn't meet your needs then yes we will accept PRs. You might be able to find some discussion in either the bs-fetch or bs-webapi repos that can talk to why they may have been omitted... but we are re-evaluating all decisions with this update anyway.

tom-sherman commented 2 years ago

@TheSpyder Thanks for the context! I seems to be me that while there is a way of creating a plane RequestInit object, there is no way of passing that to Request.make. There is also no way to construct a ResponseInit object or a Response instance.

So, I see two changes that could be made:

Here's what I'm doing currently to work around the lack of Response constructor: https://github.com/tom-sherman/remix-rescript-example/blob/bfed82c4a26d1121b5198d7cd3e4872d4399cc03/app/entry.server.res#L1-L11

TheSpyder commented 2 years ago

It's already possible to pass a RequestInit.t to Request.make: https://github.com/tinymce/rescript-webapi/blob/18d9d553201d575fa8e194d3370298b556b28747/src/Webapi/Webapi__Fetch.res#L386-L389

I'd be happy to accept a PR that adds matching Response.make and ResponseInit.make methods in a similar style. That's a non-breaking change. Hopefully we can find ways to make this better in a later shake-up of the fetch API bindings!