After updating drupal/facets from 3.0.3 to 3.0.4, phpstan reports the following error in our custom Facets widget:

Method Drupal\facets\Widget\WidgetPluginBase::__construct() invoked
with 3 parameters, 4 required.

It looks like a new required argument was added to the WidgetPluginBase constructor.
Since this was introduced in a patch release (3.0.3 -> 3.0.4), it creates a backwards compatibility break for existing custom or contributed widgets extending WidgetPluginBase.
A BC/deprecation layer should have been provided when changing the constructor arguments, as described in Drupal's deprecation guidelines. See https://www.drupal.org/about/core/policies/core-change-policies/how-to-d...

A similar issue was already reported for the Select2 module:
https://www.drupal.org/project/select2/issues/3618004

Steps to reproduce

  • Have a custom widget extending WidgetPluginBase using the constructor from Facets 3.0.3.
  • Update Facets to 3.0.4.
  • Run PHPStan.

Proposed resolution

Keep backwards compatibility with the previous constructor and introduce the new argument using a BC/deprecation layer.

Issue fork facets-3618467

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

ioana apetri created an issue. See original summary.

ioana apetri’s picture

ioana apetri commented
Issue summary: View changes

ioana apetri opened merge request !401

ioana apetri’s picture

ioana apetri commented
Status: Active » Needs review
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Version: 3.0.4 » 3.0.x-dev

Re targetting to the dev release because that is where the fix will be applied.

joelpittet changed the visibility of the branch 3618467-facets to hidden.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Thanks @ioana for the issue and MR. I am taking in a bit of a different direction and I want to explain myself a bit.

The constructor changes all came from #3589162: Support special hierarchy processors as exposed filters in 3.0.4 and unintentionally introduced a BC break. MR !401 uses the right deprecation pattern for a minor release, but adding new deprecations in a patch release would create another semver problem.

For 3.0.7 I’ve therefore reverted the affected constructor changes in WidgetPluginBase and LinksWidget back to their 3.0.3 versions. Those two need to move together because LinksWidget relies on the service injected by its parent.

I also checked contrib usage and confirmed these are the classes causing the live ArgumentCountError failures, including the Select2 report.

The dependency injection work is still worth keeping. MR !401 looks like the right basis to reintroduce it properly in 3.1.x, where we can add the necessary deprecation layer.

I hope this sits well with you? I feel this is the only issue I want to address before the next patch release that should get things back to a stable footing. If you're ok with this direction, I will get this in and a release out ASAP. Want to run the changes by you first though.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Fixed

I am going to merge that and we can revisit the deprecation for a minor in a separate issue. This helps unblock (in my mind) a patch release for D10

Thanks again @ioana for getting this off the ground!

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.