From 25dd0abcaf1c497e71490d6bb802dc545d1fd1a3 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Tue, 21 Jan 2025 14:40:56 +0200 Subject: [PATCH 01/16] Use coercion to encode query-string values in match->path --- modules/reitit-core/src/reitit/coercion.cljc | 3 +- modules/reitit-core/src/reitit/core.cljc | 21 ++++++++- .../src/reitit/coercion/malli.cljc | 4 +- .../src/reitit/coercion/schema.cljc | 5 ++- .../reitit-spec/src/reitit/coercion/spec.cljc | 5 ++- test/cljc/reitit/coercion_test.cljc | 43 +++++++++++++++++-- 6 files changed, 72 insertions(+), 9 deletions(-) diff --git a/modules/reitit-core/src/reitit/coercion.cljc b/modules/reitit-core/src/reitit/coercion.cljc index bc3dc6ca..0891a0ca 100644 --- a/modules/reitit-core/src/reitit/coercion.cljc +++ b/modules/reitit-core/src/reitit/coercion.cljc @@ -19,7 +19,8 @@ (-open-model [this model] "Returns a new model which allows extra keys in maps") (-encode-error [this error] "Converts error in to a serializable format") (-request-coercer [this type model] "Returns a `value format => value` request coercion function") - (-response-coercer [this model] "Returns a `value format => value` response coercion function")) + (-response-coercer [this model] "Returns a `value format => value` response coercion function") + (-query-string-coercer [this model] "Returns a `value => value` query string coercion function")) #?(:clj (defmethod print-method ::coercion [coercion ^Writer w] diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc index 95633aec..b1416a01 100644 --- a/modules/reitit-core/src/reitit/core.cljc +++ b/modules/reitit-core/src/reitit/core.cljc @@ -1,7 +1,9 @@ (ns reitit.core (:require [reitit.exception :as exception] [reitit.impl :as impl] - [reitit.trie :as trie])) + [reitit.trie :as trie] + ;; FIXME: Should avoid coercion require here? + [reitit.coercion :as coercion])) ;; ;; Expand @@ -71,7 +73,22 @@ ([match] (match->path match nil)) ([match query-params] - (some-> match :path (cond-> (seq query-params) (str "?" (impl/query-string 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? + (str "?" (let [schema (-> match :data :parameters :query + ;; FIXME: Why? + first) + coercion (-> match :data :coercion) + coercer (when (and schema coercion) + (coercion/-query-string-coercer coercion schema)) + query-params (or (when 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))))))) ;; ;; Different routers diff --git a/modules/reitit-malli/src/reitit/coercion/malli.cljc b/modules/reitit-malli/src/reitit/coercion/malli.cljc index b94b17b3..3416079a 100644 --- a/modules/reitit-malli/src/reitit/coercion/malli.cljc +++ b/modules/reitit-malli/src/reitit/coercion/malli.cljc @@ -176,6 +176,8 @@ (-request-coercer [_ type schema] (-coercer schema type transformers :decode opts)) (-response-coercer [_ schema] - (-coercer schema :response transformers :encode opts)))))) + (-coercer schema :response transformers :encode opts)) + (-query-string-coercer [_ schema] + (-coercer schema :string transformers :encode opts)))))) (def coercion (create default-options)) diff --git a/modules/reitit-schema/src/reitit/coercion/schema.cljc b/modules/reitit-schema/src/reitit/coercion/schema.cljc index b2fd14d1..adbbcf05 100644 --- a/modules/reitit-schema/src/reitit/coercion/schema.cljc +++ b/modules/reitit-schema/src/reitit/coercion/schema.cljc @@ -101,6 +101,9 @@ value)))) (-response-coercer [this schema] (if (coerce-response? schema) - (coercion/-request-coercer this :response schema))))) + (coercion/-request-coercer this :response schema))) + (-query-string-coercer [this schema] + ;; TODO: Can this be implemented? + nil))) (def coercion (create default-options)) diff --git a/modules/reitit-spec/src/reitit/coercion/spec.cljc b/modules/reitit-spec/src/reitit/coercion/spec.cljc index bf4bed82..56c6e6a7 100644 --- a/modules/reitit-spec/src/reitit/coercion/spec.cljc +++ b/modules/reitit-spec/src/reitit/coercion/spec.cljc @@ -148,6 +148,9 @@ value)))) (-response-coercer [this spec] (if (coerce-response? spec) - (coercion/-request-coercer this :response spec))))) + (coercion/-request-coercer this :response spec))) + (-query-string-coercer [this spec] + ;; TODO: Can this be implemented? + nil))) (def coercion (create default-options)) diff --git a/test/cljc/reitit/coercion_test.cljc b/test/cljc/reitit/coercion_test.cljc index ee18c59c..6ab91745 100644 --- a/test/cljc/reitit/coercion_test.cljc +++ b/test/cljc/reitit/coercion_test.cljc @@ -1,5 +1,8 @@ (ns reitit.coercion-test - (:require [clojure.test :refer [deftest is testing]] + (:require [clojure.spec.alpha :as cs] + [clojure.string :as str] + [clojure.test :refer [deftest is testing]] + [malli.core :as m] [malli.experimental.lite :as l] [reitit.coercion :as coercion] [reitit.coercion.malli] @@ -7,8 +10,8 @@ [reitit.coercion.spec] [reitit.core :as r] [schema.core :as s] - [clojure.spec.alpha :as cs] - [spec-tools.data-spec :as ds]) + [spec-tools.data-spec :as ds] + [malli.transform :as mt]) #?(:clj (:import (clojure.lang ExceptionInfo)))) @@ -150,3 +153,37 @@ {:compile coercion/compile-request-coercers})] (is (= {:path {:user-id 123, :company "metosin"}} (:parameters (match-by-path-and-coerce! router "/metosin/users/123")))))) + +(deftest match->path-parameter-coercion-test + (testing "default handling for query-string collection" + (let [router (r/router ["/:a/:b" ::route])] + (is (= "/olipa/kerran?x=a&x=b" + (-> router + (r/match-by-name! ::route {:a "olipa", :b "kerran"}) + (r/match->path {:x [:a :b]})))))) + + (testing "custom encode/string for a collection" + (let [router (r/router ["/:a/:b" + {:name ::route + :coercion reitit.coercion.malli/coercion + :parameters {:query [:map + [:x + [:vector + {:encode/string (fn [xs] + (str/join "," (map name xs))) + :decode/string (fn [s] + (if (string? s) + (mapv keyword (str/split s #",")) + s))} + :keyword]]]}}] + {:compile coercion/compile-request-coercers})] + ;; NOTE: "," is urlencoded by the impl/query-string step, is that ok? + (is (= "/olipa/kerran?x=a%2Cb" + (-> router + (r/match-by-name! ::route {:a "olipa", :b "kerran"}) + (r/match->path {:x [:a :b]})))) + + (is (= {:query {:x [:a :b]}} + (-> (r/match-by-path router "/olipa/kerran") + (assoc :query-params {:x "a,b"}) + (coercion/coerce!))))))) From 1819fa5d754c77acc761826914fb58e84448376c Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Tue, 21 Jan 2025 14:59:26 +0200 Subject: [PATCH 02/16] Note --- modules/reitit-core/src/reitit/core.cljc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc index b1416a01..1f09c664 100644 --- a/modules/reitit-core/src/reitit/core.cljc +++ b/modules/reitit-core/src/reitit/core.cljc @@ -80,6 +80,8 @@ ;; or memoized? Does it matter much? (str "?" (let [schema (-> match :data :parameters :query ;; FIXME: Why? + ;; Due the Malli schema merge? Needs + ;; coercion/-compile-model call first. first) coercion (-> match :data :coercion) coercer (when (and schema coercion) From 5829e1c6567e1d16f844309bc3912106e0bbaf50 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Tue, 21 Jan 2025 16:41:02 +0200 Subject: [PATCH 03/16] Add reitit.frontend test case --- modules/reitit-core/src/reitit/core.cljc | 9 ++---- test/cljs/reitit/frontend/core_test.cljs | 38 ++++++++++++++++++++---- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc index 1f09c664..7d28ed42 100644 --- a/modules/reitit-core/src/reitit/core.cljc +++ b/modules/reitit-core/src/reitit/core.cljc @@ -78,12 +78,9 @@ ;; on the coercion? ;; NOTE: Re-creates coercer on every call, could this be pre-compiled somewhere ;; or memoized? Does it matter much? - (str "?" (let [schema (-> match :data :parameters :query - ;; FIXME: Why? - ;; Due the Malli schema merge? Needs - ;; coercion/-compile-model call first. - first) - coercion (-> match :data :coercion) + (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 (or (when coercer diff --git a/test/cljs/reitit/frontend/core_test.cljs b/test/cljs/reitit/frontend/core_test.cljs index 0d7a7bc7..9bb054ba 100644 --- a/test/cljs/reitit/frontend/core_test.cljs +++ b/test/cljs/reitit/frontend/core_test.cljs @@ -1,13 +1,14 @@ (ns reitit.frontend.core-test - (:require [clojure.test :refer [deftest testing is are]] + (:require [clojure.string :as str] + [clojure.test :refer [are deftest is testing]] + [reitit.coercion :as rc] + [reitit.coercion.malli :as rcm] + [reitit.coercion.schema :as rcs] [reitit.core :as r] [reitit.frontend :as rf] - [reitit.coercion :as rc] - [schema.core :as s] - [reitit.coercion.schema :as rcs] - [reitit.coercion.malli :as rcm] [reitit.frontend.test-utils :refer [capture-console]] - [reitit.impl :as impl])) + [reitit.impl :as impl] + [schema.core :as s])) (deftest query-params-test (is (= {:foo "1"} @@ -297,3 +298,28 @@ (testing "Fragment encoding" (is (= "foo#foo+bar+%25" (rf/match->path {:path "foo"} nil "foo bar %"))))) + +(defn instant->string + [x] + (str "x" (.toISOString x))) + +(defn string->instant + [x] + (if (string? x) + (js/Date. (subs x 1 (count x))) + x)) + +(deftest match->path-coercion-test + (testing "default Date toString" + (is (str/starts-with? + (rf/match->path {:path "foo"} {:date (js/Date. 2024 0 1 12 13 0)}) + "foo?date=Mon+Jan+01+2024+12"))) + + (is (= "foo?date=x2024-01-01T10%3A13%3A00.000Z" + (rf/match->path {:data {:coercion rcm/coercion + :parameters {:query [[:map + [:date {:decode/string string->instant + :encode/string instant->string} + :any]]]}} + :path "foo"} + {:date (js/Date. 2024 0 1 12 13 0)})))) From dba8d159cc932aba48dd39a6a938126511fd03f6 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Wed, 22 Jan 2025 10:22:58 +0200 Subject: [PATCH 04/16] . --- modules/reitit-core/src/reitit/core.cljc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc index 7d28ed42..c3261b7c 100644 --- a/modules/reitit-core/src/reitit/core.cljc +++ b/modules/reitit-core/src/reitit/core.cljc @@ -78,14 +78,15 @@ ;; 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 (or (when coercer - (coercer query-params :default)) - query-params)] + 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))))))) From 5f1046553387ba7fdf0de665dd124dd1f79dc885 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Wed, 22 Jan 2025 10:34:20 +0200 Subject: [PATCH 05/16] Another test case --- test/cljc/reitit/coercion_test.cljc | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/cljc/reitit/coercion_test.cljc b/test/cljc/reitit/coercion_test.cljc index 6ab91745..baedd1d3 100644 --- a/test/cljc/reitit/coercion_test.cljc +++ b/test/cljc/reitit/coercion_test.cljc @@ -186,4 +186,30 @@ (is (= {:query {:x [:a :b]}} (-> (r/match-by-path router "/olipa/kerran") (assoc :query-params {:x "a,b"}) + (coercion/coerce!)))))) + + (testing "encoding and multiple query param values" + (let [router (r/router ["/:a/:b" + {:name ::route + :coercion reitit.coercion.malli/coercion + :parameters {:query [:map + [:x + [:vector + [:keyword + {:decode/string (fn [s] + ;; Encoding coercer calls decoder -> validate -> encoder + ;; Decoder doesn't need to do anything as in this case the value is already "decoded" + (if (string? s) + (keyword (subs s 2)) + s)) + :encode/string (fn [k] (str "__" (name k)))}]]]]}}] + {:compile coercion/compile-request-coercers})] + (is (= "/olipa/kerran?x=__a&x=__b" + (-> router + (r/match-by-name! ::route {:a "olipa", :b "kerran"}) + (r/match->path {:x [:a :b]})))) + + (is (= {:query {:x [:a :b]}} + (-> (r/match-by-path router "/olipa/kerran") + (assoc :query-params {:x ["__a" "__b"]}) (coercion/coerce!))))))) From 21e5840f13bad3d6ec76c2f305f4f82e327bb586 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Wed, 22 Jan 2025 11:02:43 +0200 Subject: [PATCH 06/16] Ensure extra query-string params aren't removed by coercion --- .../src/reitit/coercion/malli.cljc | 8 +++- test/cljc/reitit/coercion_test.cljc | 15 ++++++- test/cljs/reitit/frontend/easy_test.cljs | 27 ++++++++++--- test/cljs/reitit/frontend/history_test.cljs | 39 +++++++++++++------ 4 files changed, 70 insertions(+), 19 deletions(-) diff --git a/modules/reitit-malli/src/reitit/coercion/malli.cljc b/modules/reitit-malli/src/reitit/coercion/malli.cljc index 3416079a..1bf4fc9b 100644 --- a/modules/reitit-malli/src/reitit/coercion/malli.cljc +++ b/modules/reitit-malli/src/reitit/coercion/malli.cljc @@ -178,6 +178,12 @@ (-response-coercer [_ schema] (-coercer schema :response transformers :encode opts)) (-query-string-coercer [_ schema] - (-coercer schema :string transformers :encode opts)))))) + ;; TODO: Create encoding function that only does encode, no decoding and validation? + (-coercer (mu/open-schema schema) + :string + ;; Tune transformer to not strip extra keys + {:string {:default (-transformer string-transformer-provider (assoc opts :strip-extra-keys false))}} + :encode + opts)))))) (def coercion (create default-options)) diff --git a/test/cljc/reitit/coercion_test.cljc b/test/cljc/reitit/coercion_test.cljc index baedd1d3..88f763cb 100644 --- a/test/cljc/reitit/coercion_test.cljc +++ b/test/cljc/reitit/coercion_test.cljc @@ -160,7 +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]})))))) + (r/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"})))))) (testing "custom encode/string for a collection" (let [router (r/router ["/:a/:b" @@ -183,6 +189,13 @@ (r/match-by-name! ::route {:a "olipa", :b "kerran"}) (r/match->path {: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] + :extra "extra-param"}))))) + (is (= {:query {:x [:a :b]}} (-> (r/match-by-path router "/olipa/kerran") (assoc :query-params {:x "a,b"}) diff --git a/test/cljs/reitit/frontend/easy_test.cljs b/test/cljs/reitit/frontend/easy_test.cljs index e36d9eee..63e4ecb5 100644 --- a/test/cljs/reitit/frontend/easy_test.cljs +++ b/test/cljs/reitit/frontend/easy_test.cljs @@ -1,16 +1,30 @@ (ns reitit.frontend.easy-test - (:require [clojure.test :refer [deftest testing is are async]] + (:require [clojure.string :as str] + [clojure.test :refer [are async deftest is testing]] + [goog.events :as gevents] + [reitit.coercion.malli :as rcm] [reitit.core :as r] [reitit.frontend.easy :as rfe] - [reitit.frontend.history :as rfh] - [goog.events :as gevents])) + [reitit.frontend.history :as rfh])) (def browser (exists? js/window)) (def router (r/router ["/" ["" ::frontpage] ["foo" ::foo] - ["bar/:id" ::bar]])) + ["bar/:id" + {:name ::bar + :coercion rcm/coercion + :parameters {:query [:map + [:q {:optional true} + [:keyword + {:decode/string (fn [s] + (if (string? s) + (keyword (if (str/starts-with? s "__") + (subs s 2) + s)) + s)) + :encode/string (fn [k] (str "__" (name k)))}]]]}}]])) ;; TODO: Only tests fragment history, also test HTML5? @@ -38,10 +52,11 @@ ;; 0. / 3 (do (is (= "/" url) "go back") - (rfe/navigate ::bar {:path-params {:id 1}})) + (rfe/navigate ::bar {:path-params {:id 1} + :query-params {:q "x"}})) ;; 0. / ;; 1. /bar/1 - 4 (do (is (= "/bar/1" url) + 4 (do (is (= "/bar/1?q=__x" url) "push-state 2") (rfe/replace-state ::bar {:id 2})) ;; 0. / diff --git a/test/cljs/reitit/frontend/history_test.cljs b/test/cljs/reitit/frontend/history_test.cljs index 4d3951be..5bbcfbee 100644 --- a/test/cljs/reitit/frontend/history_test.cljs +++ b/test/cljs/reitit/frontend/history_test.cljs @@ -3,14 +3,28 @@ [reitit.core :as r] [reitit.frontend.history :as rfh] [reitit.frontend.test-utils :refer [capture-console]] - [goog.events :as gevents])) + [goog.events :as gevents] + [reitit.coercion.malli :as rcm] + [clojure.string :as str])) (def browser (exists? js/window)) (def router (r/router ["/" ["" ::frontpage] ["foo" ::foo] - ["bar/:id" ::bar]])) + ["bar/:id" + {:name ::bar + :coercion rcm/coercion + :parameters {:query [:map + [:q {:optional true} + [:keyword + {:decode/string (fn [s] + (if (string? s) + (keyword (if (str/starts-with? s "__") + (subs s 2) + s)) + s)) + :encode/string (fn [k] (str "__" (name k)))}]]]}}]])) (deftest fragment-history-test (when browser @@ -24,9 +38,12 @@ (rfh/href history ::foo))) (is (= "#/bar/5" (rfh/href history ::bar {:id 5}))) - (is (= "#/bar/5?q=x" + (testing "query string coercion doesn't strip extra keys" + (is (= "#/bar/5?extra=a" + (rfh/href history ::bar {:id 5} {:extra "a"})))) + (is (= "#/bar/5?q=__x" (rfh/href history ::bar {:id 5} {:q "x"}))) - (is (= "#/bar/5?q=x#foo" + (is (= "#/bar/5?q=__x#foo" (rfh/href history ::bar {:id 5} {:q "x"} "foo"))) (let [{:keys [value messages]} (capture-console (fn [] @@ -58,11 +75,11 @@ (.back js/window.history)) 4 (do (is (= "/" url) "go back") - (rfh/push-state history ::bar {:id 1})) - 5 (do (is (= "/bar/1" url) + (rfh/push-state history ::bar {:id 1} {:extra "a"})) + 5 (do (is (= "/bar/1?extra=a" url) "push-state 2") - (rfh/replace-state history ::bar {:id 2})) - 6 (do (is (= "/bar/2" url) + (rfh/replace-state history ::bar {:id 2} {:q "x"})) + 6 (do (is (= "/bar/2?q=__x" url) "replace-state") (.back js/window.history)) 7 (do (is (= "/" url) @@ -84,7 +101,7 @@ (rfh/href history ::foo))) (is (= "/bar/5" (rfh/href history ::bar {:id 5}))) - (is (= "/bar/5?q=x" + (is (= "/bar/5?q=__x" (rfh/href history ::bar {:id 5} {:q "x"}))) (let [{:keys [value messages]} (capture-console (fn [] @@ -119,8 +136,8 @@ (rfh/push-state history ::bar {:id 1})) 5 (do (is (= "/bar/1" url) "push-state 2") - (rfh/replace-state history ::bar {:id 2})) - 6 (do (is (= "/bar/2" url) + (rfh/replace-state history ::bar {:id 2} {:q "x"})) + 6 (do (is (= "/bar/2?q=__x" url) "replace-state") (.back js/window.history)) 7 (do (is (= "/" url) From 1ba77a72675eeda51bfabadf2135763e6adf2aad Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Wed, 22 Jan 2025 11:10:39 +0200 Subject: [PATCH 07/16] Apply query parameters encoding on rfe/set-query --- .../reitit-frontend/src/reitit/frontend/history.cljs | 11 ++++++++--- test/cljs/reitit/frontend/easy_test.cljs | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/modules/reitit-frontend/src/reitit/frontend/history.cljs b/modules/reitit-frontend/src/reitit/frontend/history.cljs index 86043ddc..e8259894 100644 --- a/modules/reitit-frontend/src/reitit/frontend/history.cljs +++ b/modules/reitit-frontend/src/reitit/frontend/history.cljs @@ -289,13 +289,18 @@ New query params can be given as a map, or a function taking the old params and returning the new modified params. - Note: The query parameter values aren't coereced, so the - update fn will see string values for all query params." + The current path is matched against the routing tree, and the match data + (schema, coercion) is used to encode the query parameters." ([history new-query-or-update-fn] (set-query history new-query-or-update-fn nil)) ([history new-query-or-update-fn {:keys [replace] :as opts}] (let [current-path (-get-path history) - new-path (rf/set-query-params current-path new-query-or-update-fn)] + ;; FIXME: What if there is no match? + match (rf/match-by-path (.-router history) current-path) + query-params (if (fn? new-query-or-update-fn) + (new-query-or-update-fn (:query (:parameters match))) + new-query-or-update-fn) + new-path (rf/match->path match query-params (:fragment (:parameters match)))] (if replace (.replaceState js/window.history nil "" (-href history new-path)) (.pushState js/window.history nil "" (-href history new-path))) diff --git a/test/cljs/reitit/frontend/easy_test.cljs b/test/cljs/reitit/frontend/easy_test.cljs index 63e4ecb5..a82be64e 100644 --- a/test/cljs/reitit/frontend/easy_test.cljs +++ b/test/cljs/reitit/frontend/easy_test.cljs @@ -69,11 +69,11 @@ ;; 2. /bar/2?a=1 6 (do (is (= "/bar/2?a=1" url) "update-query with map") - (rfe/set-query #(assoc % :b "foo") {:replace true})) + (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&b=foo" url) + 7 (do (is (= "/bar/2?a=1&q=__x" url) "update-query with fn") (.go js/window.history -2)) ;; 0. / From f60a7ad9028999c56d170c591db335466e46b0fb Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Wed, 22 Jan 2025 11:14:35 +0200 Subject: [PATCH 08/16] Fixes --- test/cljs/reitit/frontend/core_test.cljs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/test/cljs/reitit/frontend/core_test.cljs b/test/cljs/reitit/frontend/core_test.cljs index 9bb054ba..559da199 100644 --- a/test/cljs/reitit/frontend/core_test.cljs +++ b/test/cljs/reitit/frontend/core_test.cljs @@ -310,16 +310,21 @@ x)) (deftest match->path-coercion-test - (testing "default Date toString" + (testing "default keyword to string" (is (str/starts-with? - (rf/match->path {:path "foo"} {:date (js/Date. 2024 0 1 12 13 0)}) - "foo?date=Mon+Jan+01+2024+12"))) + (rf/match->path {:path "foo"} {:q :x}) + "foo?q=x"))) - (is (= "foo?date=x2024-01-01T10%3A13%3A00.000Z" + (is (= "foo?q=__x" (rf/match->path {:data {:coercion rcm/coercion :parameters {:query [[:map - [:date {:decode/string string->instant - :encode/string instant->string} - :any]]]}} + [:q {:decode/string (fn [s] + (if (string? s) + (keyword (if (str/starts-with? s "__") + (subs s 2) + s)) + s)) + :encode/string (fn [k] (str "__" (name k)))} + :keyword]]]}} :path "foo"} - {:date (js/Date. 2024 0 1 12 13 0)})))) + {:q "x"})))) From 1b37c87aa24228af003ce8cb8f9d29f5af3555d1 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Wed, 22 Jan 2025 14:18:54 +0200 Subject: [PATCH 09/16] Test set-query without a match --- .../src/reitit/frontend/history.cljs | 18 +++++++++------ test/cljs/reitit/frontend/easy_test.cljs | 22 ++++++++++++++----- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/modules/reitit-frontend/src/reitit/frontend/history.cljs b/modules/reitit-frontend/src/reitit/frontend/history.cljs index e8259894..e9042772 100644 --- a/modules/reitit-frontend/src/reitit/frontend/history.cljs +++ b/modules/reitit-frontend/src/reitit/frontend/history.cljs @@ -290,17 +290,21 @@ the old params and returning the new modified params. The current path is matched against the routing tree, and the match data - (schema, coercion) is used to encode the query parameters." + (schema, coercion) is used to encode the query parameters. + If the current path doesn't match any route, the query parameters + are parsed from the path without coercion and new values + are also stored without coercion encoding." ([history new-query-or-update-fn] (set-query history new-query-or-update-fn nil)) ([history new-query-or-update-fn {:keys [replace] :as opts}] (let [current-path (-get-path history) - ;; FIXME: What if there is no match? - match (rf/match-by-path (.-router history) current-path) - query-params (if (fn? new-query-or-update-fn) - (new-query-or-update-fn (:query (:parameters match))) - new-query-or-update-fn) - new-path (rf/match->path match query-params (:fragment (:parameters match)))] + match (rf/match-by-path (:router history) current-path) + new-path (if match + (let [query-params (if (fn? new-query-or-update-fn) + (new-query-or-update-fn (:query (:parameters match))) + new-query-or-update-fn)] + (rf/match->path match query-params (:fragment (:parameters match)))) + (rf/set-query-params current-path new-query-or-update-fn))] (if replace (.replaceState js/window.history nil "" (-href history new-path)) (.pushState js/window.history nil "" (-href history new-path))) diff --git a/test/cljs/reitit/frontend/easy_test.cljs b/test/cljs/reitit/frontend/easy_test.cljs index a82be64e..bc5d9f8f 100644 --- a/test/cljs/reitit/frontend/easy_test.cljs +++ b/test/cljs/reitit/frontend/easy_test.cljs @@ -76,8 +76,20 @@ 7 (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") + (.pushState js/window.history nil "" "#/non-matching-path")) + + 9 (do (is (= "/non-matching-path" url)) + (rfe/set-query #(assoc % :q "x"))) + + 10 (do (is (= "/non-matching-path?q=x" url)) + (.go js/window.history -2)) + ;; 0. / - 8 (do (is (= "/" url) + 11 (do (is (= "/" url) "go back two events") ;; Reset to ensure old event listeners aren't called @@ -85,10 +97,10 @@ (fn on-navigate [match history] (let [url (rfh/-get-path history)] (case (swap! n inc) - 9 (do (is (= "/" url) - "start at root") - (rfe/push-state ::foo)) - 10 (do (is (= "/foo" url) + 12 (do (is (= "/" url) + "start at root") + (rfe/push-state ::foo)) + 13 (do (is (= "/foo" url) "push-state") (rfh/stop! @rfe/history) (done)) From 7ae2bfafc2345d7198a247015ba857453c639810 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Wed, 22 Jan 2025 14:20:02 +0200 Subject: [PATCH 10/16] Cleanup --- test/cljs/reitit/frontend/core_test.cljs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/test/cljs/reitit/frontend/core_test.cljs b/test/cljs/reitit/frontend/core_test.cljs index 559da199..0fe6ec31 100644 --- a/test/cljs/reitit/frontend/core_test.cljs +++ b/test/cljs/reitit/frontend/core_test.cljs @@ -299,16 +299,6 @@ (is (= "foo#foo+bar+%25" (rf/match->path {:path "foo"} nil "foo bar %"))))) -(defn instant->string - [x] - (str "x" (.toISOString x))) - -(defn string->instant - [x] - (if (string? x) - (js/Date. (subs x 1 (count x))) - x)) - (deftest match->path-coercion-test (testing "default keyword to string" (is (str/starts-with? From ce6d9e26cdda2718c4e830cd75fbe55bf94cf58e Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Wed, 22 Jan 2025 14:35:56 +0200 Subject: [PATCH 11/16] Update docstrings and changelog --- CHANGELOG.md | 6 ++++ .../src/reitit/frontend/easy.cljs | 35 +++++++++++-------- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c6a455c..66809b52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ We use [Break Versioning][breakver]. The version numbers follow a `. Date: Tue, 28 Jan 2025 14:34:21 +0200 Subject: [PATCH 12/16] Move query string coercion to coercion ns from core --- modules/reitit-core/src/reitit/coercion.cljc | 31 ++++++++++ modules/reitit-core/src/reitit/core.cljc | 21 +------ .../reitit-frontend/src/reitit/frontend.cljs | 8 ++- test/cljc/reitit/coercion_test.cljc | 22 +++---- test/cljs/reitit/frontend/easy_test.cljs | 57 ++++++++++--------- 5 files changed, 80 insertions(+), 59 deletions(-) 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))))) From 7e9116f77ef920909401fa6d701f21919dbae7a4 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Tue, 28 Jan 2025 15:09:31 +0200 Subject: [PATCH 13/16] Simplify Malli coercion for query-params to only encode --- .../src/reitit/coercion/malli.cljc | 22 +++++++++++++------ test/cljc/reitit/coercion_test.cljc | 12 +++------- test/cljs/reitit/frontend/core_test.cljs | 7 +----- test/cljs/reitit/frontend/easy_test.cljs | 10 ++------- test/cljs/reitit/frontend/history_test.cljs | 10 ++------- 5 files changed, 23 insertions(+), 38 deletions(-) diff --git a/modules/reitit-malli/src/reitit/coercion/malli.cljc b/modules/reitit-malli/src/reitit/coercion/malli.cljc index 1bf4fc9b..0f14ab7f 100644 --- a/modules/reitit-malli/src/reitit/coercion/malli.cljc +++ b/modules/reitit-malli/src/reitit/coercion/malli.cljc @@ -76,6 +76,20 @@ (assoc error :transformed transformed)))) value)))))))) +(defn- -query-string-coercer + "Create coercer for query-parameters, always allows extra params and does + encoding using string-transformer." + [schema transfomers options] + (let [;; Always allow extra paramaters on query-parameters encoding + open-schema (mu/open-schema schema) + ;; Do not remove extra keys + string-transformer (-transformer string-transformer-provider (assoc options :strip-extra-keys false)) + encoder (m/encoder open-schema options string-transformer)] + (fn [value format] + (if encoder + (encoder value) + value)))) + ;; ;; public api ;; @@ -178,12 +192,6 @@ (-response-coercer [_ schema] (-coercer schema :response transformers :encode opts)) (-query-string-coercer [_ schema] - ;; TODO: Create encoding function that only does encode, no decoding and validation? - (-coercer (mu/open-schema schema) - :string - ;; Tune transformer to not strip extra keys - {:string {:default (-transformer string-transformer-provider (assoc opts :strip-extra-keys false))}} - :encode - opts)))))) + (-query-string-coercer schema transformers opts)))))) (def coercion (create default-options)) diff --git a/test/cljc/reitit/coercion_test.cljc b/test/cljc/reitit/coercion_test.cljc index 0e072214..0fcfd125 100644 --- a/test/cljc/reitit/coercion_test.cljc +++ b/test/cljc/reitit/coercion_test.cljc @@ -178,9 +178,7 @@ {:encode/string (fn [xs] (str/join "," (map name xs))) :decode/string (fn [s] - (if (string? s) - (mapv keyword (str/split s #",")) - s))} + (mapv keyword (str/split s #",")))} :keyword]]]}}] {:compile coercion/compile-request-coercers}) match (r/match-by-name! router ::route {:a "olipa", :b "kerran"})] @@ -211,12 +209,8 @@ [:x [:vector [:keyword - {:decode/string (fn [s] - ;; Encoding coercer calls decoder -> validate -> encoder - ;; Decoder doesn't need to do anything as in this case the value is already "decoded" - (if (string? s) - (keyword (subs s 2)) - s)) + ;; For query strings encode only calls encode, so no need to check if decode if value is encoded or not. + {:decode/string (fn [s] (keyword (subs s 2))) :encode/string (fn [k] (str "__" (name k)))}]]]]}}] {:compile coercion/compile-request-coercers})] (is (= "/olipa/kerran?x=__a&x=__b" diff --git a/test/cljs/reitit/frontend/core_test.cljs b/test/cljs/reitit/frontend/core_test.cljs index 0fe6ec31..c99fba12 100644 --- a/test/cljs/reitit/frontend/core_test.cljs +++ b/test/cljs/reitit/frontend/core_test.cljs @@ -308,12 +308,7 @@ (is (= "foo?q=__x" (rf/match->path {:data {:coercion rcm/coercion :parameters {:query [[:map - [:q {:decode/string (fn [s] - (if (string? s) - (keyword (if (str/starts-with? s "__") - (subs s 2) - s)) - s)) + [:q {:decode/string (fn [s] (keyword (subs s 2))) :encode/string (fn [k] (str "__" (name k)))} :keyword]]]}} :path "foo"} diff --git a/test/cljs/reitit/frontend/easy_test.cljs b/test/cljs/reitit/frontend/easy_test.cljs index d66935d5..1e0284ad 100644 --- a/test/cljs/reitit/frontend/easy_test.cljs +++ b/test/cljs/reitit/frontend/easy_test.cljs @@ -1,6 +1,5 @@ (ns reitit.frontend.easy-test - (:require [clojure.string :as str] - [clojure.test :refer [are async deftest is testing]] + (:require [clojure.test :refer [are async deftest is testing]] [goog.events :as gevents] [reitit.coercion.malli :as rcm] [reitit.core :as r] @@ -18,12 +17,7 @@ :parameters {:query [:map [:q {:optional true} [:keyword - {:decode/string (fn [s] - (if (string? s) - (keyword (if (str/starts-with? s "__") - (subs s 2) - s)) - s)) + {:decode/string (fn [s] (keyword (subs s 2))) :encode/string (fn [k] (str "__" (name k)))}]]]}}]])) ;; TODO: Only tests fragment history, also test HTML5? diff --git a/test/cljs/reitit/frontend/history_test.cljs b/test/cljs/reitit/frontend/history_test.cljs index 5bbcfbee..55aea67a 100644 --- a/test/cljs/reitit/frontend/history_test.cljs +++ b/test/cljs/reitit/frontend/history_test.cljs @@ -4,8 +4,7 @@ [reitit.frontend.history :as rfh] [reitit.frontend.test-utils :refer [capture-console]] [goog.events :as gevents] - [reitit.coercion.malli :as rcm] - [clojure.string :as str])) + [reitit.coercion.malli :as rcm])) (def browser (exists? js/window)) @@ -18,12 +17,7 @@ :parameters {:query [:map [:q {:optional true} [:keyword - {:decode/string (fn [s] - (if (string? s) - (keyword (if (str/starts-with? s "__") - (subs s 2) - s)) - s)) + {:decode/string (fn [s] (keyword (subs s 2))) :encode/string (fn [k] (str "__" (name k)))}]]]}}]])) (deftest fragment-history-test From dfc5a4ef67adf1708cbca648a5827fd3dadd3c15 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Tue, 28 Jan 2025 15:12:46 +0200 Subject: [PATCH 14/16] Remove todo comments --- modules/reitit-schema/src/reitit/coercion/schema.cljc | 1 - modules/reitit-spec/src/reitit/coercion/spec.cljc | 1 - 2 files changed, 2 deletions(-) diff --git a/modules/reitit-schema/src/reitit/coercion/schema.cljc b/modules/reitit-schema/src/reitit/coercion/schema.cljc index adbbcf05..d3b37d9a 100644 --- a/modules/reitit-schema/src/reitit/coercion/schema.cljc +++ b/modules/reitit-schema/src/reitit/coercion/schema.cljc @@ -103,7 +103,6 @@ (if (coerce-response? schema) (coercion/-request-coercer this :response schema))) (-query-string-coercer [this schema] - ;; TODO: Can this be implemented? nil))) (def coercion (create default-options)) diff --git a/modules/reitit-spec/src/reitit/coercion/spec.cljc b/modules/reitit-spec/src/reitit/coercion/spec.cljc index 56c6e6a7..004cda93 100644 --- a/modules/reitit-spec/src/reitit/coercion/spec.cljc +++ b/modules/reitit-spec/src/reitit/coercion/spec.cljc @@ -150,7 +150,6 @@ (if (coerce-response? spec) (coercion/-request-coercer this :response spec))) (-query-string-coercer [this spec] - ;; TODO: Can this be implemented? nil))) (def coercion (create default-options)) From 4eb29d3ed9d627181411b522a236ca58197652d6 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Tue, 28 Jan 2025 15:46:37 +0200 Subject: [PATCH 15/16] Extend frontend docs --- doc/frontend/basics.md | 7 ++- doc/frontend/browser.md | 5 +- doc/frontend/coercion.md | 59 +++++++++++++++++++ .../src/reitit/frontend/history.cljs | 28 +++++---- 4 files changed, 85 insertions(+), 14 deletions(-) create mode 100644 doc/frontend/coercion.md diff --git a/doc/frontend/basics.md b/doc/frontend/basics.md index 6c049075..bdc18410 100644 --- a/doc/frontend/basics.md +++ b/doc/frontend/basics.md @@ -8,6 +8,10 @@ history events - Stateful wrapper for easy use of history integration - Optional [controller extension](./controllers.md) +You likely won't use `reitit.frontend` directly in your apps and instead you +will use the API documented in the browser integration docs, which wraps these +lower level functions. + ## Core functions `reitit.frontend` provides some useful functions wrapping core functions: @@ -23,7 +27,8 @@ enabled. `match-by-name` and `match-by-name!` with optional `path-paramers` and logging errors to `console.warn` instead of throwing errors to prevent -React breaking due to errors. +React breaking due to errors. These can also [encode query-parameters](./coercion.md) +using schema from match data. ## Next diff --git a/doc/frontend/browser.md b/doc/frontend/browser.md index 1541265b..4024fe26 100644 --- a/doc/frontend/browser.md +++ b/doc/frontend/browser.md @@ -13,6 +13,9 @@ There are also secondary functions following HTML5 History API: `push-state` to navigate to new route adding entry to the history and `replace-state` to change route without leaving previous entry in browser history. +See [coercion notes](./coercion.md) to see how frontend route parameters +can be decoded and encoded. + ## Fragment router Fragment is simple integration which stores the current route in URL fragment, @@ -62,7 +65,7 @@ event handler for page change events. ## History manipulation -Reitit doesn't include functions to manipulate the history stack, i.e. +Reitit doesn't include functions to manipulate the history stack, i.e., go back or forwards, but calling History API functions directly should work: ``` diff --git a/doc/frontend/coercion.md b/doc/frontend/coercion.md new file mode 100644 index 00000000..a58e57f5 --- /dev/null +++ b/doc/frontend/coercion.md @@ -0,0 +1,59 @@ +# Frontend coercion + +The Reitit frontend leverages [coercion](../coercion/coercion.md) for path, +query, and fragment parameters. The coercion uses the input schema defined +in the match data under `:parameters`. + +## Behavior of Coercion + +1. **Route Matching** + When matching a route from a path, the resulting match will include the + coerced values (if coercion is enabled) under `:parameters`. If coercion is + disabled, the parsed string values are stored in the same location. + The original un-coerced values are always available under `:path-params`, + `:query-params`, and `:fragment` (a single string). + +2. **Creating Links and Navigating** + When generating a URL (`href`) or navigating (`push-state`, `replace-state`, `navigate`) + to a route, coercion can be + used to encode query-parameter values into strings. This happens before + Reitit performs basic URL encoding on the values. This feature is + especially useful for handling the encoding of specific types, such as + keywords or dates, into strings. + +3. **Updating current query parameters** + When using `set-query` to modify current query parameters, Reitit frontend + first tries to find a match for the current path so the match can be used to + first decode query parameters and then to encode them. If the current path + doesn't match the routing tree, `set-query` keeps all the query parameter + values as strings. + +## Notes + +- **Value Encoding Support**: Only Malli supports value encoding. +- **Limitations**: Path parameters and fragment values are not encoded using + the match schema. + +## Example + +```cljs +(def router (r/router ["/" + ["" ::frontpage] + ["bar" + {:name ::bar + :coercion rcm/coercion + :parameters {:query [:map + [:q {:optional true} + [:keyword + {:decode/string (fn [s] (keyword (subs s 2))) + :encode/string (fn [k] (str "__" (name k)))}]]]}}]])) + +(rfe/href ::bar {} {:q :hello}) +;; Result "/bar?q=__hello", the :q value is first encoded + +(rfe/push-state ::bar {} {:q :world}) +;; Result "/bar?q=__world" +;; The current match will contain both the original value and parsed & decoded parameters: +;; {:query-params {:q "__world"} +;; :parameters {:query {:q :world}}} +``` diff --git a/modules/reitit-frontend/src/reitit/frontend/history.cljs b/modules/reitit-frontend/src/reitit/frontend/history.cljs index e9042772..adb446f9 100644 --- a/modules/reitit-frontend/src/reitit/frontend/history.cljs +++ b/modules/reitit-frontend/src/reitit/frontend/history.cljs @@ -187,9 +187,10 @@ The URL is formatted using Reitit frontend history handler, so using it with anchor element href will correctly trigger route change event. - Note: currently collections in query parameters are encoded as field-value - pairs separated by &, i.e. \"?a=1&a=2\", if you want to encode them - differently, convert the collections to strings first." + By default currently collections in query parameters are encoded as field-value + pairs separated by &, i.e. \"?a=1&a=2\". To encode them differently, you can + either use Malli coercion to encode values, or just turn the values to strings + before calling the function." ([history name] (href history name nil)) ([history name path-params] @@ -208,9 +209,10 @@ Will also trigger on-navigate callback on Reitit frontend History handler. - Note: currently collections in query-parameters are encoded as field-value - pairs separated by &, i.e. \"?a=1&a=2\", if you want to encode them - differently, convert the collections to strings first. + By default currently collections in query parameters are encoded as field-value + pairs separated by &, i.e. \"?a=1&a=2\". To encode them differently, you can + either use Malli coercion to encode values, or just turn the values to strings + before calling the function. See also: https://developer.mozilla.org/en-US/docs/Web/API/History/pushState" @@ -236,9 +238,10 @@ Will also trigger on-navigate callback on Reitit frontend History handler. - Note: currently collections in query-parameters are encoded as field-value - pairs separated by &, i.e. \"?a=1&a=2\", if you want to encode them - differently, convert the collections to strings first. + By default currently collections in query parameters are encoded as field-value + pairs separated by &, i.e. \"?a=1&a=2\". To encode them differently, you can + either use Malli coercion to encode values, or just turn the values to strings + before calling the function. See also: https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState" @@ -264,9 +267,10 @@ Will also trigger on-navigate callback on Reitit frontend History handler. - Note: currently collections in query-parameters are encoded as field-value - pairs separated by &, i.e. \"?a=1&a=2\", if you want to encode them - differently, convert the collections to strings first. + By default currently collections in query parameters are encoded as field-value + pairs separated by &, i.e. \"?a=1&a=2\". To encode them differently, you can + either use Malli coercion to encode values, or just turn the values to strings + before calling the function. See also: https://developer.mozilla.org/en-US/docs/Web/API/History/pushState From 5ca22193d02a4c589de8c763194d7825439edcdd Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Fri, 31 Jan 2025 09:39:19 +0200 Subject: [PATCH 16/16] Use defined :string :default transformer for query-string-coercer --- .../src/reitit/coercion/malli.cljc | 14 +++++--- test/cljs/reitit/frontend/core_test.cljs | 33 ++++++++++++++----- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/modules/reitit-malli/src/reitit/coercion/malli.cljc b/modules/reitit-malli/src/reitit/coercion/malli.cljc index 0f14ab7f..813bbc20 100644 --- a/modules/reitit-malli/src/reitit/coercion/malli.cljc +++ b/modules/reitit-malli/src/reitit/coercion/malli.cljc @@ -9,7 +9,8 @@ [malli.swagger :as swagger] [malli.transform :as mt] [malli.util :as mu] - [reitit.coercion :as coercion])) + [reitit.coercion :as coercion] + [clojure.string :as string])) ;; ;; coercion @@ -79,11 +80,13 @@ (defn- -query-string-coercer "Create coercer for query-parameters, always allows extra params and does encoding using string-transformer." - [schema transfomers options] + [schema string-transformer-provider options] (let [;; Always allow extra paramaters on query-parameters encoding open-schema (mu/open-schema schema) ;; Do not remove extra keys - string-transformer (-transformer string-transformer-provider (assoc options :strip-extra-keys false)) + string-transformer (if (satisfies? TransformationProvider string-transformer-provider) + (-transformer string-transformer-provider (assoc options :strip-extra-keys false)) + string-transformer-provider) encoder (m/encoder open-schema options string-transformer)] (fn [value format] (if encoder @@ -126,6 +129,9 @@ ([opts] (let [{:keys [transformers lite compile options error-keys encode-error] :as opts} (merge default-options opts) show? (fn [key] (contains? error-keys key)) + ;; Query-string-coercer needs to construct transfomer without strip-extra-keys so it will + ;; use the transformer-provider directly. + string-transformer-provider (:default (:string transformers)) transformers (walk/prewalk #(if (satisfies? TransformationProvider %) (-transformer % opts) %) transformers) compile (if lite (fn [schema options] (compile (binding [l/*options* options] (l/schema schema)) options)) @@ -192,6 +198,6 @@ (-response-coercer [_ schema] (-coercer schema :response transformers :encode opts)) (-query-string-coercer [_ schema] - (-query-string-coercer schema transformers opts)))))) + (-query-string-coercer schema string-transformer-provider opts)))))) (def coercion (create default-options)) diff --git a/test/cljs/reitit/frontend/core_test.cljs b/test/cljs/reitit/frontend/core_test.cljs index c99fba12..3bb7907f 100644 --- a/test/cljs/reitit/frontend/core_test.cljs +++ b/test/cljs/reitit/frontend/core_test.cljs @@ -1,6 +1,8 @@ (ns reitit.frontend.core-test (:require [clojure.string :as str] [clojure.test :refer [are deftest is testing]] + [malli.core :as m] + [malli.transform :as mt] [reitit.coercion :as rc] [reitit.coercion.malli :as rcm] [reitit.coercion.schema :as rcs] @@ -305,11 +307,26 @@ (rf/match->path {:path "foo"} {:q :x}) "foo?q=x"))) - (is (= "foo?q=__x" - (rf/match->path {:data {:coercion rcm/coercion - :parameters {:query [[:map - [:q {:decode/string (fn [s] (keyword (subs s 2))) - :encode/string (fn [k] (str "__" (name k)))} - :keyword]]]}} - :path "foo"} - {:q "x"})))) + (testing "default string transformer" + (is (= "foo?q=__x" + (rf/match->path {:data {:coercion rcm/coercion + :parameters {:query [[:map + [:q {:decode/string (fn [s] (keyword (subs s 2))) + :encode/string (fn [k] (str "__" (name k)))} + :keyword]]]}} + :path "foo"} + {:q "x"})))) + + (testing "custom string transformer" + (is (= "foo?q=--x" + (rf/match->path {:data {:coercion (rcm/create (assoc-in rcm/default-options + [:transformers :string :default] + (mt/transformer + {:name :foo-string + :encoders {:foo/type {:leave (fn [x] (str "--" x))}}}))) + :parameters {:query [[:map + [:q (m/-simple-schema + {:type :foo/type + :pred string?})]]]}} + :path "foo"} + {:q "x"})))))