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

chi's future #43

Closed
nhooyr opened this issue May 28, 2016 · 12 comments
Closed

chi's future #43

nhooyr opened this issue May 28, 2016 · 12 comments

Comments

@nhooyr
Copy link

nhooyr commented May 28, 2016

Instead of an additional parameter of type context.Context with a new handler type, an alternative approach was favoured in the standard library.
See: golang/go@c1c7547

@pkieltyka
Copy link
Member

It's already done in v2 branch :) this was my entire all along and even messaging Go core team nudging them to add context to stdlib! It all worked out

On May 27, 2016, at 11:21 PM, Anmol Sethi notifications@github.com wrote:

Instead of an additional parameter of type context.Context with a new handler type, an alternative approach was favoured in the standard library.
See: golang/go@c1c7547


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@lwc
Copy link

lwc commented Jun 2, 2016

Love chi, it's fast becoming my default router choice for golang. The 1.7 compatibility branch is beginning to look like the perfect router for go.

If we're talking about a version 2.0 with breaking changes to the API, could I please ask you to consider introducing the notion of names for the routes? Named routes are handy for reverse lookups in templates etc, and also in monitoring tools such as Datadog and New Relic.

@pkieltyka
Copy link
Member

pkieltyka commented Jun 2, 2016

Thanks Luke :) Can you give me an example of some pseudo code for the named routes?
It's likely chi can already do it with either Handle, Route, or Mount, but maybe there's a case I'm missing.

On Jun 1, 2016, at 11:49 PM, Luke Cawood notifications@github.com wrote:

Love chi, it's fast becoming my default router choice for golang. The 1.7 compatibility branch is beginning to look like the perfect router for go.

If we're talking about a version 2.0 with breaking changes to the API, could I please ask you to consider introducing the notion of names for the routes? Named routes are handy for reverse lookups in templates etc, and also in monitoring tools such as Datadog and New Relic.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@lwc
Copy link

lwc commented Jun 2, 2016

Sure thing, this is what I had in mind:

    app := chi.NewRouter()

    app.Post("/users", "user_create", func(w http.ResponseWriter, r *http.Request) {
        user := pretendUserCreator(r)
        http.Redirect(w, r, app.URLFor("user_view", map[string]interface{}{"user_id": user.ID()}), http.StatusFound)
    })

    app.Get("/users/:user_id", "user_view", func(ctx context.Context, w http.ResponseWriter, r *http.Request) {
        userID := chi.URLParam(ctx, "user_id")
        pretendDisplayUser(w, userID)
    })

Gives a nice DRY way to generate urls to routes.

@pkieltyka
Copy link
Member

pkieltyka commented Jun 2, 2016

ah, railzzz, ..... shoot myself in the face :) my suggestion if you want this is to write your own helpers. Chi's philosophy is to stay minimal and build-your-own-adventure, besides a robust way to nest routes and manage context.

however, this is also why I've made chi.Router an interface.

You can most certainly enhance the interface, or implement this ability yourself.

package nmux // aka named-mux

type Mux struct {
  *chi.Mux
}

var _ chi.Router = &Mux{} // ensures the chi router interface is met by Mux struct

// some example code I pulled from an ACL router that is built by enhancing chi's mux.. you'll have to update it slightly to remove resourcecontrol and Route()
func (r *Mux) Use(middlewares ...interface{}) {
    r.Mux.Use(middlewares...)
}

func (r *Mux) Handle(pattern string, handlers ...interface{}) {
    h := append([]interface{}{Route(pattern), ResourceControl}, handlers...)
    r.Mux.Handle(pattern, h...)
}

func (r *Mux) Connect(pattern string, handlers ...interface{}) {
    h := append([]interface{}{Route(pattern), ResourceControl}, handlers...)
    r.Mux.Connect(pattern, h...)
}

func (r *Mux) Head(pattern string, handlers ...interface{}) {
    h := append([]interface{}{Route(pattern), ResourceControl}, handlers...)
    r.Mux.Head(pattern, h...)
}

func (r *Mux) Get(pattern string, handlers ...interface{}) {
    h := append([]interface{}{Route(pattern), ResourceControl}, handlers...)
    r.Mux.Get(pattern, h...)
}

