Merge pull request #716 from metosin/query-string-encoding

Use coercion to encode query-string values in match->path

CHANGELOG.md

@@ -15,6 +15,12 @@ We use [Break Versioning][breakver]. The version numbers follow a `<major>.<mino
 ## UNRELEASED
 
 * Improve OpenAPI docs, plus don't emit `:description` in the wrong place [#702](https://github.com/metosin/reitit/pull/702)
+* *POTENTIALLY BREAKING* The frontend functions (href, push/replace-state, set-query) now
+  encode query-string values using configured coercion when possible (only Malli supports encoding).
+    - You can use this to encode query parameter values before they are URL-encoded. This works for DateTimes, collections etc.
+    - In most cases this shouldn't break existing uses, but it is possible even without
+      a custom encoding function, the default Malli string-transformer could encode some values differently
+      then previously.
 
 ## 0.7.2 (2024-09-02)
 

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
 

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:
 
 ```

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}}}
+```

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))))
@@ -19,7 +20,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]
@@ -219,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))))

modules/reitit-core/src/reitit/core.cljc

@@ -68,10 +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) (str "?" (impl/query-string query-params))))))
+   (some-> match :path (cond-> (seq query-params)
+                         (str "?" (impl/query-string query-params))))))
 
 ;;
 ;; Different routers

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
+  "Create routing path from given match and optional query-parameters map and
-  optional fragment string."
+  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))))))
 

modules/reitit-frontend/src/reitit/frontend/easy.cljs

@@ -48,9 +48,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
+  By default 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
+  pairs separated by &, i.e. \"?a=1&a=2\". To encode them differently, you can
-  differently, convert the collections to strings first."
+  either use Malli coercion to encode values, or just turn the values to strings
+  before calling the function."
   ([name]
    (rfh/href @history name nil nil nil))
   ([name path-params]
@@ -69,9 +70,10 @@
 
   Will also trigger on-navigate callback on Reitit frontend History handler.
 
-  Note: currently collections in query parameters are encoded as field-value
+  By default 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
+  pairs separated by &, i.e. \"?a=1&a=2\". To encode them differently, you can
-  differently, convert the collections to strings first.
+  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"
@@ -93,9 +95,10 @@
 
   Will also trigger on-navigate callback on Reitit frontend History handler.
 
-  Note: currently collections in query-parameters are encoded as field-value
+  By default 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
+  pairs separated by &, i.e. \"?a=1&a=2\". To encode them differently, you can
-  differently, convert the collections to strings first.
+  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"
@@ -122,9 +125,10 @@
 
   Will also trigger on-navigate callback on Reitit frontend History handler.
 
-  Note: currently collections in query-parameters are encoded as field-value
+  By default 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
+  pairs separated by &, i.e. \"?a=1&a=2\". To encode them differently, you can
-  differently, convert the collections to strings first.
+  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
@@ -142,8 +146,11 @@
   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
+  The current path is matched against the routing tree, and the match data
-  update fn will see string values for all query params."
+  (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."
   ([new-query-or-update-fn]
    (rfh/set-query @history new-query-or-update-fn))
   ([new-query-or-update-fn {:keys [replace] :as opts}]

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
+  By default 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
+  pairs separated by &, i.e. \"?a=1&a=2\". To encode them differently, you can
-  differently, convert the collections to strings first."
+  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
+  By default 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
+  pairs separated by &, i.e. \"?a=1&a=2\". To encode them differently, you can
-  differently, convert the collections to strings first.
+  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
+  By default 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
+  pairs separated by &, i.e. \"?a=1&a=2\". To encode them differently, you can
-  differently, convert the collections to strings first.
+  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
+  By default 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
+  pairs separated by &, i.e. \"?a=1&a=2\". To encode them differently, you can
-  differently, convert the collections to strings first.
+  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
@@ -289,13 +293,22 @@
   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
+  The current path is matched against the routing tree, and the match data
-  update fn will see string values for all query params."
+  (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)
-         new-path (rf/set-query-params current-path new-query-or-update-fn)]
+         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)))

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
@@ -76,6 +77,22 @@
                      (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 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 (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
+        (encoder value)
+        value))))
+
 ;;
 ;; public api
 ;;
@@ -112,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))
@@ -176,6 +196,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]
+         (-query-string-coercer schema string-transformer-provider opts))))))
 
 (def coercion (create default-options))

modules/reitit-schema/src/reitit/coercion/schema.cljc

@@ -101,6 +101,8 @@
             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]
+      nil)))
 
 (def coercion (create default-options))

modules/reitit-spec/src/reitit/coercion/spec.cljc

@@ -148,6 +148,8 @@
             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]
+      nil)))
 
 (def coercion (create default-options))

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,72 @@
                 {: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"})
+                 (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"})
+                 (coercion/match->path {:x [:a :b]
+                                        :extra "extra-param"}))))))
+
+  (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]
+                                                                      (mapv keyword (str/split s #",")))}
+                                                    :keyword]]]}}]
+                           {: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"
+             (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"})
+                   (coercion/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"})
+                 (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
+                                                     ;; 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"
+             (-> router
+                 (r/match-by-name! ::route {:a "olipa", :b "kerran"})
+                 (coercion/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!)))))))

test/cljs/reitit/frontend/core_test.cljs

@@ -1,13 +1,16 @@
 (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]]
+            [malli.core :as m]
+            [malli.transform :as mt]
+            [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 +300,33 @@
   (testing "Fragment encoding"
     (is (= "foo#foo+bar+%25"
            (rf/match->path {:path "foo"} nil "foo bar %")))))
+
+(deftest match->path-coercion-test
+  (testing "default keyword to string"
+    (is (str/starts-with?
+          (rf/match->path {:path "foo"} {:q :x})
+          "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"})))))

test/cljs/reitit/frontend/easy_test.cljs

@@ -1,16 +1,24 @@
 (ns reitit.frontend.easy-test
-  (:require [clojure.test :refer [deftest testing is are async]]
+  (:require [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]
+            [reitit.frontend.history :as rfh]))
-            [goog.events :as gevents]))
 
 (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] (keyword (subs s 2)))
+                                                 :encode/string (fn [k] (str "__" (name k)))}]]]}}]]))
 
 ;; TODO: Only tests fragment history, also test HTML5?
 
@@ -26,61 +34,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}}))
+                                (rfe/navigate ::bar {:path-params {:id 1}
+                                                     :query-params {:q "x"}}))
                           ;; 0. /
                           ;; 1. /bar/1
-                          4 (do (is (= "/bar/1" 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 % :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)
+                          8 (do (is (= "/bar/2?a=1&q=__x" url)
                                     "update-query with fn")
                                 (.go js/window.history -2))
-                          ;; 0. /
-                          8 (do (is (= "/" url)
-                                    "go back two events")
 
-                                ;; Reset to ensure old event listeners aren't called
+                          ;; Go to non-matching path and check set-query works
-                                (rfe/start! router
+                          ;; (without coercion) without a match
-                                            (fn on-navigate [match history]
+                          9 (do (is (= "/" url) "go back two events")
-                                              (let [url (rfh/-get-path history)]
+                                (.pushState js/window.history nil "" "#/non-matching-path"))
-                                                (case (swap! n inc)
+
-                                                  9 (do (is (= "/" url)
+                          10 (do (is (= "/non-matching-path" url))
-                                                            "start at root")
+                                (rfe/set-query #(assoc % :q "x")))
-                                                        (rfe/push-state ::foo))
+
-                                                  10 (do (is (= "/foo" url)
+                          11 (do (is (= "/non-matching-path?q=x" url))
-                                                             "push-state")
+                                 (.go js/window.history -2))
-                                                         (rfh/stop! @rfe/history)
+
-                                                         (done))
+                          ;; 0. /
-                                                  (do
+                          12 (do (is (= "/" url)
-                                                    (is false (str "extra event 2" {:n @n, :url url}))
+                                     "go back two events")
-                                                    (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)))))

test/cljs/reitit/frontend/history_test.cljs

@@ -3,14 +3,22 @@
             [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]))
 
 (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] (keyword (subs s 2)))
+                                                 :encode/string (fn [k] (str "__" (name k)))}]]]}}]]))
 
 (deftest fragment-history-test
   (when browser
@@ -24,9 +32,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 +69,11 @@
                                 (.back js/window.history))
                           4 (do (is (= "/" url)
                                     "go back")
-                                (rfh/push-state history ::bar {:id 1}))
+                                (rfh/push-state history ::bar {:id 1} {:extra "a"}))
-                          5 (do (is (= "/bar/1" url)
+                          5 (do (is (= "/bar/1?extra=a" url)
                                     "push-state 2")
-                                (rfh/replace-state history ::bar {:id 2}))
+                                (rfh/replace-state history ::bar {:id 2} {:q "x"}))
-                          6 (do (is (= "/bar/2" url)
+                          6 (do (is (= "/bar/2?q=__x" url)
                                     "replace-state")
                                 (.back js/window.history))
                           7 (do (is (= "/" url)
@@ -84,7 +95,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 +130,8 @@
                                 (rfh/push-state history ::bar {:id 1}))
                           5 (do (is (= "/bar/1" url)
                                     "push-state 2")
-                                (rfh/replace-state history ::bar {:id 2}))
+                                (rfh/replace-state history ::bar {:id 2} {:q "x"}))
-                          6 (do (is (= "/bar/2" url)
+                          6 (do (is (= "/bar/2?q=__x" url)
                                     "replace-state")
                                 (.back js/window.history))
                           7 (do (is (= "/" url)