Problem/Motivation

When rest module is enabled, it's \Drupal\rest\EventSubscriber\ResourceResponseSubscriber is initialized on every request as it's a response subscriber. It has the serializer injected, which in turn gets every single registered normalizer inject, all with their own dependencies. That's 25 service in my case, all with their own dependencies.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3615308

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:

Comments

berdir created an issue. See original summary.

berdir’s picture

On the site I saw this, this loads about 160 classes less on every request, based on our preload script that generates a preload.php from the files that loaded on a dynamic page cache hit.

berdir’s picture

Status: Active » Needs review

\Drupal\jsonapi\EventSubscriber\ResourceResponseSubscriber is similar although likely far fewer services. Can include that in the same MR or create a separate one, either works for me.

berdir’s picture

Title: Use Service Locator in ResourceResponseSubscriber » Use Service Closure in ResourceResponseSubscriber
berdir’s picture

Updated jsonapi now as well. It's identical and straightforward and there are quite a few services involved as well there.

These are event subscribers, which are like hooks and hook classes excluded from BC, so I think we don't do BC there.

berdir’s picture

Status: Needs review » Needs work
berdir’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

From reviewing a syslog closure ticket this seems to be inline and pipeline is green.

Not sure if it needs a CR though?

berdir’s picture

IMHO not as they are event subscribers and for jsonapi even explicitly also tagged as internal. Found no subclasses of them either.

smustgrave’s picture

Kinda what I figured but thanks for checking!

  • catch committed 7fce6988 on main
    task: #3615308 Use Service Closure in ResourceResponseSubscriber
    
    By:...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Yes unlike plugins and controllers where there's a reasonable chance someone somewhere is subclassing them depending on which one it is, there's very little chance for an event subscriber, so I think we're fine without any bc here.

Committed/pushed to main, thanks!

This will need a backport MR for 11.x but I was surprised to get commit conflicts at all, suggesting we haven't backported something else to these constructors which might make a backport more complex. So I'm moving to fixed, but if you think an 11.x backport is worthwhile please re-open with an MR.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

berdir’s picture

Status: Fixed » Needs review

Yes, we haven't backported the autowiring/constructor stuff, so the 11.x MR is quite a bit bigger, but I think it's worth it, 11.x will be with us for a while, and for sites using these modules the improvement is pretty significant, especially when they're not primarily decoupled but just have a few integration services or so.

catch’s picture

Version: main » 11.x-dev
Status: Needs review » Fixed

That looks pretty straightforward. Committed/pushed to 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed 5348691e on 11.x
    task: #3615308 Use Service Closure in ResourceResponseSubscriber
    
    By:...
berdir’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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