Skip to content

Commit 2fb35d0

Browse files
charlot-shawrschmukler
authored andcommitted
fix: handle reitit route denormalization
Co-authored-By: Ryan Schmukler <rschmukler@gmail.com> Fixes an issue where we didn't handle denormalized route syntax in reitit. Closes #17
1 parent 771b8d1 commit 2fb35d0

File tree

4 files changed

+111
-12
lines changed

4 files changed

+111
-12
lines changed

src/hyper/routes.clj

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,21 @@
33
44
Pure functions for looking up route metadata by name, resolving
55
titles/head content, and reading the current routes (supporting
6-
live-reloading via Var indirection).")
6+
live-reloading via Var indirection)."
7+
(:require [reitit.core :as reitit]))
78

89
(defn index-routes
9-
"Build a {route-name → route-data} map from a routes vector for O(1) lookups."
10+
"Build a {route-name → route-data} map from a routes vector for O(1) lookups.
11+
Handles both flat and arbitrarily-nested reitit-style route trees by compiling
12+
a temporary router and reading back the flattened routes."
1013
[routes]
11-
(into {}
12-
(keep (fn [[_path data]]
13-
(when-let [n (:name data)]
14-
[n data])))
15-
routes))
14+
(if (seq routes)
15+
(into {}
16+
(keep (fn [[_path data]]
17+
(when-let [n (:name data)]
18+
[n data])))
19+
(-> routes reitit/router reitit/routes))
20+
{}))
1621

1722
(defn find-render-fn
1823
"Find the :get handler for a named route."

src/hyper/server.clj

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -427,20 +427,21 @@
427427
compiles the reitit router, and updates app-state with the current routes and router.
428428
Returns the compiled Ring handler (without outer middleware)."
429429
[user-routes app-state* page-wrapper system-routes]
430-
(let [wrapped-routes (mapv (fn [[path route-data]]
430+
(let [flat-routes (-> user-routes reitit/router reitit/routes)
431+
wrapped-routes (mapv (fn [[path route-data]]
431432
(if (and (:get route-data) (not (:hyper/disabled? route-data)))
432433
[path (update route-data :get (fn [handler] (page-wrapper handler)))]
433434
[path route-data]))
434-
user-routes)
435+
flat-routes)
435436
all-routes (concat system-routes wrapped-routes)
436437
router (ring/router all-routes
437438
{:conflicts nil
438439
:data {:coercion malli/coercion
439440
:middleware [ring-coercion/coerce-request-middleware]}})]
440-
;; Store routes and router in app-state for access during actions/renders/navigation
441+
;; Store the flattened routes and router in app-state for access during actions/renders/navigation
441442
(swap! app-state* assoc
442443
:router router
443-
:routes user-routes)
444+
:routes flat-routes)
444445
(ring/ring-handler router (ring/create-default-handler))))
445446

446447
(defn- wrap-static

test/hyper/routes_test.clj

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,19 @@
1313
["/users/:id" {:name :user-profile
1414
:get (fn [_] [:div "User"])}]])
1515

16+
(def denormalized-sample-routes
17+
[[""
18+
["/"
19+
["" {:name :home
20+
:get (fn [_] [:div "Home"])
21+
:title "Home"}]]
22+
["/about"
23+
["" {:name :about
24+
:get (fn [_] [:div "About"])
25+
:title (fn [_req] "About Us")}]]
26+
["/users/:id" {:name :user-profile
27+
:get (fn [_] [:div "User"])}]]])
28+
1629
(deftest index-routes-test
1730
(testing "builds name->data map"
1831
(let [idx (routes/index-routes sample-routes)]
@@ -30,7 +43,20 @@
3043

3144
(testing "returns empty map for empty routes"
3245
(is (= {} (routes/index-routes [])))
33-
(is (= {} (routes/index-routes nil)))))
46+
(is (= {} (routes/index-routes nil))))
47+
48+
(testing "handles nested/denormalized reitit-style route trees"
49+
(let [idx (routes/index-routes denormalized-sample-routes)]
50+
(is (= 3 (count idx)))
51+
(is (contains? idx :home))
52+
(is (contains? idx :about))
53+
(is (contains? idx :user-profile))))
54+
55+
(testing "nested routes preserve route data"
56+
(let [idx (routes/index-routes denormalized-sample-routes)]
57+
(is (= "Home" (:title (idx :home))))
58+
(is (fn? (:title (idx :about))))
59+
(is (fn? (:get (idx :home)))))))
3460

3561
(deftest find-render-fn-test
3662
(let [idx (routes/index-routes sample-routes)]

test/hyper/server_test.clj

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,73 @@
232232
(is (= 302 (:status unauthed)))
233233
(is (= "/login" (get-in unauthed [:headers "Location"]))))))
234234

235+
(deftest test-create-handler-with-denormalized-routes
236+
(testing "Denormalized (nested) routes are served and receive hyper context"
237+
(let [received-req (atom nil)
238+
app-state* (atom (state/init-state))
239+
routes [[""
240+
["/"
241+
["" {:name :home
242+
:get (fn [req]
243+
(reset! received-req req)
244+
[:div "Home"])}]]
245+
["/about"
246+
["" {:name :about
247+
:get (fn [_] [:div "About"])
248+
:title "About Us"}]]
249+
["/users/:id" {:name :user-profile
250+
:get (fn [_] [:div "User"])}]]]
251+
handler (server/create-handler routes app-state*)
252+
response (handler {:uri "/" :request-method :get})]
253+
(is (= 200 (:status response))
254+
"Nested home route should be served")
255+
(is (.contains (:body response) "Home"))
256+
(is (some? @received-req)
257+
"Handler should have been called")
258+
(is (string? (:hyper/session-id @received-req))
259+
"Request should carry :hyper/session-id")
260+
(is (string? (:hyper/tab-id @received-req))
261+
"Request should carry :hyper/tab-id")
262+
(is (= app-state* (:hyper/app-state @received-req))
263+
"Request should carry :hyper/app-state")))
264+
265+
(testing "All sibling routes in a denormalized tree are reachable"
266+
(let [app-state* (atom (state/init-state))
267+
routes [[""
268+
["/"
269+
["" {:name :home
270+
:get (fn [_] [:div "Home"])}]]
271+
["/about"
272+
["" {:name :about
273+
:get (fn [_] [:div "About"])
274+
:title "About Us"}]]
275+
["/users/:id" {:name :user-profile
276+
:get (fn [_] [:div "User"])}]]]
277+
handler (server/create-handler routes app-state*)]
278+
(is (= 200 (:status (handler {:uri "/about" :request-method :get}))))
279+
(is (.contains (:body (handler {:uri "/about" :request-method :get})) "About"))
280+
(is (= 200 (:status (handler {:uri "/users/42" :request-method :get}))))
281+
(is (.contains (:body (handler {:uri "/users/42" :request-method :get})) "User"))))
282+
283+
(testing "Denormalized routes are indexed correctly in app-state"
284+
(let [app-state* (atom (state/init-state))
285+
routes [[""
286+
["/"
287+
["" {:name :home
288+
:get (fn [_] [:div "Home"])}]]
289+
["/about"
290+
["" {:name :about
291+
:get (fn [_] [:div "About"])
292+
:title "About Us"}]]
293+
["/users/:id" {:name :user-profile
294+
:get (fn [_] [:div "User"])}]]]
295+
_handler (server/create-handler routes app-state*)
296+
route-idx (routes/live-route-index app-state*)]
297+
(is (contains? route-idx :home))
298+
(is (contains? route-idx :about))
299+
(is (contains? route-idx :user-profile))
300+
(is (= "About Us" (routes/find-route-title route-idx :about))))))
301+
235302
(deftest test-create-handler-with-hyper-disabled
236303
(testing "render fn can disable endpoint wrapping"
237304
(let [app-state* (atom (state/init-state))

0 commit comments

Comments
 (0)