From dd724f0d0e1143e4b89374c65214988232d29f3e Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Thu, 23 Mar 2023 15:13:27 +0200 Subject: [PATCH 1/7] Fix #600: Add frontend function to update query-params for current path --- modules/reitit-frontend/src/reitit/frontend.cljs | 11 +++++++++++ .../reitit-frontend/src/reitit/frontend/easy.cljs | 13 ++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/modules/reitit-frontend/src/reitit/frontend.cljs b/modules/reitit-frontend/src/reitit/frontend.cljs index faa258e0..70dd5482 100644 --- a/modules/reitit-frontend/src/reitit/frontend.cljs +++ b/modules/reitit-frontend/src/reitit/frontend.cljs @@ -88,3 +88,14 @@ match) (do (js/console.warn "missing route" name) nil)))) + +(defn update-path-query-params + "Given Reitit-frontend path, update the query params + with given function and arguments. + + NOTE: coercion is not applied to the query params" + [path f & args] + (let [^goog.Uri uri (Uri/parse path) + new-query (apply f (query-params uri) args)] + (.setQueryData uri (QueryData/createFromMap (clj->js new-query))) + (.toString uri))) diff --git a/modules/reitit-frontend/src/reitit/frontend/easy.cljs b/modules/reitit-frontend/src/reitit/frontend/easy.cljs index 8a76b900..45223817 100644 --- a/modules/reitit-frontend/src/reitit/frontend/easy.cljs +++ b/modules/reitit-frontend/src/reitit/frontend/easy.cljs @@ -2,7 +2,8 @@ "Easy wrapper over reitit.frontend.history, handling the state. Only one router can be active at a time." - (:require [reitit.frontend.history :as rfh])) + (:require [reitit.frontend.history :as rfh] + [reitit.frontend :as rf])) (defonce history (atom nil)) @@ -101,3 +102,13 @@ (rfh/replace-state @history name path-params nil)) ([name path-params query-params] (rfh/replace-state @history name path-params query-params))) + +(defn update-query + "Takes the current location and updates the query params + with given fn and arguments." + [f & args] + ;; TODO: rfh version? + (let [current-path (rfh/-get-path @history) + new-path (apply rf/update-path-query-params current-path f args)] + (.pushState js/window.history nil "" new-path) + (rfh/-on-navigate @history new-path))) From e3e93eaffb817ed5461b340c88d919aac77dd4f1 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Thu, 23 Mar 2023 15:36:48 +0200 Subject: [PATCH 2/7] Add some tests --- .../src/reitit/frontend/easy.cljs | 2 ++ test/cljs/reitit/frontend/core_test.cljs | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/modules/reitit-frontend/src/reitit/frontend/easy.cljs b/modules/reitit-frontend/src/reitit/frontend/easy.cljs index 45223817..4c2518cf 100644 --- a/modules/reitit-frontend/src/reitit/frontend/easy.cljs +++ b/modules/reitit-frontend/src/reitit/frontend/easy.cljs @@ -104,11 +104,13 @@ (rfh/replace-state @history name path-params query-params))) (defn update-query + ;; TODO: Sync the docstring with other namespaces "Takes the current location and updates the query params with given fn and arguments." [f & args] ;; TODO: rfh version? (let [current-path (rfh/-get-path @history) new-path (apply rf/update-path-query-params current-path f args)] + ;; TODO: replaceState version (.pushState js/window.history nil "" new-path) (rfh/-on-navigate @history new-path))) diff --git a/test/cljs/reitit/frontend/core_test.cljs b/test/cljs/reitit/frontend/core_test.cljs index d0684398..220861a1 100644 --- a/test/cljs/reitit/frontend/core_test.cljs +++ b/test/cljs/reitit/frontend/core_test.cljs @@ -227,3 +227,20 @@ :token_type "bearer" :expires_in 3600}}}) (m (rf/match-by-path router "/5?mode=foo#access_token=foo&refresh_token=bar&provider_token=baz&token_type=bearer&expires_in=3600")))))))) + +(deftest update-path-query-params-test + (is (= "foo?bar=1" + (rf/update-path-query-params "foo" #(assoc % :bar 1)))) + + (is (= "foo?asd=1&bar=1" + (rf/update-path-query-params "foo?asd=1" #(assoc % :bar 1)))) + + (is (= "foo?bar=1" + (rf/update-path-query-params "foo?asd=1&bar=1" #(dissoc % :asd)))) + + (is (= "foo" + (rf/update-path-query-params "foo?asd=1" #(dissoc % :asd)))) + + (testing "Need to coerce current values manually" + (is (= "foo?foo=2" + (rf/update-path-query-params "foo?foo=1" update :foo #(inc (js/parseInt %))))))) From a558365252921129b8258ce6fda3cf90f650c67e Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Thu, 23 Mar 2023 15:41:48 +0200 Subject: [PATCH 3/7] Cleanup --- test/cljs/reitit/frontend/core_test.cljs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/cljs/reitit/frontend/core_test.cljs b/test/cljs/reitit/frontend/core_test.cljs index 220861a1..710c4e98 100644 --- a/test/cljs/reitit/frontend/core_test.cljs +++ b/test/cljs/reitit/frontend/core_test.cljs @@ -230,16 +230,16 @@ (deftest update-path-query-params-test (is (= "foo?bar=1" - (rf/update-path-query-params "foo" #(assoc % :bar 1)))) + (rf/update-path-query-params "foo" assoc :bar 1))) (is (= "foo?asd=1&bar=1" - (rf/update-path-query-params "foo?asd=1" #(assoc % :bar 1)))) + (rf/update-path-query-params "foo?asd=1" assoc :bar 1))) (is (= "foo?bar=1" - (rf/update-path-query-params "foo?asd=1&bar=1" #(dissoc % :asd)))) + (rf/update-path-query-params "foo?asd=1&bar=1" dissoc :asd))) (is (= "foo" - (rf/update-path-query-params "foo?asd=1" #(dissoc % :asd)))) + (rf/update-path-query-params "foo?asd=1" dissoc :asd))) (testing "Need to coerce current values manually" (is (= "foo?foo=2" From f78116e346f9ce952c027fb6e65eb928e6900f27 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Thu, 23 Mar 2023 15:46:25 +0200 Subject: [PATCH 4/7] Test fragment --- test/cljs/reitit/frontend/core_test.cljs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/cljs/reitit/frontend/core_test.cljs b/test/cljs/reitit/frontend/core_test.cljs index 710c4e98..541d2b84 100644 --- a/test/cljs/reitit/frontend/core_test.cljs +++ b/test/cljs/reitit/frontend/core_test.cljs @@ -232,6 +232,10 @@ (is (= "foo?bar=1" (rf/update-path-query-params "foo" assoc :bar 1))) + (testing "Keep fragment" + (is (= "foo?bar=1&zzz=2#aaa" + (rf/update-path-query-params "foo?bar=1#aaa" assoc :zzz 2)))) + (is (= "foo?asd=1&bar=1" (rf/update-path-query-params "foo?asd=1" assoc :bar 1))) From 48bbdba8edc7f9c9ef368295dd9fbe08a76f2bb2 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Fri, 24 Mar 2023 11:16:09 +0200 Subject: [PATCH 5/7] Implement navigate and set-query functions --- .../reitit-frontend/src/reitit/frontend.cljs | 26 ++++---- .../src/reitit/frontend/easy.cljs | 54 ++++++++++++---- .../src/reitit/frontend/history.cljs | 61 +++++++++++++++++-- test/cljs/reitit/frontend/core_test.cljs | 34 ++++++++--- test/cljs/reitit/frontend/easy_test.cljs | 38 +++++++++--- 5 files changed, 167 insertions(+), 46 deletions(-) diff --git a/modules/reitit-frontend/src/reitit/frontend.cljs b/modules/reitit-frontend/src/reitit/frontend.cljs index 70dd5482..89b41fc0 100644 --- a/modules/reitit-frontend/src/reitit/frontend.cljs +++ b/modules/reitit-frontend/src/reitit/frontend.cljs @@ -12,7 +12,7 @@ (vec vs)))) (defn query-params - "Given goog.Uri, read query parameters into Clojure map." + "Given goog.Uri, read query parameters into a Clojure map." [^Uri uri] (let [q (.getQueryData uri)] (->> q @@ -20,6 +20,19 @@ (map (juxt keyword #(query-param q %))) (into {})))) +(defn set-query-params + "Given Reitit-frontend path, update the query params + with given function and arguments. + + Note: coercion is not applied to the query params" + [path new-query-or-update-fn] + (let [^goog.Uri uri (Uri/parse path) + new-query (if (fn? new-query-or-update-fn) + (new-query-or-update-fn (query-params uri)) + new-query-or-update-fn)] + (.setQueryData uri (QueryData/createFromMap (clj->js new-query))) + (.toString uri))) + (defn match-by-path "Given routing tree and current path, return match with possibly coerced parameters. Return nil if no match found. @@ -88,14 +101,3 @@ match) (do (js/console.warn "missing route" name) nil)))) - -(defn update-path-query-params - "Given Reitit-frontend path, update the query params - with given function and arguments. - - NOTE: coercion is not applied to the query params" - [path f & args] - (let [^goog.Uri uri (Uri/parse path) - new-query (apply f (query-params uri) args)] - (.setQueryData uri (QueryData/createFromMap (clj->js new-query))) - (.toString uri))) diff --git a/modules/reitit-frontend/src/reitit/frontend/easy.cljs b/modules/reitit-frontend/src/reitit/frontend/easy.cljs index 4c2518cf..bcd47f88 100644 --- a/modules/reitit-frontend/src/reitit/frontend/easy.cljs +++ b/modules/reitit-frontend/src/reitit/frontend/easy.cljs @@ -2,8 +2,7 @@ "Easy wrapper over reitit.frontend.history, handling the state. Only one router can be active at a time." - (:require [reitit.frontend.history :as rfh] - [reitit.frontend :as rf])) + (:require [reitit.frontend.history :as rfh])) (defonce history (atom nil)) @@ -103,14 +102,43 @@ ([name path-params query-params] (rfh/replace-state @history name path-params query-params))) -(defn update-query - ;; TODO: Sync the docstring with other namespaces - "Takes the current location and updates the query params - with given fn and arguments." - [f & args] - ;; TODO: rfh version? - (let [current-path (rfh/-get-path @history) - new-path (apply rf/update-path-query-params current-path f args)] - ;; TODO: replaceState version - (.pushState js/window.history nil "" new-path) - (rfh/-on-navigate @history new-path))) +;; This duplicates previous two, but the map parameter will be easier way to +;; extend the functions, e.g. to work with fragment string. Toggling push vs +;; replace can be also simpler with a flag. +;; Navigate and set-query are also similer to react-router API. +(defn + ^{:see-also ["reitit.frontend.history/navigate"]} + navigate + "Updates the browser location and either pushes new entry to the history stack + or replaces the latest entry in the the history stack (controlled by + `replace` option) using URL built from a route defined by name given + parameters. + + 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. + + See also: + https://developer.mozilla.org/en-US/docs/Web/API/History/pushState + https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState" + ([name] + (rfh/navigate @history name)) + ([name {:keys [path-params query-params replace] :as opts}] + (rfh/navigate @history name opts))) + +(defn + ^{:see-also ["reitit.frontend.history/set-query"]} + set-query + "Update query parameters for the current route. + + 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." + ([new-query-or-update-fn] + (rfh/set-query @history new-query-or-update-fn)) + ([new-query-or-update-fn {:keys [replace] :as opts}] + (rfh/set-query @history new-query-or-update-fn opts))) diff --git a/modules/reitit-frontend/src/reitit/frontend/history.cljs b/modules/reitit-frontend/src/reitit/frontend/history.cljs index f95dcf12..3cbec5ab 100644 --- a/modules/reitit-frontend/src/reitit/frontend/history.cljs +++ b/modules/reitit-frontend/src/reitit/frontend/history.cljs @@ -9,9 +9,9 @@ (defprotocol History (-init [this] "Create event listeners") (-stop [this] "Remove event listeners") - (-on-navigate [this path]) - (-get-path [this]) - (-href [this path])) + (-on-navigate [this path] "Find a match for current routing path and call on-navigate callback") + (-get-path [this] "Get the current routing path") + (-href [this path] "Converts given routing path to browser location")) ;; This version listens for both pop-state and hash-change for ;; compatibility for old browsers not supporting History API. @@ -177,7 +177,9 @@ (if history (-stop history))) -(defn href +(defn + ^{:see-also ["reitit.core/match->path"]} + href "Generate a URL for a route defined by name, with given path-params and query-params. The URL is formatted using Reitit frontend history handler, so using it with @@ -219,7 +221,9 @@ (.pushState js/window.history nil "" (-href history path)) (-on-navigate history path)))) -(defn replace-state +(defn + ^{:see-also ["reitit.core/match->path"]} + replace-state "Updates the browser location and replaces latest entry in the history stack using URL built from a route defined by name, with given path-params and query-params. @@ -241,3 +245,50 @@ path (reitit/match->path match query-params)] (.replaceState js/window.history nil "" (-href history path)) (-on-navigate history path)))) + +(defn + ^{:see-also ["reitit.core/match->path"]} + navigate + "Updates the browser location and either pushes new entry to the history stack + or replaces the latest entry in the the history stack (controlled by + `replace` option) using URL built from a route defined by name given + parameters. + + 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. + + See also: + https://developer.mozilla.org/en-US/docs/Web/API/History/pushState + https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState" + ([history name] + (navigate history name nil)) + ([history name {:keys [path-params query-params replace] :as opts}] + (let [match (rf/match-by-name! (:router history) name path-params) + path (reitit/match->path match query-params)] + (if replace + (.replaceState js/window.history nil "" (-href history path)) + (.pushState js/window.history nil "" (-href history path))) + (-on-navigate history path)))) + +(defn + ^{:see-also ["reitit.frontend/set-query-params"]} + set-query + "Update query parameters for the current route. + + 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." + ([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)] + (if replace + (.replaceState js/window.history nil "" (-href history new-path)) + (.pushState js/window.history nil "" (-href history new-path))) + (-on-navigate history new-path)))) diff --git a/test/cljs/reitit/frontend/core_test.cljs b/test/cljs/reitit/frontend/core_test.cljs index 541d2b84..ffa919c9 100644 --- a/test/cljs/reitit/frontend/core_test.cljs +++ b/test/cljs/reitit/frontend/core_test.cljs @@ -8,6 +8,19 @@ [reitit.coercion.malli :as rcm] [reitit.frontend.test-utils :refer [capture-console]])) +(deftest query-params-test + (is (= {:foo "1"} + (rf/query-params (.parse goog.Uri "?foo=1")))) + + (is (= {:foo "1" :bar "aaa"} + (rf/query-params (.parse goog.Uri "?foo=1&bar=aaa")))) + + (is (= {:foo ""} + (rf/query-params (.parse goog.Uri "?foo=")))) + + (is (= {:foo ""} + (rf/query-params (.parse goog.Uri "?foo"))))) + (defn m [x] (assoc x :data nil :result nil)) @@ -228,23 +241,30 @@ :expires_in 3600}}}) (m (rf/match-by-path router "/5?mode=foo#access_token=foo&refresh_token=bar&provider_token=baz&token_type=bearer&expires_in=3600")))))))) -(deftest update-path-query-params-test +(deftest set-query-params-test (is (= "foo?bar=1" - (rf/update-path-query-params "foo" assoc :bar 1))) + (rf/set-query-params "foo" {:bar 1}) + (rf/set-query-params "foo" #(assoc % :bar 1)))) (testing "Keep fragment" (is (= "foo?bar=1&zzz=2#aaa" - (rf/update-path-query-params "foo?bar=1#aaa" assoc :zzz 2)))) + (rf/set-query-params "foo?bar=1#aaa" #(assoc % :zzz 2))))) (is (= "foo?asd=1&bar=1" - (rf/update-path-query-params "foo?asd=1" assoc :bar 1))) + (rf/set-query-params "foo?asd=1" #(assoc % :bar 1)))) (is (= "foo?bar=1" - (rf/update-path-query-params "foo?asd=1&bar=1" dissoc :asd))) + (rf/set-query-params "foo?asd=1&bar=1" #(dissoc % :asd)))) + + (is (= "foo?bar" + (rf/set-query-params "foo?asd=1&bar" #(dissoc % :asd)))) + + (is (= "foo?bar" + (rf/set-query-params "foo" #(assoc % :bar "")))) (is (= "foo" - (rf/update-path-query-params "foo?asd=1" dissoc :asd))) + (rf/set-query-params "foo?asd=1" #(dissoc % :asd)))) (testing "Need to coerce current values manually" (is (= "foo?foo=2" - (rf/update-path-query-params "foo?foo=1" update :foo #(inc (js/parseInt %))))))) + (rf/set-query-params "foo?foo=1" (fn [q] (update q :foo #(inc (js/parseInt %))))))))) diff --git a/test/cljs/reitit/frontend/easy_test.cljs b/test/cljs/reitit/frontend/easy_test.cljs index 7def2701..abe11545 100644 --- a/test/cljs/reitit/frontend/easy_test.cljs +++ b/test/cljs/reitit/frontend/easy_test.cljs @@ -30,33 +30,53 @@ (is (= "/" url) "start at root") (rfe/push-state ::foo)) + ;; 0. / + ;; 1. /foo 2 (do (is (= "/foo" url) "push-state") (.back js/window.history)) + ;; 0. / 3 (do (is (= "/" url) "go back") - (rfe/push-state ::bar {:id 1})) + (rfe/navigate ::bar {:path-params {:id 1}})) + ;; 0. / + ;; 1. /bar/1 4 (do (is (= "/bar/1" url) "push-state 2") (rfe/replace-state ::bar {:id 2})) + ;; 0. / + ;; 1. /bar/2 5 (do (is (= "/bar/2" url) "replace-state") - (.back js/window.history)) - 6 (do (is (= "/" url) - "go back after replace state") + (rfe/set-query {:a 1})) + ;; 0. / + ;; 1. /bar/2 + ;; 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})) + ;; 0. / + ;; 1. /bar/2 + ;; 2. /bar/2?a=1&b=foo + 7 (do (is (= "/bar/2?a=1&b=foo" 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 (rfe/start! router (fn on-navigate [match history] (let [url (rfh/-get-path history)] (case (swap! n inc) - 7 (do (is (= "/" url) + 9 (do (is (= "/" url) "start at root") (rfe/push-state ::foo)) - 8 (do (is (= "/foo" url) - "push-state") - (rfh/stop! @rfe/history) - (done)) + 10 (do (is (= "/foo" url) + "push-state") + (rfh/stop! @rfe/history) + (done)) (do (is false (str "extra event 2" {:n @n, :url url})) (done))))) From dad8f530a672aa86a695b376f2295b8095d686d9 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Fri, 24 Mar 2023 11:32:22 +0200 Subject: [PATCH 6/7] Add example and update docs --- doc/frontend/browser.md | 12 +++++++++-- .../src/frontend/core.cljs | 21 ++++++++++++------- .../reitit-frontend/src/reitit/frontend.cljs | 16 +++++++------- .../src/reitit/frontend/history.cljs | 8 +++---- 4 files changed, 36 insertions(+), 21 deletions(-) diff --git a/doc/frontend/browser.md b/doc/frontend/browser.md index 50349fcc..1541265b 100644 --- a/doc/frontend/browser.md +++ b/doc/frontend/browser.md @@ -2,8 +2,16 @@ Reitit includes two browser history integrations. -Functions follow HTML5 History API: `push-state` to change route, `replace-state` -to change route without leaving previous entry in browser history. +Main functions are `navigate` and `set-query`. Navigate is used to navigate +to named routes, and the options parameter can be used to control all +parameters and if `pushState` or `replaceState` should be used to control +browser history stack. The `set-query` function can be used to change +or modify query parameters for the current route, it takes either map of +new query params or function from old params to the new params. + +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. ## Fragment router diff --git a/examples/frontend-controllers/src/frontend/core.cljs b/examples/frontend-controllers/src/frontend/core.cljs index 60cd6f17..6a2388f2 100644 --- a/examples/frontend-controllers/src/frontend/core.cljs +++ b/examples/frontend-controllers/src/frontend/core.cljs @@ -19,10 +19,17 @@ [:ul [:li [:a {:href (rfe/href ::item {:id 1})} "Item 1"]] [:li [:a {:href (rfe/href ::item {:id 2} {:foo "bar"})} "Item 2"]]] - (if id + (when id [:h2 "Selected item " id]) - (if (:foo query) - [:p "Optional foo query param: " (:foo query)])])) + [:p "Query params: " [:pre (pr-str query)]] + [:ul + [:li [:a {:on-click #(rfe/set-query {:a 1})} "set a=1"]] + [:li [:a {:on-click #(rfe/set-query {:a 2} {:replace true})} "set a=2 and replaceState"]] + [:li [:a {:on-click (fn [_] (rfe/set-query #(assoc % :foo "zzz")))} "add foo=zzz"]]] + [:button + {:on-click #(rfe/navigate ::item {:path-params {:id 3} + :query-params {:foo "aaa"}})} + "Navigate example, go to item 3"]])) (defonce match (r/atom nil)) @@ -31,9 +38,8 @@ [:ul [:li [:a {:href (rfe/href ::frontpage)} "Frontpage"]] [:li - [:a {:href (rfe/href ::item-list)} "Item list"] - ]] - (if @match + [:a {:href (rfe/href ::item-list)} "Item list"]]] + (when @match (let [view (:view (:data @match))] [view @match])) [:pre (with-out-str (fedn/pprint @match))]]) @@ -63,7 +69,8 @@ ["/:id" {:name ::item :parameters {:path {:id s/Int} - :query {(s/optional-key :foo) s/Keyword}} + :query {(s/optional-key :a) s/Int + (s/optional-key :foo) s/Keyword}} :controllers [{:parameters {:path [:id]} :start (fn [{:keys [path]}] (js/console.log "start" "item controller" (:id path))) diff --git a/modules/reitit-frontend/src/reitit/frontend.cljs b/modules/reitit-frontend/src/reitit/frontend.cljs index 89b41fc0..38608e13 100644 --- a/modules/reitit-frontend/src/reitit/frontend.cljs +++ b/modules/reitit-frontend/src/reitit/frontend.cljs @@ -1,11 +1,11 @@ (ns reitit.frontend (:require [clojure.set :as set] [reitit.coercion :as coercion] - [reitit.core :as r]) - (:import goog.Uri - goog.Uri.QueryData)) + [reitit.core :as r] + goog.Uri + goog.Uri.QueryData)) -(defn- query-param [^QueryData q k] +(defn- query-param [^goog.uri.QueryData q k] (let [vs (.getValues q k)] (if (< (alength vs) 2) (aget vs 0) @@ -13,7 +13,7 @@ (defn query-params "Given goog.Uri, read query parameters into a Clojure map." - [^Uri uri] + [^goog.Uri uri] (let [q (.getQueryData uri)] (->> q (.getKeys) @@ -26,11 +26,11 @@ Note: coercion is not applied to the query params" [path new-query-or-update-fn] - (let [^goog.Uri uri (Uri/parse path) + (let [^goog.Uri uri (goog.Uri/parse path) new-query (if (fn? new-query-or-update-fn) (new-query-or-update-fn (query-params uri)) new-query-or-update-fn)] - (.setQueryData uri (QueryData/createFromMap (clj->js new-query))) + (.setQueryData uri (goog.Uri.QueryData/createFromMap (clj->js new-query))) (.toString uri))) (defn match-by-path @@ -40,7 +40,7 @@ :on-coercion-error - a sideeffecting fn of `match exception -> nil`" ([router path] (match-by-path router path nil)) ([router path {:keys [on-coercion-error]}] - (let [uri (.parse Uri path) + (let [uri (.parse goog.Uri path) coerce! (if on-coercion-error (fn [match] (try (coercion/coerce! match) diff --git a/modules/reitit-frontend/src/reitit/frontend/history.cljs b/modules/reitit-frontend/src/reitit/frontend/history.cljs index 3cbec5ab..5764e10f 100644 --- a/modules/reitit-frontend/src/reitit/frontend/history.cljs +++ b/modules/reitit-frontend/src/reitit/frontend/history.cljs @@ -3,8 +3,8 @@ events." (:require [goog.events :as gevents] [reitit.core :as reitit] - [reitit.frontend :as rf]) - (:import goog.Uri)) + [reitit.frontend :as rf] + goog.Uri)) (defprotocol History (-init [this] "Create event listeners") @@ -78,7 +78,7 @@ the page location is updated using History API." [router e el uri] (let [current-domain (if (exists? js/location) - (.getDomain (.parse Uri js/location)))] + (.getDomain (.parse goog.Uri js/location)))] (and (or (and (not (.hasScheme uri)) (not (.hasDomain uri))) (= current-domain (.getDomain uri))) (not (.-altKey e)) @@ -109,7 +109,7 @@ ignore-anchor-click (fn [e] ;; Returns the next matching ancestor of event target (when-let [el (closest-by-tag (event-target e) "a")] - (let [uri (.parse Uri (.-href el))] + (let [uri (.parse goog.Uri (.-href el))] (when (ignore-anchor-click-predicate router e el uri) (.preventDefault e) (let [path (str (.getPath uri) From e2217887e3d7c719f3551b76ba7a1aab53880c63 Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Fri, 24 Mar 2023 13:59:02 +0200 Subject: [PATCH 7/7] Comments about differences to reitit.impl --- .../reitit-frontend/src/reitit/frontend.cljs | 3 +++ test/cljs/reitit/frontend/core_test.cljs | 20 ++++++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/modules/reitit-frontend/src/reitit/frontend.cljs b/modules/reitit-frontend/src/reitit/frontend.cljs index 38608e13..907a07a7 100644 --- a/modules/reitit-frontend/src/reitit/frontend.cljs +++ b/modules/reitit-frontend/src/reitit/frontend.cljs @@ -30,6 +30,9 @@ new-query (if (fn? new-query-or-update-fn) (new-query-or-update-fn (query-params uri)) new-query-or-update-fn)] + ;; NOTE: Differences to reitit.impl/query-string? + ;; reitit fn adds "=" even if value is empty string + ;; reitit encodes " " as "+" while browser and goog.Uri encode as "%20" (.setQueryData uri (goog.Uri.QueryData/createFromMap (clj->js new-query))) (.toString uri))) diff --git a/test/cljs/reitit/frontend/core_test.cljs b/test/cljs/reitit/frontend/core_test.cljs index ffa919c9..a0e0abe0 100644 --- a/test/cljs/reitit/frontend/core_test.cljs +++ b/test/cljs/reitit/frontend/core_test.cljs @@ -6,7 +6,8 @@ [schema.core :as s] [reitit.coercion.schema :as rcs] [reitit.coercion.malli :as rcm] - [reitit.frontend.test-utils :refer [capture-console]])) + [reitit.frontend.test-utils :refer [capture-console]] + [reitit.impl :as impl])) (deftest query-params-test (is (= {:foo "1"} @@ -244,7 +245,17 @@ (deftest set-query-params-test (is (= "foo?bar=1" (rf/set-query-params "foo" {:bar 1}) - (rf/set-query-params "foo" #(assoc % :bar 1)))) + (rf/set-query-params "foo" #(assoc % :bar 1)) + ;; Also compare to reitit.impl version which is used by match->path (and history fns) + (str "foo?" (impl/query-string {:bar 1})))) + + (testing "Encoding" + (is (= "foo?bar=foo%20bar" + (rf/set-query-params "foo" {:bar "foo bar"}) + (rf/set-query-params "foo" #(assoc % :bar "foo bar")) + ;; FIXME: Reitit.impl encodes space as "+" + ; (str "foo?" (impl/query-string {:bar "foo bar"})) + ))) (testing "Keep fragment" (is (= "foo?bar=1&zzz=2#aaa" @@ -260,7 +271,10 @@ (rf/set-query-params "foo?asd=1&bar" #(dissoc % :asd)))) (is (= "foo?bar" - (rf/set-query-params "foo" #(assoc % :bar "")))) + (rf/set-query-params "foo" #(assoc % :bar "")) + ;; FIXME: Reitit.impl adds "=" for empty string values + ; (str "foo?" (impl/query-string {:bar ""})) + )) (is (= "foo" (rf/set-query-params "foo?asd=1" #(dissoc % :asd))))