func (r *Mux) Post(pattern string, handlers ...interface{}) {
    h := append([]interface{}{Route(pattern), ResourceControl}, handlers...)
    r.Mux.Post(pattern, h...)
}

func (r *Mux) Put(pattern string, handlers ...interface{}) {
    h := append([]interface{}{Route(pattern), ResourceControl}, handlers...)
    r.Mux.Put(pattern, h...)
}

func (r *Mux) Patch(pattern string, handlers ...interface{}) {
    h := append([]interface{}{Route(pattern), ResourceControl}, handlers...)
    r.Mux.Patch(pattern, h...)
}

func (r *Mux) Delete(pattern string, handlers ...interface{}) {
    h := append([]interface{}{Route(pattern), ResourceControl}, handlers...)
    r.Mux.Delete(pattern, h...)
}

func (r *Mux) Trace(pattern string, handlers ...interface{}) {
    h := append([]interface{}{Route(pattern), ResourceControl}, handlers...)
    r.Mux.Trace(pattern, h...)
}

func (r *Mux) Options(pattern string, handlers ...interface{}) {
    h := append([]interface{}{Route(pattern), ResourceControl}, handlers...)
    r.Mux.Options(pattern, h...)
}

func (r *Mux) Group(fn func(r chi.Router)) chi.Router {
    return r.Mux.Group(fn)
}

func (r *Mux) Mount(path string, handlers ...interface{}) {
    r.Mux.Mount(path, handlers...)
}

func (r *Mux) URLFor(key string, params map[string]interface{}) string {
  // TODO
}

func NewRouter() *Mux {
    r := &Mux{chi.NewRouter()}
    return r
}
// EOF

// ... now... you can use your own mux

app := nmux.NewRouter()
app.Post("/users", func(ctx context.Context, w http.ResponseWriter, r *http.Request) {
   http.Redirect(w, r, app.URLFor("users", map[string]interface{}{"id", user.ID()}
})

you'll have to implement your own named route registry into the custom Mux{}, but it will work with all of chi and be passed throughout.

A simpler, but a bit more verbose approach is just to write some explicit helper functions that generate urls based on some scheme... URLFor("users/:id", Params{"id", user.ID()}) which could implement your own conventions that adhere to REST for example.

Having said that though, I'm not sure why you need to redirect to these resources after making a new object, its better to just return the object itself on Post(), the same presenter function pretendDisplayUser(w, userID) used in Get() should be used to return in Post(), no redirection.. better.

@c2h5oh
Copy link
Contributor

c2h5oh commented Jun 2, 2016

It's not as easy as it looks - how would the named routes work in this example?

app := chi.NewRouter()

userRouter := chi.NewRouter()
userRouter.Post("/", "user_create", UserCreateHandler)
userRouter.Get("/:id", "user_view", UserViewHandler)

app.Mount("/users", userRouter)
app.Mount("/admin/users", adminMiddleware, userRouter)

@pkieltyka
Copy link
Member

pkieltyka commented Jun 2, 2016

either way, named routes (as defined by above examples) are not the concern of core chi, but would need to be a router enhancer that is outside of the core lib.

@pkieltyka
Copy link
Member

@c2h5oh the middleware approach could work as well, but it would have to register the path's when the middleware is mounted, and the handler itself would just next.ServeHTTP(w,r) and nothing more.

@lwc
Copy link

lwc commented Jun 3, 2016

Haha, I certainly wasn't channeling rails! Also yeah that example was contrived to illustrate my requirement, not adhere to REST best practices 😉

It's a fairly common DRY practice to repurpose your routes & router for url generation, but I can appreciate you keeping it out of scope. Decorating the router is a viable solution.

@pkieltyka
Copy link
Member

btw, what do you guys think about 3c613e2 ? for chi v2, I believe the names Group() and Stack() reflect better the intention of the methods. That is, to group routes by a routing pattern and to stack one router ontop of another without a pattern.

@VojtechVitek
Copy link
Contributor

Let's discuss in #48.

@pkieltyka
Copy link
Member

Closing to move discussion to #48 and #49

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

No branches or pull requests

5 participants