Right now, the way the plugin FactoryInterface is defined bounds $configuration to be an array. This sucks when you actually want to use a non-array as plugin configuration as you are bound to a method signature that does not make sense to you. As there is no way this method gets invoked in a generic manner anyway, I do think this is needlessly restrictive - so let's loosen the interface and base class to allow for non arrays also.
A concrete need for non-array based $configuration came up in #2047229: Make use of classes for entity field and data definitions, for which we'd need to convert the typed data manager createInstance() to receive a class as second parameter, which right now is an array. I'd much prefer to be able to actually fix that to a sane method signature there.
$configuration is forced to be a primitive array while many system will want to use classed objects as configuration to have dedicated methods and well defined configuration elements. Avoiding D7 array of doom syndrome.
Remove the array type hint for $configuration from all plugin (and manager) interfaces and implementations.
Patch review.
None.
All implementers and extenders of the plugin system will have to remove the array type hint from their function signatures. This will affect many modules but the change is VERY easy to make.
| Issue category | Task |
|---|---|
| Issue priority | Normal because it is only a major DX WTF. |
| Unfrozen changes | ? |
| Prioritized changes | The main goal of this issue is DX |
| Disruption | Disruptive for core/contributed and custom modules/themes because it will require widespread changes. |
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | configuration-array-2100549-12.patch | 221.44 KB | arla |
| #9 | configuration-array-2100549-9.patch | 221.48 KB | arla |
| #3 | configuration-array-2100549-3.patch | 184.33 KB | berdir |
| d8_plugin_factory.patch | 10.54 KB | fago |
Comments
Comment #1
dawehnerd8_plugin_factory.patch queued for re-testing.
Comment #3
berdirYeah, this would still be useful to have. Unfortuantely we have a lot more array $configuration type hints in core now.
We already removed it from $plugin_definition too..
Comment #4
chx commentedEdit: nevermind.
Comment #5
chx commentedEh, nevermind.
Comment #6
fagoThanks berdir for cathing all the type-hints, patch looks good except for one missing conversion.
array -> mixed
Else, this looks ready.
Comment #9
arla commentedRerolled and found more occurrences that were added since last patch.
Comment #10
berdirWe probably need a beta evaluation here, I suspect this isn't going to make it. It's not a complicated API change, but it affects tons of places.
Comment #12
arla commentedReroll too slow :/ Trying again.
Comment #13
fagoComment #15
klausiComment #17
klausiComment #31
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #32
smustgrave commentedwanted to bump this 1 more time before closing.