Phoenix Router Plug impl does too many things

Phoenix Forum Proposals/Suggestions
,
1

Lets start by a story, I work with a team on a pretty big CMS with lots of modules. We split the cms using umbrella apps. So we have composer to manage contents, publisher to manage publications and media state, ad-ops for our ads etc.

This make sens and simplify a LOT the code understanding. Because when a new dev has to work on something, apps divide the amount of code base to be aware.

Then we face the problem of “phoenix liveview could not forward requests”. The suggestion were to create a “global router” and each “sub router” must be wrapped in a big quote. But it doesnt works very well in our case. So I create a small lib squid that create this virtual router for us (before v0.2.0).

But then I faced another problem, starting the app was slow, really slow. The more routes / routers we have, the more time it needed to start the app.

After looking in the code base of phoenix router, I found this little line phoenix/lib/phoenix/router.ex at v1.8.1 · phoenixframework/phoenix · GitHub

This line is called when by plug MyAppWeb.Router in the endpoint. And guess what? The real problem is here phoenix/lib/phoenix/router.ex at v1.8.1 · phoenixframework/phoenix · GitHub

This prevent ANY others routers to be called. And yeah I guess we could find a better way to manage NoRouteError. Without this error we could easily have something like

defmodule MyAppWeb.Endpoint do
  # ... basic stuff
  plug MyUmbrellaApp1.Router
  plug MyUmbrellaApp2.Router
end

I tried something on squid (v0.2.0) and it works well squid/lib/squid/router.ex at main · Omerlo-Technologies/squid · GitHub .

I’m not here to talk about squid, but more to find an elegant way to fix that directly on Phoenix.

IMO the forward macro wasn’t the real problem, we stay focus on it whereas trying to understand the real problem.

Edit : the proposal is to rework a little bit the route handling of Phoenix.Router macro

1 Like
2

I am missing how the start of the app slows down by adding routes and how it is connected to the code you point to…

(Also: bump!)

3

It was slow because of squid v0.1.X : the code creates a virtual router. This virtual router was build in runtime by readings attributes from N router modules. So on every starting it has to create the module. In our code, with have almost thousand of routes (that’s why it it took times).

But with the v0.2.X we rely on the routing system from phoenix. So now we don’t have any virtual router anymore. That mean the starting time isn’t impact anymore. This is the reason why I think we could change a little bit how phoenix work to have this multi router system directly on phoenix framework.

But if you want / need this, you can use squid right now. We use it in production so I want to say it’s pretty safe to use. I keep in in major 0 since we may have big changes with time (I’m 90% sure we’ll not change how the routing system works)

–

If you want to read the bad code I did, it’s here squid/lib/squid_web/router.ex at 96f6dc8d6c15354a4208ef6a4317ac9609bf0481 · Omerlo-Technologies/squid · GitHub

The “good” code is here: squid/lib/squid/router.ex at main · Omerlo-Technologies/squid · GitHub

(Also thx for the bump ;p)

4

Maybe a first step would be to find out what the actual proposal is. Maybe “allow routers to be chained”, which could look like this:

plug MyAppWeb.RouterA, raise_not_found: false
plug MyAppWeb.RouterB, raise_not_found: false
plug MyAppWeb.RouterC

Would that make sense?

2 Likes
5

That seems to be the request indeed.

Thinking of it, would it not be more explicit to

  • name it with_catch_all
  • also allow a custom function for handling unfound. (Explicit, helping goto-def)
6

Yes and no, because if you raise_not_found this will only display the routes defined in the last router. If we want to display all routes properly, we need refactor how routes works at base :confused: