rowtype-yoga / purescript-fetch Goto Github PK

Ran a pretty basic example

module Main where

import Prelude
import Effect (Effect)
import Effect.Aff (launchAff_)
import Effect.Class (liftEffect)
import Effect.Console (log)
import Fetch (fetch)

main :: Effect Unit
main =
  launchAff_ do
    { status, text } <-
      fetch "https://httpbin.org/get"
        { headers: { "Content-Type": "applications/json" } }
    responsText <- text
    liftEffect do
      log responsText

and got this error

ReferenceError: Headers is not defined
    at Module.unsafeNew (file:///home/gavinok/.local/Dropbox/Programming/purescript/real-fetch/output/Fetch.Core.Headers/foreign.js:2:3)
    at __do (file:///home/gavinok/.local/Dropbox/Programming/purescript/real-fetch/output/Fetch.Core.Headers/index.js:25:33)
    at Module.unsafePerformEffect (file:///home/gavinok/.local/Dropbox/Programming/purescript/real-fetch/output/Effect.Unsafe/foreign.js:2:10)
    at file:///home/gavinok/.local/Dropbox/Programming/purescript/real-fetch/output/Fetch.Core.Headers/index.js:24:30
    at file:///home/gavinok/.local/Dropbox/Programming/purescript/real-fetch/output/Fetch.Core.Headers/index.js:37:70
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:533:24)
    at async loadESM (node:internal/process/esm_loader:91:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12)
[error] Running failed; exit code: 1

I was hoping to use fetch in Node, and to pass a Buffer as request body, but this is not currently possible.

main :: Effect Unit
main = do
  buffer <- Buffer.fromString "Hello, world" UTF8
  Aff.launchAff_ do
    void (Fetch.fetchBody "http://example.com" { body: buffer })

This fails to compile because there's no ToRequestBody instance for Buffer.

I opened an issue in web-fetch purescript-web/purescript-web-fetch#15 about adding RequestBody.fromBuffer (which would enable adding a ToRequestBody Buffer instance here) but that's not the right approach as it would incur a dependency on node-buffer and make both the "core" web-fetch and this fetch not portable between environments.

I think maybe a better solution would be to have instance ToRequestBody RequestBody in this library, and then a hypothetical node-fetch could wrap this fetch implementation and have its own RequestBody module with Node-specific conversion functions like fromBuffer :: Buffer -> RequestBody:

main :: Effect Unit
main = do
  buffer <- Buffer.fromString "Hello, world" UTF8
  Aff.launchAff_ do
    void (Fetch.fetchBody "http://example.com" { body: RequestBody.fromBuffer buffer })

Let me know if that makes sense, or if there are reasons for omitting the instance ToRequestBody RequestBody that I'm over-looking. Thanks!

This stems from this discussion on Discourse.

Right now it is impossible to call fetch with a still-unknown record for the options, even if you copy all the constraints straight from fetch. E.g., this will not compile:

fetch2 
  :: forall input output thruIn thruOut headers
  . Union input thruIn (HighlevelRequestOptions headers String) 
  => Union output thruOut UnsafeRequestOptions 
  => ToCoreRequestOptions input output 
  => String 
  -> { | input } 
  -> Aff Response
fetch2 = fetch

The trouble is that the compiler can't solve for thruIn used by fetch, because the thruIn of fetch2 isn't known to be the same type as thruIn for fetch. When this library gets updated to purescript 0.15.10, would you consider making thruIn a target of a visible type application so that this would compile?

fetch2 
  :: forall input output thruIn thruOut headers
  . Union input thruIn (HighlevelRequestOptions headers String) 
  => Union output thruOut UnsafeRequestOptions 
  => ToCoreRequestOptions input output 
  => String 
  -> { | input } 
  -> Aff Response
fetch2 = fetch @thruIn

My understanding is that the visible type application is optional, so if you're applying fetch to a real concrete record, you can leave the type application off, and the usage would be unchanged to what it is now.

Thanks a lot for providing the lib!

From my perspective Foreign is too generic typing for the json property - we lose information here and it is rather misleading for newcomers (it introduces people to Foreign as proper type for representing Json which is not).

I think that we can provide custom Json opaque type or use argonaut-core one. What do you think?

If you attempt to do run json on a response without a body you get an ugly messageless error right now.
I think json could either return a Maybe or we could default to {}.

Sorry if this has already been discussed, but searching issues for "cancel", "abort", "signal" yields no results.

In affjax it's possible to cancel an HTTP request using killFiber

affjaxProgram = do
  fiber <- Aff.launchAff do
    void (Affjax.Web.get ResponseFormat.json url)
  Aff.launchAff_ do
    Aff.killFiber (Exception.error "Cancel request") fiber

Translating that to use this library would be something like

fetchProgram = do
  fiber <- Aff.launchAff do
    void (Fetch.fetch url {})
  Aff.launchAff_ do
    Aff.killFiber (Exception.error "Cancel request") fiber

but because there's no Canceler set up, the request goes through.

See minimal example on Try PureScript

It could be nice to have cancellation implemented for fetch (using AbortController), for parity with affjax (which folks seem to be migrating away from since it uses deprecated XHR) and for use with APIs like useAff in react-basic-hooks that assume ability to cancel, but maybe that is beyond the scope of this library.

(Apologies for opening so many issues 😅! I'd be happy to work on any of those that I've opened if the changes would be welcome.)

I might be overlooking something, but I haven't been able to understand why the "body" type is fixed to String in the fetch type definition. If we overload the type to be any body that satisfies the Union input thruIn (HighlevelRequestOptions headers body) constraint

  fetch
-   :: forall input output @thruIn thruOut headers
-    . Union input thruIn (HighlevelRequestOptions headers String)
+   :: forall input output @thruIn thruOut headers body
+    . Union input thruIn (HighlevelRequestOptions headers body)
    => Union output thruOut CoreRequest.UnsafeRequestOptions
    => ToCoreRequestOptions input output
    => String
    -> { | input }
    -> Aff Response

the implementation still satisfies that type and it seems like that would cover the use cases of fetchBody.

While I understand adding a try catch to all initiations of fetch is a bit of a pain why not add a fetch' or something that will catch errors like ERR_CONNECTION_REFUSED and the response can be an Aff Maybe ...