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

Change headers to maps #223

Closed
wojtekmach opened this issue Aug 19, 2023 · 2 comments · Fixed by #224
Closed

Change headers to maps #223

wojtekmach opened this issue Aug 19, 2023 · 2 comments · Fixed by #224

Comments

@wojtekmach
Copy link
Owner

wojtekmach commented Aug 19, 2023

Currently Req request and response headers are lists of tuples, e.g.: [{"foo", "bar"}]. This is a standard across the ecosystem (with minor difference that some Erlang libraries use charlists instead of binaries.) This particular representation makes sense because the same header might appear multiple times, see RFC 9110 § 5.2.

There are some problems with this particular choice though:

  1. We cannot use headers[key]
  2. We cannot use pattern matching

In short, this representation isn't very ergonomic to use.

Option 1: Headers as Maps

Both of these problems can be solved by switching the representation to maps of string keys and list values:

- [{"foo", "bar"}]
+ %{"foo" => ["bar"]}

This is similar to Go's net/http Header type.

Let's see some example rewrites:

- [value] = Req.Response.get_header(response, "content-type")
+ [value] = response.headers["content-type"]

or in certain situations even better:

case response do
  %{headers: %{"content-type" => ["application/json" <> _]}} ->
end

Option 2: Headers as Structs

An interesting idea is to go further and have a struct:

defmodule Req.Headers do
  defstruct [:map]
end

resp = Req.get!("https://elixir-lang.org")
resp.headers["content-type"] #=> ["text/html"]
resp.headers["Content-Type"] #=> ["text/html"] (RFC says they need to be treated case-insensitively!)

and we still could pattern match:

case response do
  %{headers: %{map: %{"content-type" => ["text/html" <> _]}}} ->
end

however here we cannot treat the names case insensitively so it's probably better to continue assuming they are lowercase (and enforce it: #196).

The struct gives us opportunity for slightly different Access implementation however:

- resp.headers["content-type"] #=> ["text/html"]
+ resp.headers["content-type"] #=> "text/html"

and the semantics would be similar to Go's Header.Get():

Get gets the first value associated with the given key.

and then we'd have something like Header.Values().

With structs we can implement protocols. For example, we may want to have an enumerable implementation for interop:

Enum.to_list(resp.headers) #=> [{"content-type", "text/html"}, ...]

And perhaps Collectable too:

Enum.into([{"content-type", "text/html"}], resp.headers)

Basically, we have extensibility.

All that being said, while I thing these are nice things, I feel like structs are a more complicated solution than plain maps and I'm not sure they give us significant benefits at the moment.

@wojtekmach
Copy link
Owner Author

Closing in favour of #224.

@georgeguimaraes
Copy link

Super interesting argumentation here!

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

Successfully merging a pull request may close this issue.

2 participants