Use PHP attributes for route discovery

Think the MR got altered. Seeing 900+ changes for ckeditor, composer, etc. Is that expected?

@smustgrave seems ok now. I saw something similar on a different issue. I think its a gitlab problem.

Will add to my list.

@catch / #15 I went with an event due to consistency with the existing routing building events.

I can take a look at #13:

a unit test of AttributeRouteDiscovery would be good.

I've opened an issue to decide on where to put annotation classes, so all annotation issues can work to a common standard: 📌 [policy, no patch] Decide on a namespace for PHP Attribute classes Fixed .

A couple of extracted comments from @alexpott and @berdir in slack:

alexpott
  3 days ago
OTOH we could teach POTX to extract _title from Route attributes like it does with routing yml


alexpott
  3 days ago
I have no idea how _title ends up in TranslatableMarkup :smile:


alexpott
  3 days ago
Oh that happens here… \Drupal\Core\Controller\TitleResolver::getTitle

berdir
  3 days ago
you're too fast, was just about to paste that


berdir
  3 days ago
didn't know we make any raw variable available as a placeholder, can't imagine that this is often used and seems pretty scary


alexpott
  3 days ago
Yeah so I think we’ll need to open an issue to extend POTX to support discovery of certain attributes… which given what it already does shouldn’t be too bad.

Before reviewing can the remaining tasks be crossed out for what has been completed please.

I've added more test coverage so crossed that out. Not sure if it's enough test coverage though.

Seems to have some open threads.

Addressed feedback.

Left a review and nitpicks, not sure core need to adopt sophisticated logic from SF with automatic route name generation

Somehow it discovery should report about routes without name

@andypost think there are some questions in the MR for you? There are still some open threads.

not sure core need to adopt sophisticated logic from SF with automatic route name generation

Why not? Naming thing with IDs opens the possibility of namespace conflicts. Less hard coded IDs would be a good thing.

I mean if route name will be generated from class-name or whatever it will be DX-- to use such names in URL generator and making links

> Why not? Naming thing with IDs opens the possibility of namespace conflicts. Less hard coded IDs would be a good thing.

All it means is that instead of having a code ID that shouldn't change, you have a class that you can't rename.

Routes need to be referred to by other routes, child menu items etc etc.

I actually think the introduction of route IDs in D8 was a really good thing -- much easier to deal with route IDs rather than paths.

As a custom module developer and site builder, I think I'd be fairly averse to auto generated route names based on the controller class.

Would it include the namespace? I might have more than one DownloadController across a suite of modules for example.

• I shouldn't need a cli tool to discover a route name.
• I definitely don't want a route name to change if a controller class changes.
• I do want to be able to define a route name explicitly and search for its definition in my IDE.

What about _title_callback?

As a custom module developer and site builder, I think I'd be fairly averse to auto generated route names based on the controller class.

Personally I think I like automatic route names based on the class name.
Obviously it should include the namespace.

But if we want to keep custom ids, one pattern that can be useful in combination with attributes is class constants.

class MyController {
  const ROUTE_ID = 'mymodule.myroute';

  #[Route(id: self::ROUTE_ID, ...)]
  public function page(..) {}
}

Then other code can target that constant to refer to the route.

$url = Url::fromRoute(MyController::ROUTE_ID);

Of course, this again makes that code dependent on the class holding the constant.
To avoid that, the constant could live in a separate class..

A big question is also whether we want to convert the other yml files to attributes, e.g. for local tasks, menu links etc.
And as a consequence, will we refer to routes more often from yml files or from php code?
In yml files we cannot use constants (afaik), we cannot use class imports, and string ids generally look nicer than class names in yml.
In php code, I prefer class constants over string literals.

I wonder if we could have local tasks, menu links, etc, also defined in the controller attribute. When creating a controller, and you want a local task to appear, it might be quite nice DX to just simply attach an attribute to the controller, rather than having to edit YAML files and have to manage the references. If things like local tasks can be defined in an attribute with the controller, that also simplifies it slightly because then you don't need to know or care what the route name is.

@AaronMcHale great idea, ++ to add to summary and probably as follow-up issue

Missing hook_menu()? :)))

> Personally I think I like automatic route names based on the class name.
> Obviously it should include the namespace.

It's not explicitly stated in the BC policy, but route names have to be public API so that you can work with them to generate links, add extra pages to the UI, etc.

Automatic route names would mean the controller class name becomes API. I'm not automatically against that, but it's not what we're used to.

Also, it would worsen DX because how is a developer supposed to find the route name?

Building on my previous comment, and the concerns about automatic route naming. If we're going to use attributes anyway, then why not just define the route name in the attribute.

That would address the BC and DX problems stated above, automatic route naming sounds nice in theory but it doesn't sound like it's worth it for the trade-offs mentioned.

It could also make it harder for new developers to understand what's going on, if the route names are generated automatically, we're abstracting away a key piece of information that could really help someone new understand routes.

The big DX win here is the ability to define the route (and potentially links, etc) right there in the controller class. Just that in itself is a huge win!

Not sure if this is irrelevant but a contrib module is already using PHP attributes for hook discovery - https://www.drupal.org/project/hux →

> If we're going to use attributes anyway, then why not just define the route name in the attribute.

+1

> Missing hook_menu()? :)))

