diff --git a/modules/reitit-core/src/reitit/coercion.cljc b/modules/reitit-core/src/reitit/coercion.cljc index 0891a0ca..9c36d48f 100644 --- a/modules/reitit-core/src/reitit/coercion.cljc +++ b/modules/reitit-core/src/reitit/coercion.cljc @@ -1,5 +1,6 @@ (ns reitit.coercion (:require [#?(:clj reitit.walk :cljs clojure.walk) :as walk] + [reitit.core :as r] [reitit.impl :as impl]) #?(:clj (:import (java.io Writer)))) @@ -220,3 +221,33 @@ [match] (if-let [coercers (-> match :result :coerce)] (coerce-request coercers match))) + +(defn coerce-query-params + "Uses an input schema and coercion implementation from the given match to + encode query-parameters map. + + If no match, no input schema or coercion implementation, just returns the + original parameters map." + [match query-params] + (when query-params + (let [coercion (-> match :data :coercion) + schema (when coercion + (-compile-model coercion (-> match :data :parameters :query) nil)) + coercer (when (and schema coercion) + (-query-string-coercer coercion schema))] + (if coercer + (let [result (coercer query-params :default)] + (if (error? result) + (throw (ex-info (str "Query parameters coercion failed") + result)) + result)) + query-params)))) + +(defn match->path + "Create routing path from given match and optional query-parameters map. + + Query-parameters are encoded using the input schema and coercion implementation." + ([match] + (r/match->path match)) + ([match query-params] + (r/match->path match (coerce-query-params match query-params)))) diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc index c3261b7c..58893e77 100644 --- a/modules/reitit-core/src/reitit/core.cljc +++ b/modules/reitit-core/src/reitit/core.cljc @@ -1,9 +1,7 @@ (ns reitit.core (:require [reitit.exception :as exception] [reitit.impl :as impl] - [reitit.trie :as trie] - ;; FIXME: Should avoid coercion require here? - [reitit.coercion :as coercion])) + [reitit.trie :as trie])) ;; ;; Expand @@ -70,25 +68,12 @@ (:template match) (:required match) path-params))))) (defn match->path + "Create routing path from given match and optional query-parameters map." ([match] (match->path match nil)) ([match query-params] (some-> match :path (cond-> (seq query-params) - ;; TODO: Should the coercion be applied elsewhere (FE ns?) so the core ns doesn't depend - ;; on the coercion? - ;; NOTE: Re-creates coercer on every call, could this be pre-compiled somewhere - ;; or memoized? Does it matter much? - ;; TODO: query-coercer could be compiled in reitit.frontend/router, same as request coercers. - (str "?" (let [coercion (-> match :data :coercion) - schema (when coercion - (coercion/-compile-model coercion (-> match :data :parameters :query) nil)) - coercer (when (and schema coercion) - (coercion/-query-string-coercer coercion schema)) - query-params (if coercer - (coercer query-params :default) - query-params)] - ;; Default encoding for values will handle values that aren't encoded using coercer - (impl/query-string query-params))))))) + (str "?" (impl/query-string query-params)))))) ;; ;; Different routers diff --git a/modules/reitit-frontend/src/reitit/frontend.cljs b/modules/reitit-frontend/src/reitit/frontend.cljs index bc5ed851..f6acaef9 100644 --- a/modules/reitit-frontend/src/reitit/frontend.cljs +++ b/modules/reitit-frontend/src/reitit/frontend.cljs @@ -40,14 +40,16 @@ (defn ^{:see-also ["reitit.core/match->path"]} match->path - "Create routing path from given match and optional query-string map and - optional fragment string." + "Create routing path from given match and optional query-parameters map and + optional fragment string. + + Query-parameters are encoded using the input schema and coercion implementation." ([match] (match->path match nil nil)) ([match query-params] (match->path match query-params nil)) ([match query-params fragment] - (when-let [path (r/match->path match query-params)] + (when-let [path (coercion/match->path match query-params)] (cond-> path (and fragment (seq fragment)) (str "#" (impl/form-encode fragment)))))) diff --git a/test/cljc/reitit/coercion_test.cljc b/test/cljc/reitit/coercion_test.cljc index 88f763cb..0e072214 100644 --- a/test/cljc/reitit/coercion_test.cljc +++ b/test/cljc/reitit/coercion_test.cljc @@ -160,13 +160,13 @@ (is (= "/olipa/kerran?x=a&x=b" (-> router (r/match-by-name! ::route {:a "olipa", :b "kerran"}) - (r/match->path {:x [:a :b]})))) + (coercion/match->path {:x [:a :b]})))) (is (= "/olipa/kerran?x=a&x=b&extra=extra-param" (-> router (r/match-by-name! ::route {:a "olipa", :b "kerran"}) - (r/match->path {:x [:a :b] - :extra "extra-param"})))))) + (coercion/match->path {:x [:a :b] + :extra "extra-param"})))))) (testing "custom encode/string for a collection" (let [router (r/router ["/:a/:b" @@ -182,18 +182,20 @@ (mapv keyword (str/split s #",")) s))} :keyword]]]}}] - {:compile coercion/compile-request-coercers})] - ;; NOTE: "," is urlencoded by the impl/query-string step, is that ok? + {:compile coercion/compile-request-coercers}) + match (r/match-by-name! router ::route {:a "olipa", :b "kerran"})] + (is (= {:x "a,b"} + (coercion/coerce-query-params match {:x [:a :b]}))) + + ;; NOTE: "," is urlencoded by the impl/query-string step (is (= "/olipa/kerran?x=a%2Cb" - (-> router - (r/match-by-name! ::route {:a "olipa", :b "kerran"}) - (r/match->path {:x [:a :b]})))) + (coercion/match->path match {:x [:a :b]}))) (testing "extra query-string parameters aren't removed by coercion" (is (= "/olipa/kerran?x=a%2Cb&extra=extra-param" (-> router (r/match-by-name! ::route {:a "olipa", :b "kerran"}) - (r/match->path {:x [:a :b] + (coercion/match->path {:x [:a :b] :extra "extra-param"}))))) (is (= {:query {:x [:a :b]}} @@ -220,7 +222,7 @@ (is (= "/olipa/kerran?x=__a&x=__b" (-> router (r/match-by-name! ::route {:a "olipa", :b "kerran"}) - (r/match->path {:x [:a :b]})))) + (coercion/match->path {:x [:a :b]})))) (is (= {:query {:x [:a :b]}} (-> (r/match-by-path router "/olipa/kerran") diff --git a/test/cljs/reitit/frontend/easy_test.cljs b/test/cljs/reitit/frontend/easy_test.cljs index bc5d9f8f..d66935d5 100644 --- a/test/cljs/reitit/frontend/easy_test.cljs +++ b/test/cljs/reitit/frontend/easy_test.cljs @@ -40,74 +40,75 @@ (fn on-navigate [match history] (let [url (rfh/-get-path history)] (case (swap! n inc) - 1 (do (is (some? (:popstate-listener history))) + 1 (rfh/push-state history ::frontpage) + 2 (do (is (some? (:popstate-listener history))) (is (= "/" url) "start at root") (rfe/push-state ::foo nil {:a 1} "foo bar")) ;; 0. / ;; 1. /foo?a=1#foo+bar - 2 (do (is (= "/foo?a=1#foo+bar" url) + 3 (do (is (= "/foo?a=1#foo+bar" url) "push-state") (.back js/window.history)) ;; 0. / - 3 (do (is (= "/" url) + 4 (do (is (= "/" url) "go back") (rfe/navigate ::bar {:path-params {:id 1} :query-params {:q "x"}})) ;; 0. / ;; 1. /bar/1 - 4 (do (is (= "/bar/1?q=__x" url) + 5 (do (is (= "/bar/1?q=__x" url) "push-state 2") (rfe/replace-state ::bar {:id 2})) ;; 0. / ;; 1. /bar/2 - 5 (do (is (= "/bar/2" url) + 6 (do (is (= "/bar/2" url) "replace-state") (rfe/set-query {:a 1})) ;; 0. / ;; 1. /bar/2 ;; 2. /bar/2?a=1 - 6 (do (is (= "/bar/2?a=1" url) + 7 (do (is (= "/bar/2?a=1" url) "update-query with map") (rfe/set-query #(assoc % :q "x") {:replace true})) ;; 0. / ;; 1. /bar/2 ;; 2. /bar/2?a=1&b=foo - 7 (do (is (= "/bar/2?a=1&q=__x" url) + 8 (do (is (= "/bar/2?a=1&q=__x" url) "update-query with fn") (.go js/window.history -2)) ;; Go to non-matching path and check set-query works ;; (without coercion) without a match - 8 (do (is (= "/" url) "go back two events") + 9 (do (is (= "/" url) "go back two events") (.pushState js/window.history nil "" "#/non-matching-path")) - 9 (do (is (= "/non-matching-path" url)) + 10 (do (is (= "/non-matching-path" url)) (rfe/set-query #(assoc % :q "x"))) - 10 (do (is (= "/non-matching-path?q=x" url)) + 11 (do (is (= "/non-matching-path?q=x" url)) (.go js/window.history -2)) ;; 0. / - 11 (do (is (= "/" url) - "go back two events") + 12 (do (is (= "/" url) + "go back two events") - ;; Reset to ensure old event listeners aren't called - (rfe/start! router - (fn on-navigate [match history] - (let [url (rfh/-get-path history)] - (case (swap! n inc) - 12 (do (is (= "/" url) - "start at root") - (rfe/push-state ::foo)) - 13 (do (is (= "/foo" url) - "push-state") - (rfh/stop! @rfe/history) - (done)) - (do - (is false (str "extra event 2" {:n @n, :url url})) - (done))))) - {:use-fragment true})) + ;; Reset to ensure old event listeners aren't called + (rfe/start! router + (fn on-navigate [match history] + (let [url (rfh/-get-path history)] + (case (swap! n inc) + 13 (do (is (= "/" url) + "start at root") + (rfe/push-state ::foo)) + 14 (do (is (= "/foo" url) + "push-state") + (rfh/stop! @rfe/history) + (done)) + (do + (is false (str "extra event 2" {:n @n, :url url})) + (done))))) + {:use-fragment true})) (do (is false (str "extra event 1" {:n @n, :url url})) (done)))))