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

HeadersInit.makeWithArray #7

Open
sidraval opened this issue Aug 4, 2017 · 10 comments
Open

HeadersInit.makeWithArray #7

sidraval opened this issue Aug 4, 2017 · 10 comments

Comments

@sidraval
Copy link

sidraval commented Aug 4, 2017

Hello,

I'm having trouble using HeadersInit.makeWithArray. When I use it, I do not see the Authorization header being sent along with my request. When I use HeadersInit.make with an object literal, it works properly:

(* No header set in request *)
let authHeaders token = Array.make 1 ("Authorizaton", "Bearer " ^ token) |> HeadersInit.makeWithArray
fetchWithInit gDriveURL (RequestInit.make ~headers:(authHeaders token) ())

(* Header set properly *)
let authHeaders' token = [%bs.obj { authorization = "Bearer " ^ token }] |> HeadersInit.make
fetchWithInit gDriveURL (RequestInit.make ~headers:(authHeaders' token) ())

I'm not sure if it's something I'm doing incorrectly, or a bug in the library -- thought I'd raise an issue and find out!

sidraval pushed a commit to thoughtbot/bs-google-drive-slideshow that referenced this issue Aug 4, 2017
@glennsl
Copy link
Contributor

glennsl commented Aug 4, 2017

Hmm, it might not be very well supported, but it's still in the spec from what I can see: https://fetch.spec.whatwg.org/#example-headers-class

MDN and github/fetch makes no mention of it however...

Is the generated JavaScript as you'd expect?

Edit:

node-fetch supports it: https://github.com/bitinn/node-fetch#class-headers

as does github/fetch, it's just not documented: https://github.com/github/fetch/blob/master/fetch.js#L87-L90

and Firefox: https://github.com/mozilla/gecko-dev/blob/master/dom/fetch/Headers.cpp#L73-L74

@mchaver
Copy link

mchaver commented Nov 7, 2017

@glennsl
Is it possible to make headers (in BuckleScript) with dashes in the key? This doesn't work.

let headers = Bs_fetch.HeadersInit.make [%bs.obj { content-type = "application/json" }]

@glennsl
Copy link
Contributor

glennsl commented Nov 7, 2017

Hmm... In Reason you'd be able to use

let headers = Bs_fetch.HeadersInit.make({ "content-type" = "application/json" })

But in OCaml I suppose you could use a Js.Dict.t and manually cast it to < .. > Js.t.

@mchaver
Copy link

mchaver commented Nov 7, 2017

How would you do that? I didn't see find anything in the BuckleScript documentation of casting between JS types.

edit: I am guessing Js_json.object_. I'll try it in a bit.

@glennsl
Copy link
Contributor

glennsl commented Nov 7, 2017

No, you'd have to define the cast yourself: external toJsObject : Js.Dict.t -> Js.t = "%identity".

"%identity" is an OCaml feature, not BuckleScript-specific, and it's not mentioned very much because it's unsafe. By using this you circumvent the type system. (But then having make take a < .. > Js.t isn't all that safe in the first place, just very convenient, at least in Reason)

@mchaver
Copy link

mchaver commented Nov 7, 2017

@glennsl Thanks, that works.

@mchaver
Copy link

mchaver commented Nov 7, 2017

@glennsl Would you be willing to add external toJsObject : 'a Js.Dict.t -> < .. >Js.t = "%identity" to the bs-fetch since it is necessary to make headers in OCaml? Though it might make more sense in the bucklescript Js library

@glennsl
Copy link
Contributor

glennsl commented Nov 7, 2017

It would make more sense in Js.Dict I think. I also think it would be better to replace the < .. > Js.t argument with Js.Dict.t, but that would be a breaking change.

I'm working on a slightly higher-level and much more idiomatic library that builds on bs-fetch, and will probably revamp and make these bindings much lighter once that's out. I'll also fix issues like this then.

@alliannas
Copy link

I had a similar problem when trying to use HeadersInit.makeWithArray.

Ended up just using

let defaultHeaders = [%bs.raw {|
  {
    "Content-Type": "application/json",
    "OtherHeader": "SomeValue"
  }
|}];

let headers =   Bs_fetch.HeadersInit.make(defaultHeaders);

@gaelollivier
Copy link

Had the same problem using react native, I guess the array initialization is not supported widely enough :( I used makeWithDict as a workaround:

HeadersInit.makeWithDict(
  Js.Dict.fromList([
    ("Authorization", "Bearer " ++ accessToken),
  ]),
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants