-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fold some of this into Pedestal? #2
Comments
Hey @ohpauleez! Thanks for checking in, I'm happy to contribute to the discussion however I can. My gut feeling about verb maps is that they should be kept out of the route tables. Strictly speaking, they are not part of the routing and they tie
(def home ...)
(def pets ...)
(def routes
[["/" home
["/pets" pets]]]) Which makes it trivial to use in other context (e.g cljs single page apps, ring+liberator, message/topic dispatch etc.) Pedestal could still provide a convenient way to specify verb maps for specific domains like http: (def pets (interceptor.http/handler {:get get-all-pets
:post create-new-pet}))
(def home (interceptor.http/handler {:get get-home-page}))
(def routes
[["/" home
["/pets" pets]]])
|
Great feedback all around. More than likely, the route compiler will get more intelligent about verbs. It is my plan to support a set of verbs passed in as an argument during the route tree compilation. If no verbs are passed in, you'll get a one-to-one mapping. Otherwise verbs will be compiled into the tree. As it stands right now, the prefix-tree (and the map-tree) only route on path, and then perform a "constraint routing" which includes verb routing. That is, one-to-one routing is the norm (already), and the idea is to expose more control over the secondary constraint routing. Feel free to close this issue if you're satisfied :) |
If the one-to-one use case is supported then I've only one small concern about it. I loved how you evolved Pedestal in the last few iterations, progressively hiding away excessive customisation in favour of a recommended way of doing things (one preferred way of creating interceptors, one preferred route syntax, etc.) The change you propose now would create different valid outputs after route expansion, which is annoying for external tools that consume it like One positive feedback I've received for Nothing an external library cannot cope with, btw, so it's not dramatic. But my point being: pedestal will keep on feeling huge just because it has so many dsls nobody has really time to learn about. |
Hi @frankiesardo!
I was just strolling through some routing ideas and I saw a note I left myself to come back here and take a look. Now that Pedestal's routing is a separate module, you might find some pieces you could reuse from there. But more importantly...
Soon Pedestal's core route pieces won't be tied to HTTP verbs, and I'd like your input on other ideas and use-cases you think I should aware of in this area.
Feel free to close this issue and push the conversation to another medium!
The text was updated successfully, but these errors were encountered: