Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.This entity reference selection handler already has entityManager injected, and uses it all over the place, so there's no reason for getReferenceableEntities() to use the deprecated procedural function for loading entities.
I figure this is safe for 8.0.x, but feel free to move it if you disagree.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2620658-1-remove-entity-load-multiple.patch | 825 bytes | bojanz |
Comments
Comment #2
bojanz CreditAttribution: bojanz at Centarro commentedOne-line fix.
Comment #3
BerdirThe entity manager is also already deprecated but it will continue to work if we just switch that out to the new service. Don't know if that's something that we can do in 8.x though as changing the injected services breaks subclasses with additional injections.
Comment #4
catchPer https://www.drupal.org/node/2562903 class constructors are considered @internal so could change in minor releases.
This change I think we could make in 8.0.x though, so worth a follow-up for injecting the service directly.
Comment #5
BerdirI've heard that before but I really think that is a bad idea and ignores the very valid case of overriding the constructor for additional dependencies.
It might not be so common for arbitrary services but for a class like this, which is basicaly a plugin base class that is meant to be overridden, with additional dependencies (entity storage is another example for this). In those cases, the statement from that link is a lie ( "These are objects where developers are not expected to call the constructor directly in the fist place").
See UserSelection in core for an example that would break if we change the constructor.
Anyway, not the right issue to discuss this ;)
Comment #6
catch@Berdir, there might be cases we decide not to do it, and this case might be one of them.
Comment #9
catchCommitted/pushed to 8.1.x and cherry-picked to 8.0.x, thanks!
However there are 13 other usages of this in core according to https://api.drupal.org/api/drupal/core!includes!entity.inc/function/enti... so would be great to have a follow-up to remove the other usages.
Comment #10
xjm