Allow plugin service wiring via constructor parameter attributes

Created on 5 July 2022, almost 2 years ago
Updated 6 June 2024, 10 days ago

Problem/Motivation

To wire services to a plugin, developers must implement ContainerFactoryPluginInterface and a custom create() method that pulls services from the container and calls the plugin constructor. This is largely boilerplate code and must be written for each plugin implementation.

Using PHP 8 attributes, we can tag constructor parameters directly and allow the factory class to discover and inject the required services, and drop the create() method entirely.

This also has the advantage of placing the service name directly next to the parameter where it is used, instead of having to keep the create() method and constructor signature in sync in two different places.

Steps to reproduce

Proposed resolution

Add support to ContainerFactory for Symfony's #[Autowire] attribute.

Remaining tasks

Determine if there are any more edge cases to cover.

User interface changes

API changes

Plugins can now wire services by specifying an #[Autowire] attribute on the each constructor parameter, instead of writing a separate create() method.

Data model changes

Release notes snippet

πŸ“Œ Task
Status

Needs work

Version

11.0 πŸ”₯

Component
PluginΒ  β†’

Last updated about 16 hours ago

Created by

πŸ‡¬πŸ‡§United Kingdom longwave UK

Live updates comments and jobs are added and updated live.
  • Needs change record

    A change record needs to be drafted before an issue is committed. Note: Change records used to be called change notifications.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • last update about 1 year ago
    Custom Commands Failed
  • last update about 1 year ago
    29,363 pass
  • last update about 1 year ago
    29,363 pass
  • Status changed to Needs review about 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    @larowlan I added a test that is similar to your trait setup but it passed first time, so not sure what I have done wrong - any ideas?

  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

    I'll see if I can still replicate it - might have been a PHP version issue?

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    For full backward compatibility we might need to provide an autowiring trait, which perhaps means we can simplify some of this.

    class MenuLinkDefault extends MenuLinkBase implements ContainerFactoryPluginInterface {
    
      public function __construct(array $configuration, $plugin_id, $plugin_definition, StaticMenuLinkOverridesInterface $static_override) {
        ...
      }
    
      public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
        ...
      }
    }
    

    In the long run we probably want to drop the create method here and use autowiring instead, but then we are no longer implementing ContainerFactoryPluginInterface, and any subclasses will no longer have their create() method called.

    So maybe the answer is to autowire the constructor, and use a trait to replace create() that handles the autowiring? Then any subclasses will work with no changes - we could also find a way to notify downstream users that they should switch to autowiring, or maybe we just live with the trait forever until we deprecate ContainerFactoryPluginInterface entirely?

  • Status changed to Needs work about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Sounds like this should be NW for the trait mentioned in #19?

  • First commit to issue fork.
  • last update about 1 year ago
    29,410 pass
  • πŸ‡ΊπŸ‡ΈUnited States bradjones1 Digital Nomad Life

    Marked πŸ“Œ Allow typed data plugins to receive injected dependencies Closed: duplicate as a duplicate as well. Would be great to make some traction on this by answering the questions raised in #19, or perhaps this just needs an IS update to reflect the current status after the recent MR changes.

  • Status changed to Needs review 10 months ago
  • last update 10 months ago
    30,047 pass
  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    @duadua Some great ideas there.

    Following these issues:
    πŸ“Œ Enable service autowiring by adding interface aliases to core service definitions Fixed
    πŸ“Œ Enable more service autowiring by adding interface aliases to core modules Fixed
    it is possible to infer the correct service in many cases without needing the #[Autowire] attribute. Therefore we can implement a trait that autowires based on the type where possible, or the attribute where it isn't possible, and this seems fairly trivial. I don't think we have to worry about inheritance even? Subclassed plugins will continue to provide their own create() method, parent or child classes can switch to the trait any time, as far as I can see - this might need a bit of testing though.

    Proof of concept patch attached.

  • Status changed to Needs work 10 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    If a new approach is being taken could the IS be updated?

    Will new approach still require the change record? Leaving tag just in case.

  • πŸ‡¬πŸ‡§United Kingdom joachim

    Can this be expanded to all classes that have the container factory pattern -- forms and controllers IIRC.

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    Yes, but probably not in this issue; I picked plugins first as I was getting annoyed keeping create() methods in sync with the constructor but those are valid candidates as well.

  • πŸ‡¬πŸ‡§United Kingdom joachim

    Forms have exactly the same pattern of create() and __construct(), just use Drupal\Core\DependencyInjection\ContainerInjectionInterface instead.

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    But the plugin factory method and constructors have three additional arguments - array $configuration, $plugin_id, $plugin_definition - so I don't think we can use the same code/trait.

  • πŸ‡¦πŸ‡ΊAustralia acbramley

    We have the controller issue

  • πŸ‡©πŸ‡ͺGermany donquixote

    So, the trait in the patch does the job. +1.

    Is there any benefit of doing it in ContainerFactory instead of using the trait?

  • πŸ‡©πŸ‡ͺGermany donquixote

    Actually, migrate plugins get an additional parameter passed to the ::create() method.
    In MigratePluginManager:

          $plugin = $plugin_class::create(\Drupal::getContainer(), $configuration, $plugin_id, $plugin_definition, $migration);
    

    The positional mapping with 3 parameters would lose this 4th parameter.

    But technically there is no guarantee that calling code will always pass the `$migration` parameter.
    The 4th parameter is optional, to comply with LSP.

    I think such cases need a custom trait with 4 parameters instead of 3.
    The trait can be chosen per class, according to the needs of the constructor.
    E.g. some migrate plugins might not need the `$migration`, then they can use the regular trait.

  • πŸ‡¬πŸ‡§United Kingdom joachim

    > But the plugin factory method and constructors have three additional arguments - array $configuration, $plugin_id, $plugin_definition

    Field formatter plugins have more base constructor arguments too. I know ALL about this problem from Module Builder.... :/

  • πŸ‡¦πŸ‡ΊAustralia dpi Perth, Australia

    Created ✨ Add a Autowire trait for plugins Active , as an interim DX measure for all existing (legacy) plugins.

Production build 0.69.0 2024