Problem/Motivation
This is a follow-up issue to #1578832: [already commited but followup for CR?] <front> menu links are missing active trail classes.
Fixing the other issue introduced a performance penalty for the front page because an extra query had to be run in the menu system to check for link items matching <front>. This could be avoided if we had the ability to load menu links for multiple route names instead of just one at a time. Per @larowlan:
I don't think we can avoid the extra queries because we only have \Drupal\Core\Menu\MenuLinkManager::loadLinksByRoute and \Drupal\Core\Menu\MenuTreeStorageInterface::loadByRoute but I think it would be worth adding loadLinksByRoutes and loadByRoutes methods to both those services to support doing this in a single query.
@catch agreed that this task could be done in a follow-up issue.
Proposed resolution
- Add
\Drupal\Core\Menu\MenuLinkManager::loadLinksByRoutes. - Add
\Drupal\Core\Menu\MenuTreeStorageInterface::loadByRoutes. - Update
\Drupal\Core\Menu\MenuActiveTrail::getActiveLink()to use loadLinksByRoutes() for better performance.
Remaining tasks
User interface changes
Introduced terminology
API changes
Two new functions, one in MenuLinkManager and another in MenuTreeStorageInterface.
Data model changes
Release notes snippet
Issue fork drupal-3523497
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
larowlan,
Comment #4
dcam commentedComment #5
dcam commentedComment #8
vasikeI confirm this issue.
Another example of using this improvement it's for contrib modules as Menu Breadcrumb.
MR updates
- I merged the main, instead of rebase as there were too many commits
- Lost some changes in the solving conflicts
- Small PHPStan fix
- Update the tests with previous updates + new values for query counts.
- The tests failure not related with this.
Needs review , for now ... till we'll get the right status from community & maintainers.
Comment #9
smustgrave commentedVery very small comments. But new API will need a CR. This may need submaintainer sign off who is pwolanin
Comment #10
vasikeMR review addressed - all Green.
Also created an change record ... maybe also a reiew/update there would help.