Problem/Motivation

In #3392616: Update to Symfony 6.4 we decided to convert both BookNavigationCacheContext and MenuActiveTrailsCacheContext to use lazy services.

This issue was created to do just that.

Currently both classes are container aware, so not to load an expensive service when not needed.

Steps to reproduce

Proposed resolution

- Change \Drupal\book\BookManager and \Drupal\Core\Menu\MenuActiveTrail to laze loading services.
- Remove container awareness from BookNavigationCacheContext and MenuActiveTrailsCacheContext

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3397173

Command icon 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:

About issue forks

Comments

Spokje created an issue. See original summary.

spokje’s picture

spokje
Primary language Dutch
commented
Title: Convert both BookNavigationCacheContext and MenuActiveTrailsCacheContext into lazy services » [PP-1] Convert both BookNavigationCacheContext and MenuActiveTrailsCacheContext into lazy services
Status: Active » Postponed
Related issues: +#3396928: Deprecate Drupal ProxyBuilder in favor of Symfony lazy services

Postponing on the possible adoption of native Symfony lazy services in #3396928: Deprecate Drupal ProxyBuilder in favor of Symfony lazy services .

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: Unassigned » spokje
Status: Postponed » Active

#3396928: Deprecate Drupal ProxyBuilder in favor of Symfony lazy services is trickier than expected, let's not postpone better on perfect here.

spokje’s picture

spokje
Primary language Dutch
commented
Title: [PP-1] Convert both BookNavigationCacheContext and MenuActiveTrailsCacheContext into lazy services » Convert both BookNavigationCacheContext and MenuActiveTrailsCacheContext into lazy services
Related issues: -#3396928: Deprecate Drupal ProxyBuilder in favor of Symfony lazy services

Spokje opened merge request !5172

spokje’s picture

spokje
Primary language Dutch
commented
Title: Convert both BookNavigationCacheContext and MenuActiveTrailsCacheContext into lazy services » Convert both BookNavigationCacheContext and MenuActiveTrailsCacheContext to use lazy services
Issue summary: View changes
spokje’s picture

spokje
Primary language Dutch
commented
Issue summary: View changes
spokje’s picture

spokje
Primary language Dutch
commented
Status: Active » Needs review
Parent issue: #3392616: Update to Symfony 6.4 » #3397519: [META] Reduce use of ContainerAware classes where possible
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Pipeline is green and CR has the main points.

Almost marked because the syntax in the generated classes seemed off but checking others guess that's the case of how they get generated. And gitlabCI didn't complain.

If this gets in should the scope of #3396928: Deprecate Drupal ProxyBuilder in favor of Symfony lazy services be expanded?

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: spokje » Unassigned

  • catch committed 8aa03123 on 11.x 
    Issue #3397173 by Spokje, smustgrave: Convert both...

  • catch committed 1221e7e5 on 10.2.x 
    Issue #3397173 by Spokje, smustgrave: Convert both...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 10.2.x, thanks!

Spokje closed merge request !5172

spokje’s picture

spokje
Primary language Dutch
commented

Published CR.

hephaestus’s picture

hephaestus commented

This change appears to be causing the issue discussed in #3403778: MenuActiveTrail ($menu_link_manager) must be of type Menu\MenuLinkManagerInterface, DependencyInjection\Container given after upgrading to 10.2.0-beta1.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.