Skip to content
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

Make Route Dispatching a Little Less Imperative #16

Open
gf3 opened this issue Feb 27, 2014 · 11 comments
Open

Make Route Dispatching a Little Less Imperative #16

gf3 opened this issue Feb 27, 2014 · 11 comments
Milestone

Comments

@gf3
Copy link
Collaborator

gf3 commented Feb 27, 2014

Allow routes to be dispatched from (and optionally defined to?) different collections. This'll make things like testing and optional routes easier to work with.

E.g.:

(secretary/dispatch! "/users/gf3")
;; To ↓ 
(secretary/dispatch! some-defed-routes "/users/gf3")

Thanks for the suggestion, @sgrove.

@sgrove
Copy link
Contributor

sgrove commented Feb 27, 2014

Specifically, it would be great to do something like:

    (let [user-handler (fn [user-id]
                         (print "User looked up: " user-id))
          test-routes (-> {}
                          (secretary/assoc-route :v1-channel-link "/v1/channels/:channel-id" channel-handler)
                          (secretary/assoc-route :v1-user-link "/v1/users/:user-id" user-handler))]
      (secretary/dispatch! test-routes "/v1/users/10"))

And have everything self-contained/referentially transparent

@noprompt
Copy link
Collaborator

Definitely. This is along the lines of what we were discussing in #9. At work we've encountered some situations where this functionality would be nice.

Ideally, it'd be nice to have something along the lines of

(defroutes death-routes
  (route poison-path "/poison/:type" ...)
  (route stab-path "/stab/:with" ..))

which would essentially expand to the code @sgrove shared above in test-routes part. The spec for defroutes might be [name] [name opts] where name is the var name and opts would be for configuring render-route etc. route would also need to be macro and essentially just be reusing the some of the current defroute code.

@gf3
Copy link
Collaborator Author

gf3 commented Feb 28, 2014

Mmm—I like the sounds of that 👍

@noprompt
Copy link
Collaborator

Should we bump to 2.0.0 after this? Seems like it's major enough.

@gf3
Copy link
Collaborator Author

gf3 commented Feb 28, 2014

Yeah, sounds good.

@noprompt
Copy link
Collaborator

Question: Should we also implement context? See here.

@gf3
Copy link
Collaborator Author

gf3 commented Feb 28, 2014

Let's hold off for now. And do it later if we need it.

@edbond
Copy link

edbond commented Feb 28, 2014

context is a good thing. Definetely should be implemented imo. 💊

@gf3
Copy link
Collaborator Author

gf3 commented Feb 28, 2014

I agree, but perhaps in a different PR.

@noprompt
Copy link
Collaborator

Alright, good idea.

@gf3 gf3 added this to the v2.0.0 milestone Feb 28, 2014
@noprompt
Copy link
Collaborator

noprompt commented Apr 2, 2014

I've started work on this in a separate branch here. The current syntax looks like

(defroutes routes-name
  (ROUTE users-path "/users" [])
  (ROUTE user-path "/users/:id" [id])
  (ROUTE user-edit-path "/users/:id/edit" [id]))

This defines an instance of a new type secretary.core/Routes which delegates to several of the old functions for lookup, etc. The symbols defined by defroutes can also be included in other defroutes. context has not been implemented but I expect to get to it soon.

If anyone has questions or ideas let me know. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants