Merge pull request #604 from metosin/fix-377-fragment-strings

Fix #377, navigate to routes with fragment string in frontend

examples/frontend-controllers/src/frontend/core.cljs

@@ -18,7 +18,7 @@
     [:div
      [:ul
       [:li [:a {:href (rfe/href ::item {:id 1})} "Item 1"]]
-      [:li [:a {:href (rfe/href ::item {:id 2} {:foo "bar"})} "Item 2"]]]
+      [:li [:a {:href (rfe/href ::item {:id 2} {:foo "bar"} "zzz")} "Item 2"]]]
      (when id
        [:h2 "Selected item " id])
      [:p "Query params: " [:pre (pr-str query)]]

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

@@ -2,6 +2,7 @@
   (:require [clojure.set :as set]
             [reitit.coercion :as coercion]
             [reitit.core :as r]
+            [reitit.impl :as impl]
             goog.Uri
             goog.Uri.QueryData))
 
@@ -36,6 +37,16 @@
     (.setQueryData uri (goog.Uri.QueryData/createFromMap (clj->js new-query)))
     (.toString uri)))
 
+(defn
+  ^{:see-also ["reitit.core/match->path"]}
+  match->path
+  "Create routing path from given match and optional query-string map and
+  optional fragment string."
+  [match query-params fragment]
+  (when-let [path (r/match->path match query-params)]
+    (cond-> path
+      (and fragment (seq fragment)) (str "#" (impl/form-encode fragment)))))
+
 (defn match-by-path
   "Given routing tree and current path, return match with possibly
   coerced parameters. Return nil if no match found.

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

@@ -52,11 +52,13 @@
   pairs separated by &, i.e. \"?a=1&a=2\", if you want to encode them
   differently, convert the collections to strings first."
   ([name]
-   (rfh/href @history name nil nil))
+   (rfh/href @history name nil nil nil))
   ([name path-params]
-   (rfh/href @history name path-params nil))
+   (rfh/href @history name path-params nil nil))
   ([name path-params query-params]
-   (rfh/href @history name path-params query-params)))
+   (rfh/href @history name path-params query-params nil))
+  ([name path-params query-params fragment]
+   (rfh/href @history name path-params query-params fragment)))
 
 (defn
   ^{:see-also ["reitit.frontend.history/push-state"]}
@@ -74,11 +76,13 @@
   See also:
   https://developer.mozilla.org/en-US/docs/Web/API/History/pushState"
   ([name]
-   (rfh/push-state @history name nil nil))
+   (rfh/push-state @history name nil nil nil))
   ([name path-params]
-   (rfh/push-state @history name path-params nil))
+   (rfh/push-state @history name path-params nil nil))
   ([name path-params query-params]
-   (rfh/push-state @history name path-params query-params)))
+   (rfh/push-state @history name path-params query-params nil))
+  ([name path-params query-params fragment]
+   (rfh/push-state @history name path-params query-params fragment)))
 
 (defn
   ^{:see-also ["reitit.frontend.history/replace-state"]}
@@ -96,11 +100,13 @@
   See also:
   https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState"
   ([name]
-   (rfh/replace-state @history name nil nil))
+   (rfh/replace-state @history name nil nil nil))
   ([name path-params]
-   (rfh/replace-state @history name path-params nil))
+   (rfh/replace-state @history name path-params nil nil))
   ([name path-params query-params]
-   (rfh/replace-state @history name path-params query-params)))
+   (rfh/replace-state @history name path-params query-params nil))
+  ([name path-params query-params fragment]
+   (rfh/replace-state @history name path-params query-params fragment)))
 
 ;; 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
@@ -125,7 +131,7 @@
   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}]
+  ([name {:keys [path-params query-params replace fragment] :as opts}]
    (rfh/navigate @history name opts)))
 
 (defn

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

@@ -10,7 +10,7 @@
   (-init [this] "Create event listeners")
   (-stop [this] "Remove event listeners")
   (-on-navigate [this path] "Find a match for current routing path and call on-navigate callback")
-  (-get-path [this] "Get the current routing path")
+  (-get-path [this] "Get the current routing path, including query string and fragment")
   (-href [this path] "Converts given routing path to browser location"))
 
 ;; This version listens for both pop-state and hash-change for
@@ -92,6 +92,7 @@
          ;; isContentEditable property is inherited from parents,
          ;; so if the anchor is inside contenteditable div, the property will be true.
          (not (.-isContentEditable el))
+         ;; NOTE: Why doesn't this use frontend variant instead of core?
          (reitit/match-by-path router (.getPath uri)))))
 
 (defrecord Html5History [on-navigate router listen-key click-listen-key]
@@ -194,8 +195,10 @@
   ([history name path-params]
    (href history name path-params nil))
   ([history name path-params query-params]
+   (href history name path-params query-params nil))
+  ([history name path-params query-params fragment]
    (let [match (rf/match-by-name! (:router history) name path-params)]
-     (-href history (reitit/match->path match query-params)))))
+     (-href history (rf/match->path match query-params fragment)))))
 
 (defn
   ^{:see-also ["reitit.core/match->path"]}
@@ -212,12 +215,14 @@
   See also:
   https://developer.mozilla.org/en-US/docs/Web/API/History/pushState"
   ([history name]
-   (push-state history name nil nil))
+   (push-state history name nil nil nil))
   ([history name path-params]
-   (push-state history name path-params nil))
+   (push-state history name path-params nil nil))
   ([history name path-params query-params]
+   (push-state history name path-params query-params nil))
+  ([history name path-params query-params fragment]
    (let [match (rf/match-by-name! (:router history) name path-params)
-         path (reitit/match->path match query-params)]
+         path (rf/match->path match query-params fragment)]
      ;; pushState and replaceState don't trigger popstate event so call on-navigate manually
      (.pushState js/window.history nil "" (-href history path))
      (-on-navigate history path))))
@@ -238,12 +243,14 @@
   See also:
   https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState"
   ([history name]
-   (replace-state history name nil nil))
+   (replace-state history name nil nil nil))
   ([history name path-params]
-   (replace-state history name path-params nil))
+   (replace-state history name path-params nil nil))
   ([history name path-params query-params]
+   (replace-state history name path-params query-params nil))
+  ([history name path-params query-params fragment]
    (let [match (rf/match-by-name! (:router history) name path-params)
-         path (reitit/match->path match query-params)]
+         path (rf/match->path match query-params fragment)]
      (.replaceState js/window.history nil "" (-href history path))
      (-on-navigate history path))))
 
@@ -266,9 +273,9 @@
   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}]
+  ([history name {:keys [path-params query-params fragment replace] :as opts}]
    (let [match (rf/match-by-name! (:router history) name path-params)
-         path (reitit/match->path match query-params)]
+         path (rf/match->path match query-params fragment)]
      (if replace
        (.replaceState js/window.history nil "" (-href history path))
        (.pushState js/window.history nil "" (-href history path)))

test/cljs/reitit/frontend/core_test.cljs

@@ -282,3 +282,16 @@
   (testing "Need to coerce current values manually"
     (is (= "foo?foo=2"
            (rf/set-query-params "foo?foo=1" (fn [q] (update q :foo #(inc (js/parseInt %)))))))))
+
+(deftest match->path-test
+  (is (= "foo"
+         (rf/match->path {:path "foo"} nil nil)
+         (rf/match->path {:path "foo"} {} "")))
+  (is (= "foo?a=1&b=&c=foo+bar"
+         ;; NOTE: This encoding differs from set-query
+         (rf/match->path {:path "foo"} {:a "1" :b "" :c "foo bar"} nil)))
+  (is (= "foo#aaa"
+         (rf/match->path {:path "foo"} nil "aaa")))
+  (testing "Fragment encoding"
+    (is (= "foo#foo+bar+%25"
+           (rf/match->path {:path "foo"} nil "foo bar %")))))

test/cljs/reitit/frontend/easy_test.cljs

@@ -29,10 +29,10 @@
                           1 (do (is (some? (:popstate-listener history)))
                                 (is (= "/" url)
                                     "start at root")
-                                (rfe/push-state ::foo))
+                                (rfe/push-state ::foo nil {:a 1} "foo bar"))
                           ;; 0. /
-                          ;; 1. /foo
+                          ;; 1. /foo?a=1#foo+bar
-                          2 (do (is (= "/foo" url)
+                          2 (do (is (= "/foo?a=1#foo+bar" url)
                                     "push-state")
                                 (.back js/window.history))
                           ;; 0. /

test/cljs/reitit/frontend/history_test.cljs

@@ -26,6 +26,8 @@
                (rfh/href history ::bar {:id 5})))
         (is (= "#/bar/5?q=x"
                (rfh/href history ::bar {:id 5} {:q "x"})))
+        (is (= "#/bar/5?q=x#foo"
+               (rfh/href history ::bar {:id 5} {:q "x"} "foo")))
         (let [{:keys [value messages]} (capture-console
                                         (fn []
                                           (rfh/href history ::asd)))]