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

Subrouter / RouteGroups #89

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dbudworth
Copy link

Note: This is similar to closed / rejected issue #57. The difference here is that the RouteGroup imposes no request overhead as it's only a proxy to the Router.METHOD() functions that prefixes the path.

Not sure if you are interested in this being in the standard httprouter library, it could easily be a stand alone add-on. Submitting it just in case you think it will be generally useful.

-- commit message --
RouteGroup is a simple route mapper that delegates to an underlying Router
instance when mapping paths and takes no part in handling of requests
resulting in zero added per-request cost.

Implemented as a simple struct to keep in the spirit of httprouter's design.

Potential improvement being to create an interface for the common functions of
Router and RouteGroup.

@julienschmidt julienschmidt added this to the v2 milestone Sep 30, 2015
@julienschmidt
Copy link
Owner

Big thanks for this PR, it is very similar to what I imagined.
I'm currently doing a few other experiments for v2. I will merge this as soon!

@orian
Copy link

orian commented Oct 23, 2015

@julienschmidt do you plan to merge soon?

In general it might be useful to define an interface (I do that in my project):

type IRouter interface {
  // all methods here
}

Then both the Router and RouteGroup could implement it. So it could be easily passed to modules to register it's handlers.

rs pushed a commit to rs/xhandler that referenced this pull request Dec 13, 2015
rs pushed a commit to rs/xhandler that referenced this pull request Dec 13, 2015
rs pushed a commit to rs/xmux that referenced this pull request Dec 14, 2015
@julienschmidt julienschmidt mentioned this pull request Jan 17, 2016
@julienschmidt julienschmidt mentioned this pull request Oct 20, 2016
@julienschmidt julienschmidt changed the title Introduction of RouteGroup concept Subrouter / RouteGroups Oct 20, 2016
@julienschmidt
Copy link
Owner

No worries, something like that is definitely coming.

However, I'd like to keep the package as simple as possible. My current idea is to make every route essentially a subrouter, i.e. every route registration returns a handle (containing just the path as a prefix) which can be used to register subroutes. It would also further encourage hierarchical URL design.

I'm currently working on the last few projects for this term at university, afterwards I will finally bring this package up to date.

Repository owner deleted a comment from facundomedica Feb 22, 2018
Repository owner deleted a comment from Hunsin Feb 22, 2018
Repository owner deleted a comment from wrouesnel Feb 22, 2018
Repository owner deleted a comment from solarfly73 Feb 22, 2018
Repository owner deleted a comment from artem-malko Feb 22, 2018
Repository owner deleted a comment from divideandconquer Feb 22, 2018
@juandiegopalomino
Copy link

@julienschmidt hi there! This is an awesome framework, and I just wanna check on the status of this pr-- it'd be a real nice addition.

@govinda-attal
Copy link

Such a clean implementation, any update?

@leonklingele
Copy link

Hey, any news on this? 😊 @dbudworth can you rebase to master so that this can finally be merged?

@omgnuts
Copy link

omgnuts commented Jun 26, 2022

Would love this to happen! 👍🏻

@halorium
Copy link

I would love to see a sub router feature. However as I see it being implemented, it wouldn't be helpful to me. Use cases that I come across are adding an http.Handler into my httprouter api. That http.Handler will have it's own set of routes defined.

// Some 3rd party UI
// Implements the follow for example
// GET /external/stuff
// DELETE /external/stuff/:stuff_id
// POST /external/stuff
// ... and so on
thirdPartyUIHandler := thirdParty.NewHandler() // http.Handler

// my httprouter
r := httprouter.New()
// my routes
r.GET("/", api.GetRootHandler())

// now I need to add in the sub router
r.Sub("/third-party", thirdPartyUIHandler)

// that would add all the routing that the third party handler implements after "/third-party"
// e.g.
// GET /third-party/external/stuff
// DELETE /third-party/external/stuff/:stuff_id
// POST /third-party/external/stuff
// ... and so on

RouteGroup is a simple route mapper that delegates to an underlying Router
instance when mapping paths and takes no part in handling of requests
resulting in zero added per-request cost.

Implemented as a simple struct to keep in the spirit of httprouter's design.

Potential improvement being to create an interface for the common functions of
Router and RouteGroup.
@dbudworth
Copy link
Author

dbudworth commented Sep 30, 2022

Apologies, I just realized that I was asked to rebase this PR, quite some time ago.

Just did, in case there's still interest in adding the RouteGroup feature.

One annoying thing, vscode decided the comments in router.go were not proper so there was some re-indenting.
I can back those changes out if there's any interest.

@halorium there isn't really a way to interrogate an http.Handler (interface) to get the list of paths it has mapped.
But you could simply mount /third-party then use the http.StripPrefix , no? (assuming the third party lib doesn't expect the url to start with /third-party)

That would work, but it takes on the cost of "double routing", in a sense. But I think that if you are already delegating functionality to a third party library that is likely not using as efficient of a routing system, then the cost will be negligible

similark pushed a commit to similarweb/httprouter that referenced this pull request May 9, 2023
* Add contributing docs

Signed-off-by: Lucas Santos <[email protected]>

* Add README

Signed-off-by: Lucas Santos <[email protected]>

* Add magefiles

Signed-off-by: Lucas Santos <[email protected]>
@galdor
Copy link

galdor commented Apr 11, 2024

What is the current situation with this PR?

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

Successfully merging this pull request may close these issues.

9 participants