lambdaisland / uri

A pure Clojure/ClojureScript URI library
Mozilla Public License 2.0
243 stars 21 forks source link

Return into if blank input in query-map #46

Closed velios closed 9 months ago

velios commented 10 months ago

Hello, really appreciate you work. I suggest returning an empty map instead of null for the function.

In my case, it is necessary to add UTM tags to the URL with minimal changes. Source URLs come from clients with very different quality, some, for example, are sensitive to the order of arguments(sic!). Sometimes there are URLs without any parameters at all, and then the function returns nil, which does not meet my expectations from a function like query-map in its name. I expect empty-map(or into) if no query-param exist in passed url.

For example write fn

(defn merge-params
  [^String uri-string params]
  (let [uri (uri/parse (prepare-uri-string uri-string))
        ordered-query-map (uri/query-map uri {:into (ordered-map)})
        sorted-params (sort params)
        result-query (into ordered-query-map sorted-params)]
    (-> uri
        (assoc :query (uri/map->query-string result-query))
        (str))))

Because of behaviour described above, when calling a function I get an unexpected result(incorrect order)

(merge-params "https://example.com/path" {:a 1 :b 2})
=> "https://example.com/path?b=2&a=1"

Because of inner behavior that boils down to the following

(into nil [[:a 1] [:b 2]])
=> ([:b 2] [:a 1])
velios commented 10 months ago

@plexus Bump

plexus commented 10 months ago

Please don't do this, you're being rude. We're all busy people, I'll get to this when I get to it. Thanks.

velios commented 10 months ago

@plexus I'm sorry to upset you, it's a fair point, we're all limited in the amount of time we can devote to oss. But it was good to have at least a rough idea of when this might happen. I want to take the library into the project because it’s cool, but some things may not work correctly or it just seems so. Of course, I understand that I can make a fork and use it in my project, but I don’t think that this is always the right way, especially for possible bugs.

plexus commented 9 months ago

Looks good!

plexus commented 9 months ago

I'm having second thoughts about this, see my comments here: https://github.com/lambdaisland/uri/issues/50

plexus commented 9 months ago

Released in v1.17.141

[lambdaisland/uri "1.17.141"]                 ;; deps.edn
{lambdaisland/uri {:mvn/version "1.17.141"}}  ;; project.clj