Add an API for comparing the (current) route name that takes into account deprecated routes

Created on 14 February 2025, 2 months ago

Problem/Motivation

In πŸ“Œ Support route aliasing (Symfony 5.4) and allow deprecating the route name Needs work we added the ability to deprecate routes, but we later found that usages of RouteMatchInterface::getRouteName() for checking against specific route name(s) are problematic because they can't take into account deprecated route names.

Proposed resolution

Add a method on \Drupal\Core\Routing\RouteMatchInterface that can be used to compare the route name against a given string (or a list of them), and document that RouteMatchInterface::getRouteName() is discouraged for this purpose. That method can't be deprecated because there are valid use-cases for retrieving the route name, e.g. the route and route.name cache contexts, getting local tasks or actions for the current route, etc.

Remaining tasks

Discuss, implement the new method, etc.

User interface changes

Nope.

Introduced terminology

N/A

API changes

API addition: a new method on \Drupal\Core\Routing\RouteMatchInterface.

Data model changes

Nope.

Release notes snippet

TBD.

πŸ“Œ Task
Status

Active

Version

11.0 πŸ”₯

Component

routing system

Created by

πŸ‡·πŸ‡΄Romania amateescu

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @amateescu
  • πŸ‡·πŸ‡΄Romania amateescu

    Started a MR with a proposal for the new method. Feedback welcome :)

  • Pipeline finished with Failed
    2 months ago
    Total: 104s
    #424022
  • Pipeline finished with Success
    2 months ago
    Total: 337s
    #424049
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Appears small feedback has been provided by @acbramley.

    Wonderif there is a current instance in core that could use this and be converted to show it's worth?

  • Pipeline finished with Failed
    17 days ago
    Total: 139s
    #462862
  • πŸ‡·πŸ‡΄Romania amateescu

    Addressed all the feedback so far, but there's one thing left.. the main reason for this API: checking whether the passed-in route name is deprecated.

    The problem is that the only way to do that currently is \Drupal\Core\Routing\RouteProvider::getRouteAliases(), which would mean an uncached db query for every call to the new RouteMatch::isRouteName() method. I think we need a way to retrieve and cache (persistent and static) all aliased/deprecated routes somewhere in the route provider. Setting back to NW to get feedback on that.

  • πŸ‡¬πŸ‡§United Kingdom catch

    I'm a bit confused by #5 but I might have it backwards.

    RouteProvider::getRouteByName() has this:

        $result = reset($routes);
        if ($result instanceof Alias) {
          $alias = $result->getId();
          if ($result->isDeprecated()) {
            $deprecation = $result->getDeprecation($name);
            @trigger_error($deprecation['message'], E_USER_DEPRECATED);
          }
          return $this->getRouteByName($alias);
        }
    
    

    So... in isRouteName(), if it matches the current rout name, we can assume it's not a deprecated alias because no-one should ever actually land on one and just return TRUE.

    If it doesn't match, we can call RouteProvider::getRouteByName() on each of the passed in routes, this will trigger the deprecation we need.

    Route preloading should mean this isn't an additional database query. There is some rationalisation to do in πŸ› RoutePreloader loads a lot of routes Active but it's just another thing to take into account on that issue.

Production build 0.71.5 2024