Custom Routes #223
Replies: 3 comments 2 replies
-
Woah, this is sooo cool @jfmario 🙌 I'm really impressed with how much you pulled off without guidance: adding loaders, state, test, etc. I hadn't considered the nested routing using the file-system so that's super interesting to see too. Before jumping into solutions, I have a couple non-obvious design goals in mind with custom routing that are worth aligning on:
These aren't set in stone by any means. I'm not yet sure all are achievable or if I'm missing other important goals. There's plenty of room for discussion here! So far I've been considering these solutions for custom routing:
I'm not sure which one is better right now, but I'd like to evaluate both (and other possible solutions too!). My take on pros and cons: 1. Custom routes package
2. Routes as configuration
I'm realizing I should have written this up way earlier, but thanks for kicking off the discussion. I'd love to hear your thoughts on the goals and ideas for solutions! |
Beta Was this translation helpful? Give feedback.
-
Goal 1 makes perfect sense. I wasn't considering goal 2 at all, but it also makes sense. Goal 3 was nowhere in my mind, and is completely incompatible with Consider the following: func Routes(r *routes.Router, c *controller.Controller) {
r.Get("/", c.Index)
if cointoss.TossCoin() == cointoss.Heads {
r.Get("/heads-route", c.PostsController.Index)
} else {
r.Get("/" + uuid.New().String(), c.WidgetsController.Index)
}
} That's a silly example of course, but a user might want to conditionally set routes based on some environment value. So if static analysis is required I think we're stuck with routes as config. My only opinion there would be that it would still be nice to be able to have those config files at different, nested locations (with bud providing the prefixing as necessary). Editing to add: Somewhere you mentioned that you preferred having users add code instead of configuration - so this would be against routes as config, I think. I like this philosophy, but as far as I can tell it is in direct contradiction with statically analyzing routes (or almost anything that provides users with hooks of this kind). I think both options are great, but unless I'm missing something here one of those two things is going to have to give. My personal preference is user-provided routes functions, but that's partly because that's what I'm used to with frameworks and becasue I wasn't considering the benefits you listed that can come with static routes. You can do "routes:list" in laravel, but I think Laravel just boots your entire application up before executing any CLI command anyways. It doesn't take too long, so if you accepted that you can still build docs and provide a CLI hook for routes. |
Beta Was this translation helpful? Give feedback.
-
I just thought of something, and i have no idea if it is a good or bad idea. A controller could optionally report its own route: package controller
// one option
type Controller {
BudData bool `bud_route:"/my-custom-route"`
}
// another option (NOT static)
func (c *Controller) BudRoute() string {
return "/my-custom-route"
}
// another option
const BUD_CONTROLLER_ROUTE string = "/my-custom-route" |
Beta Was this translation helpful? Give feedback.
-
I was going to hit you up with another PR, but since this is nowhere near ready and is more involved, I'm making a discussion instead.
My branch is here: https://github.com/jfmario/bud/tree/feature/custom-routes
This is an attempt to support custom routing based on the discussion in #149.
There are definitely some outstanding issues that I'll list after describing the behavior. If the general API here is correct, I'll work out the other issues. Otherwise, I'll delete - it was worth doing to this to learn about how the file generation works either way.
API
Bottom line is that with this PR, the user can do this in
routes/routes.go
(or any go file underroutes/
):The reference
router
isgithub.com/livebud/bud/package/router
. Details about the importpath for
controller
is further below.A user can also do this, in
routes/my-prefix/routes.go
:These functions (which must be called
Routes
) are called in the generatedbud/internal/app/routes/routes.go
. That file is in turn loaded bybud/internal/app/web/web.go
and those routes are setup before standard controller routes.The way you have the router set up, the routes that are added first will take precedence in any conflicts. So custom routes will trump any matching default controller routes. As for conflicts at different levels of routes files, I need to investigate the order more - I set it up to crawl for routes functions the same way you had it set up to crawl for controllers.
Most of the relevant files are in
framework/routes
.This all works, and I have a working test in
framework/routes/routes_test.go
.Issues
Okay, here are the outstanding issues which are only worth addressing if you think this general idea is correct.
controllers.go
.I can't import
<package>/bud/internal/app/controller/controller.go
from the main app. So in order to work out the concept, I moved the generated controller file tobud/package/controller/controller.go
. This is arbitrary, and I'm not sure what the best place for this file is - or if there is some other way to go about exposing that controller object.Routes
functionIn this first draft, I'm checking for a function called
Routes
with exactly two parameters, one with a type named*routes.Router
and the other with a type named*controller.Controller
. This doesn't work if they import things with other names, so I need to dig in more to ensure that they are actually the correct types.For routes functions that have a prefix, I have it set up to build subrouters. I couldn't find a straightforward way to add the tree of a child router to a parent router, nor a way to walk the tree of the child router to get its routes and handlers to add to the main router.
So I edited the
add()
method or the router to also keep track of method-route-handler associations in a running array. When a child router needs to be added to a parent router, the parent walks over this array and adds those method-route-handler associations to itself. There is surely a better way to accomplish this.TestControllerChange
brokeThe test relies on the generated controller file being refreshed during the test run, which isn't happening at the new location.
I was using the
framework/controller
package as a guide forframework/routes
. If this branch is worth continuing, I'll make sure to prune things I don't need. The loader doesn't need a reference to the dependency injection service, for example.Beta Was this translation helpful? Give feedback.
All reactions