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
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
berdirOn 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.
Comment #4
berdir\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.
Comment #5
berdirComment #6
berdirUpdated 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.
Comment #7
berdirComment #8
berdirComment #9
smustgrave commentedFrom reviewing a syslog closure ticket this seems to be inline and pipeline is green.
Not sure if it needs a CR though?
Comment #10
berdirIMHO not as they are event subscribers and for jsonapi even explicitly also tagged as internal. Found no subclasses of them either.
Comment #11
smustgrave commentedKinda what I figured but thanks for checking!
Comment #13
catchYes 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.
Comment #16
berdirYes, 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.
Comment #17
catchThat looks pretty straightforward. Committed/pushed to 11.x, thanks!
Comment #20
berdir