-
Notifications
You must be signed in to change notification settings - Fork 64
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
pre- / post- actions for post-dispatch #21
Comments
Oooh—I quite like this idea. Do you think it'd also be useful to support pre/post actions for an entire route collection? |
I like this idea too but the more I think about it the more I wonder if it should be packaged with Secretary. Since (let [o (secretary/dispatch! "/some/route")]
(cond
(satisifies? IRoutePreAction x)
(route-pre-action x))
...
:else o) I still think this is a great idea but let's have a handful of solid use cases before deciding whether or not to add it. Edit: Fix weird grammar. |
It's important to note that what I'm proposing explicitly happens post routing (although I do ask in my original comment whether or not this could/should be extended to pre-routing operations too). The use cases for this depend on the de-structured route parameters being available to the pre-/during/post- route actions as I described above. Here's another use-case as an example--you could use this to filter post routing, based on routing args: (deftype FilterBadString [filter-str
IPreRoutingAction
(pre-routing-action [this str]
(when (re-find (re-pattern filter-str) str)
(secretary/dispatch! "/safe-page"))))
(defroute "/search/:str" [str]
(FilterBadString. "bad!")
;; ... do stuff ...
) In general, I could certain do this as I do now with this kind of code: (defn pre [args] ;; do stuff )
(defn post [args] ;; do stuff )
(defn wrap-pre-and-post [args f]
(pre args)
(f)
(post args))
(defroute "/myroute/:id" [id]
(wrap-pre-and-post
(fn [id]
;; do stuff))) ...and this is what I have to do now. It's okay, it works, but it requires customization of arguments and is not particularly smart. So the next step I was going to make was to basically re-implement defroute myself to essentially do what I'm describing in this issue. I certainly have no problem doing it separately but it seemed like a common enough pattern that others could make use of it. Whether or not it should be a part of secretary or not, I'm not sure...up to you guys. |
@gf3 Sorry, I didn't answer your question:
Seems like the answer is yes if there was a route collection helper that this could fit into. This depends on defroute taking advantage of the protocols, so whatever would allow you to define a route collection (defroutes?) would have to be aware of the protocols as well. But seems like you may want this for a group of route too--for example, if you have a section of your site with the same headers or something, this could help you easily set that up. |
I had to implement this for our own project--here's the gist. |
I'm withdrawing this issue--I'm glad we waited on it. Having used our own version of this in production for months, I realize that it doesn't really solve any problems that can't be solved in a simpler fashion otherwise. |
...continues discussion from IRC.
I propose to add a mechanism for performing pre-/during-/post- actions, after dispatch has happened (that is, wrapping the form in
defroute
).Here is a set of proposed protocols and some usage patterns:
By default, you would continue to call
defroute
as now:But, you could then also do something like this (mimicking Om usage patterns):
A more concrete use-case (and why I was proposing it in the first place):
This would eliminate a lot of boilerplate and satisfy the needs of many developers building complex one-page apps in CLJS (as we are).
Also proposed by noprompt: IWillNavigate/INavigate/IDidNavigate protocols. Should pre-dispatch protocols be allowed in arguments to defroute? Etc.
Okay, this is just the first proposal--let me know what you think.
The text was updated successfully, but these errors were encountered: