Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Let's add Dependency Injection and split up the classes.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3543241
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:
- 3543241-modernize-announcements-feed
changes, plain diff MR !13091
Comments
Comment #3
nicxvan commentedComment #4
mstrelan commentedLooks fine to me, only question is if we've given up on single hook classes with __invoke methods, like
\Drupal\file\Hook\CronHook? Otherwise we could do this for cron and help here, even theme if we don't intend to use it for anything else.Comment #5
nicxvan commentedI'm not really a fan of that approach to be honest.
Comment #6
nicxvan commentedComment #7
smustgrave commentedQuestion from my phpstorm. Since all the parameters are being marked readonly phpstorm wants to mark the class readonly too. Is that valid?
Comment #8
nicxvan commentedI have no idea the implications of that, I would need to read up.
Comment #9
smustgrave commentedProbably not a blocker here. Updates LGTM.
Comment #10
mstrelan commentedYeah I've considered the same. If the class is readonly then all properties are readonly, including those of child classes. I believe that would be fine, but given there is pushback for having final classes then I guess we shouldn't restrict what they can do either?
Comment #11
smustgrave commentedWas more curious, it was a recommendation IDE was making not sure where it pulls it from
Comment #12
mstrelan commentedIf the php language level in phpstorm is 8.2 or above (when readonly classes were introduced) and all properties on the class are marked readonly, then it will suggest that the class can be readonly.
Comment #13
astonvictor commentedI left one comment. Does it make sense?
Comment #14
smustgrave commentedNo it’s not needed with the hooks
Comment #15
nicxvan commentedIt's actually all constructors that don't need comments, not just ones on hooks: https://www.drupal.org/docs/develop/standards/php/api-documentation-and-...
Comment #16
astonvictor commentedok, thanks for the reply
I just saw that almost all constructor methods in the core contain comments e.g.
Constructs a {Class name} object..So, it's like optional but recommended.
Comment #17
mstrelan commentedMore like it's legacy from a time before typed parameters and properties.
Comment #18
catchI think readonly classes vs. individual readonly properties (when all of them are readonly) might be a coding standards question - in that we should choose one or the other.
Committed/pushed to 11.x, thanks!