Hmmm yes and no! Having to mess about in THREE different Yaml files to add a new admin tab is tedious. But hook_menu() wasn't simple, and there were plenty of ways that could go wrong too. I seem to remember complications around the MENU_DEFAULT_TASK value or something like that.

So I'd cautiously say yes to some sort of additional attributes to say 'Put this route in the menu' and 'Put this route in a tab set'. But as a follow-up issue - let's keep this one reasonably simple, and allow addition of future attributes when we design it.

> The defaults._controller value is determined by the method you add the attribute to

I don't want to add complexity to this issue, so it should be a follow-up, but we should bear in mind in the building of this that we might want to be able to also use a Route attribute on a form class, where defaults._form would be the route property that gets set to the form class, rather than defaults._controller set to the method.

I don't want to add complexity to this issue, so it should be a follow-up, but we should bear in mind in the building of this that we might want to be able to also use a Route attribute on a form class, where defaults._form would be the route property that gets set to the form class, rather than defaults._controller set to the method.

I support it. But this should be a different attribute class.

So I'd cautiously say yes to some sort of additional attributes to say 'Put this route in the menu' and 'Put this route in a tab set'. But as a follow-up issue - let's keep this one reasonably simple, and allow addition of future attributes when we design it.

A question is do we "pollute" the route list with these meta keys, or do we collect a different kind of item that we then use to generate routes _and_ links?

Updated MR to be from 11.x and updated all the deprecations to be for 10.3.0 as this is not going to make 10.2.0

POTX can read attributes - yay - so I think this is ready... going to add back the conversion of a single route. I think this issue is ready FWIW.

Very interesting — this looks indeed … very ready. Posted a few questions on the MR :)

We should and can build CLI tools to expose all the routes provided by a module. This would be good not just for routes determined by attributes but also dynamic routes

This is true for not just routes, but also for all plugins of any given type: that is a frequent challenge when working on migrations (), validation (), and more.

I've added code to not support certain things (locale and localized paths) form the Symfony route attribute. I guess there is a question about whether we should have our own attribute... but it is kinda nice to not have our own. Not as sure as I once was :)

Related may need some work too

@alexpott
You created two change records:
- https://www.drupal.org/node/3324758 →
- https://www.drupal.org/node/3314769 →

Neither of them had any information in it, so I started filling out some information on the first.
Is there a reason for us to keep the second one around?

@duadua I think I got a bit change-record-keen :)

I really like the idea of copying Symfony 6.4 and adding an always there FQCN route alias. That would mean getting 📌 Support route aliasing (Symfony 5.4) and allow deprecating the route name Needs work done. Definitely think it is worth a follow-up. I wonder if we should forgo the automatically generated names like 'MethodRouteLocale' and instead where no name is provide automatically name using the FQCN that Symfony would create an alias for?

Good question. I think I'd expect these to be declared in a modules services file - maybe an event subscriber. Yeah - this change introduces RoutingEvents::STATIC so these callbacks could be changed to an event subscriber listening to that event and then add their routes in.

Added https://www.drupal.org/project/drupal/issues/3401172 📌 Figure out how to handle generating dynamic Active

@duadua I think I got a bit change-record-keen :)

For a while I was trying to figure out whether there is a logical space in which the slightly different title might exist.
I tried to delete the particular change record, but there are some additional permissions required. Someone will probably see the updated title → .

I really like the idea of copying Symfony 6.4 and adding an always there FQCN route alias. That would mean getting #3159210: Support route aliasing (Symfony 5.4) and allow deprecating the route name done. Definitely think it is worth a follow-up. I wonder if we should forgo the automatically generated names like 'MethodRouteLocale' and instead where no name is provide automatically name using the FQCN that Symfony would create an alias for?

What feels very sensible for me. Looking at the current way its generated, a FQCN name would be no way longer compared to the current autogeneration.
Looking at this it just let me realize that auto-generation has the advantage of avoiding accidental re-definitions of the same route name.

CREATE TABLE IF NOT EXISTS "router" (
"name" VARCHAR(255) NOT NULL DEFAULT '',
"path" VARCHAR(255) NOT NULL DEFAULT '',
"pattern_outline" VARCHAR(255) NOT NULL DEFAULT '',
"fit" INTEGER NOT NULL DEFAULT 0,
"route" BLOB DEFAULT NULL,
"number_parts" INTEGER NOT NULL DEFAULT 0,
"alias" VARCHAR(255) NOT NULL DEFAULT '',
 PRIMARY KEY ("name")
);
CREATE INDEX "router_pattern_outline_parts" ON "router" ("pattern_outline", "number_parts");
CREATE INDEX "router_alias" ON "router" ("alias");

Looking at the schema for the router, I see there is a limit of length for the route name. Looking at everything, 255 seems plenty of space though for:

• Drupal
• Module name
• "Controller"
• Controller name
• Method name

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Couple of scenarios which comes in my mind, which we should address:

• Currently multiple route definition can point to a single class method, How we will define routes like that with php attributes?
• Route names are used to define menu links, menu tabs and at various other places also, I think we are aiming to generate auto route names but it also kind of relates to above point, How we are going to address that?
• What would be the process of overriding an existing route? Using route subscribers or something else?

Would love to see this happen, this would be a massive DX improvement! Bumping priority 😇

I suspect this is also a de facto hard blocker for 📌 Make controllers invokable for DX and maintainability Active ? 🤔

P.S.: would also help Experience Builder: #3490069-13: Auto-register XB's controllers' invokable services → .