dakrone / clj-http

An idiomatic clojure http client wrapping the apache client. Officially supported version.
http://clojars.org/clj-http
MIT License
1.78k stars 408 forks source link

Get rid of dependency on potemkin #592

Open expez opened 3 years ago

expez commented 3 years ago

Potemkin is no longer maintained and is barely used by clj-http. It's only use is in the clj-http.headers ns where potemkin/def-map-type is used to conveniently create a map with multiple versions of the same key.

This came up as I was trying to revive some very old code that was using lamina and aleph and had to resolve the potemkin dep across these libs. That turned out to be a PITA.

While I'm here: thanks for a wonderful library! 💯 I've used it for a very long time and I think this is the first time I've opened an issue 👍

rymndhng commented 3 years ago

Hi @expez!

That sounds like a good idea. I haven't put a lot of thought into the details of how to best approach it. However, I can throw in my initial thoughts:

If the library is to keep the existing functionality and the goal is to remove the external dependency, we'd have to inline the potemkin code. There is quite a lot of code that potemkin implements, which makes this option undesirable.

Another option is to drop the Header Map functionality or change it's API to be simpler. A concern here is that it would break the behavior for existing users of clj-http.

The functionality of HeaderMap is useful in some context, but it comes at the cost of simplicity. It's an abstraction that has a lot of use-cases to consider. It doesn't always work intuitively as a Clojurist would expect, see #525 and #450.

We can also consider Ring's SPEC. Following ring is a pragmatic option, and most developers will be familiar with it's representation of headers.

This seems like something we should consider for 4.x, I am hesitant to change it's behavior in 3.x.