Skip to content

[Routing] Support PHP 8.1 enums as route name #44563

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

Closed
ThomasLandauer opened this issue Dec 10, 2021 · 10 comments
Closed

[Routing] Support PHP 8.1 enums as route name #44563

ThomasLandauer opened this issue Dec 10, 2021 · 10 comments
Labels

Comments

@ThomasLandauer
Copy link
Contributor

Description

I'm trying to avoid strings as Route name, so I've created a Page class that holds consts for it:

abstract class Page
{
    public const HOME = 'SomeUselessStringThatNobodyNeeds';
}

Controller:

#[Route(path: '/', name: Page::HOME)]

I think this would be a perfect use-case for a pure enum:

enum Page
{
    case Home;
}

Controller:

#[Route(path: '/', name: Page::Home)]

Right now, this leads to:

Symfony\Component\Routing\Annotation\Route::__construct(): Argument # 2 ($name) must be of type ?string, App\Enum\Page given

Example

No response

@nicolas-grekas
Copy link
Member

Why? I feel like there would be no benefits to use an enum here personally. At least I don't see any yet.

@GromNaN
Copy link
Member

GromNaN commented Dec 11, 2021

The purpose might be to remove this "SomeUselessStringThatNobodyNeeds".

If route name can be enum values, the UrlGenerator will have to support them. And that add a new challenge to generate a URL in Twig.

@ThomasLandauer
Copy link
Contributor Author

Benefits:

  1. Gut feeling :-) From my understanding, enums were invented for cases like this; while having a class that contains just consts feels like misusing the class idea.
  2. Indeed, getting rid of the overhead of having to define some string value (which the user doesn't need).
  3. Built-in check for uniqueness: Duplicate values might happen with consts:
    const HOME    = 'foo';
    const CONTACT = 'foo';
    ... but are forbidden in enums.

@stof
Copy link
Member

stof commented Dec 11, 2021

The URL generator uses route names as array keys in the dumped cache code, allowing to get the route info in O(1) when generating URLs. So supporting enums there will be hard.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Dec 11, 2021

To me, enums are useful to restrict possibilities to a range of limited values. Route names don't fall in this category.
Using enums here would look like a hack to me and given the technical complication (eg what should debug:router, etc), I'm not convinced we should do it.

@derrabus
Copy link
Member

As much as I'd like to explore the possible uses of enums further: I agree with @nicolas-grekas that enums are not useful here. I wouldn't store route names in constants either, btw. The proposed changed would introduce complexity for now real benefit.

@derrabus
Copy link
Member

Let's close because we aready have downvotes from several core team members. Thanks for the proposal though.

@ThomasLandauer
Copy link
Contributor Author

Yeah, closing is OK. Just one more question: How do you guys manage your route names then? I was thinking the consts are a good idea, they're also mentioned at https://tomasvotruba.com/blog/2020/12/21/5-new-combos-opened-by-symfony-52-and-php-80/#2-Route-Names-Can-be-Constants

@nicolas-grekas
Copy link
Member

php/php-src#8825 could be a solution here, allowing seamless compatibility between enumerated routes and the routing component.

@derrabus derrabus closed this as not planned Won't fix, can't repro, duplicate, stale Jun 21, 2022
@ThomasLandauer
Copy link
Contributor Author

An even better idea would be to allow getting rid of the name completely: #49981

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

No branches or pull requests

6 participants