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] Clarify route aliases examples with explicit route names #20688

Open
wants to merge 2 commits into
base: 6.4
Choose a base branch
from

Conversation

welcoMattic
Copy link
Member

Refers to #20638 (comment)

Actual examples for route aliasing are not crystal clear.

Now, examples in YAML, XML and PHP provide

  • An original route, declared with an explicit name
  • An alias to this route, with an explicit name

This change will allow to provide another clear example for route aliasing using Attribute in #20638

cc @stof @damienfern

@welcoMattic welcoMattic changed the title [Routing] fix(route-alias): Clarify route aliases examples with explicit route names [Routing] Clarify route aliases examples with explicit route names Feb 25, 2025
@carsonbot carsonbot added this to the 6.4 milestone Feb 25, 2025
Co-authored-by: Christophe Coevoet <[email protected]>
@welcoMattic
Copy link
Member Author

@javiereguiluz this one could be merged to unlock #20638

@javiereguiluz
Copy link
Member

I'm not sure about these changes. I think the previous description (old route + new route) was more clear.

The new names (some route + alias to some route) could be less clear, especially the new route name. Including "alias" as part of the route alias is unnecessarily long in my opinion and could make some readers think that the route alias must contain that alias prefix.

Let me ask @xabbuh and @OskarStark about this. Thanks!

@welcoMattic
Copy link
Member Author

I'm not sure about these changes. I think the previous description (old route + new route) was more clear.

The new names (some route + alias to some route) could be less clear, especially the new route name. Including "alias" as part of the route alias is unnecessarily long in my opinion and could make some readers think that the route alias must contain that alias prefix.

Let me ask @xabbuh and @OskarStark about this. Thanks!

I can replace "alias" with "new", but it implies underlying use case: replacing the some_route usage with the alias. In my mind aliases are not necessarily here to replace existing routes, but to co-exists.
Imagine 2 websites sharing the same codebase except templates, 1 for BrandA and another for BrandB. Route aliases can be used to name route according to the Brand name in dedicated templates.

We can add a site note to mention the alias name is not mandatory to contain "alias" word.

These changes are on purpose to clarify the behavior using the Attribute way to alias route.

@stof
Copy link
Member

stof commented Feb 26, 2025

If you want to replace, you will generaly define the route with the new name and add a BC alias for the old name (especially when you want to deprecate it). That was the main confusion with the existing example. new_route_name is the deprecated name.

@welcoMattic
Copy link
Member Author

welcoMattic commented Mar 4, 2025

I think we do not have to presume what users will do with route aliasing feature (e.g creating new route and create an alias with the old route name). Just showing how to generally use route aliasing with generic terms, and then users will adapt to their needs/preferences. WDYT?

EDIT: I've update the PR by adding a small paragaph to explain clearly how route aliasing can be used to provide BC for renamed routes.

cc @javiereguiluz

@javiereguiluz
Copy link
Member

Maybe it's me, but this looks very confusing:

You create an alias ... and immediately deprecate it. The alias, being a new thing that you add, feels like the new route name, not the legacy route name.


I still think we should use names like "new_route_name" and "original_route_name" or "legacy_route_name". It's true that this alias feature can be used in different ways, but I think we should focus on its main usage and use terms that are impossible to misunderstand.

@damienfern
Copy link
Contributor

I understand @javiereguiluz POV because I had the same feeling about aliases. Isn't this a sign that aliases on routes are plugged in a wrong way ?

IMHO, here is how it should be (a first draft, I don't know if it's doable) :

        some_route_name:
            path: /some-path
            controller: App\Controller\SomeController::index
            alias: 
                  alias_to_some_route_name:
                      # this outputs the following generic deprecation message:
                      # Since acme/package 1.2: The "alias_to_some_route_name" route alias is deprecated. You should stop using it, as it will be removed in the future.
                      deprecated:
                          package: 'acme/package'
                          version: '1.2'

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

Successfully merging this pull request may close these issues.

5 participants