- Issue created by @amateescu
- Merge request !11211Add a proposal for the new method on RouteMatchInterface. β (Open) created by amateescu
- π·π΄Romania amateescu
Started a MR with a proposal for the new method. Feedback welcome :)
- πΊπΈ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?
- π·π΄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 newRouteMatch::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.
- Status changed to Needs work
about 1 month ago 7:27am 22 April 2025 The Needs Review Queue Bot β tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily 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.
- π·π΄Romania amateescu
If it doesn't match, we can call
RouteProvider::getRouteByName()
on each of the passed in routes, this will trigger the deprecation we need.@catch, the issue is that
RouteMatch::isRouteName()
will _not_ match the current route name in most (almost all) cases, so we'll be calling::getRouteByName()
all the time. Maybe it's easier to explain with an example:Let's say we have a few different modules, and each one checks for a route, e.g.
::isRouteName('module1_controller')
,::isRouteName('module2_form')
, etc.Now, imagine that those checks happen on every request, wouldn't this mean that we'll be "attaching" the knowledge of those routes to every page in the context of the route preloader?
- π¬π§United Kingdom catch
Now, imagine that those checks happen on every request, wouldn't this mean that we'll be "attaching" the knowledge of those routes to every page in the context of the route preloader?
The route preloader currently loads every non-admin route unconditionally on every request, so it wouldn't make things worse or better, unless we change the behaviour in the issue above, which we might do.
- First commit to issue fork.
- π¦πΊAustralia acbramley
Rebased and fixed tests, marking the MR as ready as it's looking great to me.
- πΊπΈUnited States smustgrave
Applied the MR and searched for \Drupal::routeMatch()->getRouteName() to see if other instances. There were 4 but they weren't doing directly route checking.
All feedback on the MR already appears to be addressed.
Believe good to go