request does not honor parameters

[geraldodev]

Hi folks,

Thanks for the library ! I think it has two bumps on
https://github.com/lambdaisland/fetch/blob/main/src/lambdaisland/fetch.cljs#L92

(defn request [url & [{:keys [method accept content-type query-params body]
                       :as   opts
                       :or   {accept       :transit-json
                              content-type :transit-json}}]]

1st one is that deconstruction of the map is inside a [] vector so clojure is not treating "rest" parameters as a map.
The correct move is to remove the enclosing vector.

The second is: these default values for accept and content-type. They will never be used, because defaulting maps in clojure just affect the variables and opts that is the variable reference which is used to capture entire map is later passed to fetch-opts.
In this case if you want to default opts you would need to merge in a let context with the desired default values.

I would remove method and accept parameters because clj-kondo told me so :)
Notice that in fetch-options it is not a problem because the map is not passed as a whole for any routine, and it does not even name the map as a variable. Which is correct, to guard the mistake that happened on request.

Like this:

(defn request [url & {:keys [query-params body content-type]
                      :as   opts}]
  (let [url     (-> url
                    uri/uri
                    (assoc :query (uri/map->query-string query-params))
                    str)
        request (cond-> (fetch-opts opts)
                  body
                  (j/assoc! :body (if (string? body)
                                    body
                                    (encode-body (or content-type :transit-json) body opts))))]

I find the content negotiation that this library provides, killer !

Regards,

Geraldo Lopes de Souza

[plexus]

The last argument to fetch is a map, we don't use keyword arguments. This is by design.

[geraldodev]

@plexus
So if we don't use the keywords arguments as & is implying. Are the & [] necessary ?

[plexus]

It indicates the map is optional.

[geraldodev]

I see. Never seen this use of optional parameter. I always go for multi arity functions.