Coder Social home page Coder Social logo

Comments (8)

pkieltyka avatar pkieltyka commented on May 4, 2024 1

@lwc thanks for the references.

@kevinconway yea it's been talked about a few times and I've spent a lot of time thinking and iterating to try to get the cleanest design of the chi Router.

Here were the options..

  1. For each routing method function, we have variations accepting a http.Handler or http.HandlerFunc - as described in this issue: Get(string, http.Handler) and GetF(string, http.HandlerFunc)
  2. Single routing methods that accept handler argument of type http.Handler
  3. Single routing methods that accept handler argument of type http.HandlerFunc

Option 1 isn't ideal because it makes the chi.Router interface huge, by adding another 9 functions to the interface. The simplicity and surface of the Router is important to me to stay as minimal as possible. Btw, I was initially thinking to support Get(string, http.HandlerFunc) and GetH(string, http.Handler).

Option 2 is what I had originally in chi v1 until I realized option 3 is a bit more elegant because it offers a cleaner API. Under option 2, you'd have to wrap endpoints such as..

func myHandler(w http.ResponseWriter, r *http.Request) {
  // ...
}

with http.HandlerFunc(myHandler), such as r.Get("/", http.HandlerFunc(myHandler)). This would be quite common and isn't ideal.

Option 3 (the current implementation) accepts func(http.ResponseWriter, *http.Request) to the routing methods as its the underlying type of http.HandlerFunc, which I believe is the most common case for passing to routing methods. The downside is for http.Handler's that are passed, they must call .ServeHTTP to make it a HandlerFunc signature. I believe if I'm foregoing option 1, then this becomes to best choice.

from chi.

kevinconway avatar kevinconway commented on May 4, 2024 1

with http.HandlerFunc(myHandler), such as r.Get("/", http.HandlerFunc(myHandler)). This would be quite common and isn't ideal.

I believe this is consistent with the standard library. The net/http module provides a Handle method that accepts any http.Handler implementation and then a HandleFunc that accepts any http.HandlerFunc implementation. My personal inclination is to match the standard lib so this mux becomes a drop-in replacement if most cases.

it makes the chi.Router interface huge, by adding another 9 functions

For cases outside of the usual Handle and HandleFunc cases, I think another possible approach is to move the HTTP method to an input string rather than the function name which would only require two new funcs:

r.HandleM("GET", "/", myHandler)
r.HandlerFuncM("GET", "/", myHandlerFunc)

In any case, if you're pretty settled on Option 3 then you can close this issue. I'll keep passing .ServeHTTP around.

from chi.

zythosec avatar zythosec commented on May 4, 2024 1

@pkieltyka @kevinconway new to this discussion, and not sure what you mean by passing around ServeHTTP, but one way I've implemented it that may help (and not sure how it can be incorporated into chi or even that it needs to be) is by wrapping the chi.Mux.ServeHTTP method.

I define a custom Router type that wraps a chi.Mux and defines a ServeHTTP method that wraps the chi.Mux.ServeHTTP method:

type Router struct {
    Mux    *chi.Mux
}

// ServeHTTP wraps the ServeHTTP of our mux choice
// This allows us to pass it into http.ListenAndServe and http.ListenAndServeTLS
func (r Router) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    r.Mux.ServeHTTP(w, r)
}

All my handlers get to keep the same standard signature:

func myHandler(w http.ResponseWriter, r *http.Request) {
   // ... 
}

I create a chi.Mux and add middleware, initialize routes, etc. and then initialize the router with the mux:

mux := chi.NewRouter()
mux.Get("/", myHandler) // option 3 syntax
r := &Router{Mux: mux}

So now I can pass it into http.ListenAndServe:

http.ListenAndServe(":3000", r)

Let me know if I'm completely out in left field, but this pattern has worked for me and has been extremely nice and clean to work with.

from chi.

zythosec avatar zythosec commented on May 4, 2024 1

Yeah I just realized that was what I was doing, and that I wasn't adding anything to the discussion. Sorry for the noise. I guess my point was option 3 works nicely for me. 😄

from chi.

DeepAnchor avatar DeepAnchor commented on May 4, 2024 1

I know this is an old issue (and closed), but I have to say that I am in agreement with @kevinconway and the

r.HandleM("GET", "/", myHandler) r.HandlerFuncM("GET", "/", myHandlerFunc)

seems like it would be the most elegant way to solve the problem, while still keeping in line with the project's spirit of minimalism.

The use-case is when you have complex request handlers with non-standard signatures that simply implement the http.Handler interface. For example in scenarios like this where you have a request handler that returns an error -a pretty common case.

Anyway, my 2c for future versions is to forego option 1 from above and make http.Handler a first class citizen of Chi.

from chi.

lwc avatar lwc commented on May 4, 2024

Dupe of #79 and was also mentioned in the original PR #48 (comment)

from chi.

pkieltyka avatar pkieltyka commented on May 4, 2024

@kevinconway good point about having Handle() and HandleFunc() the same in chi as in http.ServeMux - and in fact I checked and its matches up in the current version.

Interesting idea to pass the http method, perhaps we'll do that sometime. For now, yea let's stick to option 3 and we can leave this ticket open for a bit for others to offer some feedback too.

from chi.

pkieltyka avatar pkieltyka commented on May 4, 2024

hey @rossnanop - hmm, you can definitely wrap the chi router, and I encourage it as necessary to compose additional logic, but in the small example you already have this out of the box:

r := chi.NewRouter()
r.Get("/", myHandler)
http.ListenAndServe(":3000", r)

from chi.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.