Problem/Motivation

To wire services to a form, developers must implement a custom create() method that pulls services from the container and calls the constructor.

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

In #3394870: Allow controller service wiring via constructor parameter attributes we added an AutowireTrait and applied it to ControllerBase for controllers. We can apply the same trait to FormBase for all forms to use.

Steps to reproduce

Proposed resolution

Add AutowireTrait to FormBase.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3398534

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:

About issue forks

Comments

longwave created an issue. See original summary.

sidharth_soman made their first commit to this issue’s fork.

sidharth_soman’s picture

sidharth_soman commented

Added a commit adding the trait and removing the create method.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

longwave opened merge request !14568

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
dcam’s picture

dcam commented
Status: Needs review » Reviewed & tested by the community

This looks like a simple change for some big wins in DX and the size of the PHPStan baseline. I only made notes on the MR so it's clearer why those changes were made. I had to go figure those things out, so I wanted to make certain no one else had to. LGTM.

dcam’s picture

dcam commented
Status: Reviewed & tested by the community » Needs review

Sorry for walking that back, I thought maybe this should have a change record. The pending issue for controllers does. Maybe they could be combined?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Needs work
Issue tags: +Needs change record
Related issues: +#3558306: Support automatic setter injection using the #[Required] attribute in AutowireTrait/AutowiredInstanceTrait

Thanks for the review. Yep, let's write a change record, might as well be a new one.

#3558306: Support automatic setter injection using the #[Required] attribute in AutowireTrait/AutowiredInstanceTrait will be useful here too, we can autowire the config factory with it, but that's for a followup.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Issue tags: -Needs change record

Added a CR: https://www.drupal.org/node/3575184

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
dcam’s picture

dcam commented
Status: Needs review » Reviewed & tested by the community

I reviewed the change record and change to ConfigFormBase. I don't have any additional feedback. This looks good to me.

godotislate’s picture

godotislate
he/him
commented
Status: Reviewed & tested by the community » Needs work

1 comment on the MR. Also needs merge conflict resolution.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review

Rebased, removed the constructor docblock.

dcam’s picture

dcam commented
Status: Needs review » Reviewed & tested by the community

The requested change was made. Back to RTBC.

godotislate closed merge request !14568

  • godotislate committed fd710d9e on main 
    refactor: #3398534 Allow form service wiring via constructor parameter...
godotislate’s picture

godotislate
he/him
commented
Status: Reviewed & tested by the community » Patch (to be ported)

Committed to main. Needs a new MR for 11.x, probably for the baseline. Thanks everyone!

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: main » 11.x-dev
Status: Patch (to be ported) » Needs review

Rebased on to 11.x.

longwave opened merge request !15046

dcam’s picture

dcam commented
Status: Needs review » Needs work

Looks like there are some problems with PHPStan relating to modules that were removed from main.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review

Whoops, let's try again.

dcam’s picture

dcam commented
Status: Needs review » Reviewed & tested by the community

I verified these are the same changes that went into main, rebased for 11.x with baseline changes added. The test failure seems random, but I can't re-run it. LGTM.

  • godotislate committed ef55a8fd on 11.x 
    refactor: #3398534 Allow form service wiring via constructor parameter...

godotislate closed merge request !15046

godotislate’s picture

godotislate
he/him
commented
Status: Reviewed & tested by the community » Fixed

Committed ef55a8f and 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.

Status: Fixed » Closed (fixed)

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