meafmira / bs-axios

Bucklescript bindings for axios
71 stars 23 forks source link

Using a dictionary instead of Js.t for headers #18

Closed Yakimych closed 5 years ago

Yakimych commented 5 years ago

Hi. Is there any way to use a dictionary instead of Js.t for headers? A scenario where I would either send "header1" or "header2" depending on a condition doesn't seem to be possible to compile, since the type of headers is defined by the first value, and thus the second one would be required to have the same shape.

Axios.makeConfigWithUrl(
  ~url="https://api.chucknorris.io/jokes/random",
  ~headers=
    checkCondition() ? {"header1": "value1"} : {"header2": "value2"},
  (),
);

The error I get is:

Error: This expression has type {. "header2": string}
       but an expression was expected of type {. "header1": string}

Is there any way to omit "header1" from the second value? Seems like a dictionary would be a structure that resembles HTTP headers more closely.

meafmira commented 5 years ago

@Yakimych it's not possible right now. We should think about this. Should we change type to Js.Dict, or add additional config field like a headersDict

meafmira commented 5 years ago

@Yakimych I think now you can use a workaround:

Axios.makeConfigWithUrl(
  ~url="https://api.chucknorris.io/jokes/random",
  ~headers=
    checkCondition()
      ? {"header1": Js.Nullable.return("value1"), "header2": Js.Nullable.undefined}
      : {"header2": Js.Nullable.return("value2"), "header1": Js.Nullable.undefined},
  (),
);

or with latest bucklescript:

Axios.makeConfigWithUrl(
  ~url="https://api.chucknorris.io/jokes/random",
  ~headers=
    checkCondition()
      ? {"header1": Some("value1"), "header2": None}
      : {"header2": Some("value2"), "header1": None},
  (),
);
Yakimych commented 5 years ago

@meafmira Thanks for the reply. I don't think the workaround works, since it still sends an empty header value, which is not the same as not sending the header at all, but I might be wrong. Maybe we can consider something similar to how it is implemented in bs-fetch: https://github.com/reasonml-community/bs-fetch#030 I think they have both options (with object and dictionary).

meafmira commented 5 years ago

@Yakimych I’m not sure but I think axios ignore undefined headers

meafmira commented 5 years ago

@Yakimych bs-fetch implementation of headers is a good one. Thanks. I'll implement it soon

meafmira commented 5 years ago

@Yakimych check it here: https://github.com/meafmira/bs-axios#headers

Yakimych commented 5 years ago

Nice! Thanks!