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

[Routing] Use the controller's FQCN as default internal route name, instead of app_controller_method #52945

Open
ThomasLandauer opened this issue Dec 8, 2023 · 3 comments

Comments

@ThomasLandauer
Copy link
Contributor

Description

Currently, when a route is defined without a name, Symfony creates a default name in the form: app_<controller>_<method> (see AttributeClassLoader::getDefaultRouteName())
I'm suggesting to change this to the controller's FQCN: App\Controller\MyController.
In other words: Change the newly created FQCN alias (introduced in #50084) to be the actual name:

Controller Current system New system
name: 'foo' name: foo
alias: App\Controller\MyController
<= same
name is not set name: app_my__invoke
alias: App\Controller\MyController
name: App\Controller\MyController
alias: none

Reason: Twig!

The idea of #49981 was to get rid of manually creating a name, and always just use the FQCN. But in Twig

{% if 'App\Controller\MyController' == app.current_route %}

... doesn't work, since app.current_route is app_my__invoke.

Although I doubt that many people rely on the current default route (the syntax isn't even documented properly, see https://symfony.com/doc/current/routing.html#generating-urls), changing it would still be a BC break.

As a next step, the docs could be changed to (I could take care of that):

  • Give a clear recommendation for invokable controllers.
  • Remove the route name from most code samples, and explain it as a solution for special cases only.

Example

No response

@afilina
Copy link
Contributor

afilina commented Feb 29, 2024

I dislike convention-based naming, like ZF1 used to do everywhere. However, this seems like indeed an unnecessary BC break, which is also hard to discover. I feel like the risk outweighs the benefits.

I explicitly set all my routes with FQCN like this: #[Route('/', name: self::class)] to avoid managing arbitrary strings.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 29, 2024

I agree with Anna of.

Another idea: {% if 'App\Controller\MyController' == app.current_route %} could be turned into something like {% if is_current_route('App\Controller\MyController') %} or similar, WDYT?

@yceruto
Copy link
Member

yceruto commented Feb 29, 2024

I agree with Anna too.

Another idea: {% if app.is_current_route('App\Controller\MyController') %} and check against both name and alias.

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

No branches or pull requests

6 participants