Problem/Motivation

SourcePluginBase declares two typed properties with no default value:

protected array $propDefinition;
protected string $propId;

setConfiguration() only assigns them conditionally:

if (isset($configuration['prop_definition'])) {
  $this->propDefinition = $configuration['prop_definition'];
}
if (isset($configuration['prop_id'])) {
  $this->propId = $configuration['prop_id'];
}

In normal usage this is never a problem, because SourcePluginManager::getSource() always builds the configuration through SourcePluginBase::buildConfiguration(), which unconditionally sets both keys (even to an empty array, when no prop/slot definition applies).

But a source plugin is also a plain Drupal plugin, and nothing stops calling it the plain way: $sourcePluginManager->createInstance($id, $configuration) with a $configuration that only has settings. That is a legitimate, standard use of a plugin manager, and it is exactly how a kernel test wiring up a source in isolation would do it. When it skips prop_definition/prop_id, any call into getPropDefinition() or getPropId() throws:

Error: Typed property Drupal\ui_patterns\SourcePluginBase::$propDefinition must not be accessed before initialization

Concretely this fires today through ComponentSource::settingsForm() → getExpectedComponentIds() → getPropDefinition(), but any source relying on getPropDefinition()/getPropId() outside the getSource() factory path hits the same wall.

The class already defends its other two settable properties this way:

protected $context = [];
protected $settings = [];

$propDefinition and $propId are set through the exact same conditional pattern in setConfiguration(), they are just missing the declared default that makes the conditional safe to skip.

Steps to reproduce

  • Get any \Drupal\ui_patterns\SourcePluginManager instance.
  • Instantiate a source plugin directly, without going through getSource()/buildConfiguration(): $sourceManager->createInstance('component', ['settings' => [...]]);
  • Call a method that reads getPropDefinition() or getPropId() (e.g. ComponentSource::settingsForm(), which calls getExpectedComponentIds()).
  • Error: Typed property ...$propDefinition must not be accessed before initialization.

Proposed resolution

Give both properties the same style of default already used for $context and $settings:

protected array $propDefinition = [];
protected string $propId = '';

setConfiguration() keeps overwriting them when the keys are present; this only changes what happens when they are absent, from a fatal error to an empty/safe default.

Remaining tasks

  • Review the MR.

User interface changes

None.

API changes

None. Both properties keep their existing type and visibility; only a default value is added.

Data model changes

None.

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:

Comments

mogtofu33 created an issue. See original summary.

just_like_good_vibes’s picture

Assigned: Unassigned » just_like_good_vibes

hello, thank you for reporting. do you think we need an additional test or is this ok?

mogtofu33’s picture

Assigned: just_like_good_vibes » mogtofu33
Status: Needs review » Needs work

I thought about a test, I think we have only DefinedContextValuesTest that cover SourcePluginBase methods alone, other are indirect with a plugin.
So probably good to have one, wil add a minimal createInstance() test.

mogtofu33’s picture

Assigned: mogtofu33 » Unassigned
Status: Needs work » Needs review
just_like_good_vibes’s picture

Status: Needs review » Reviewed & tested by the community

thanks a lot, it seems goood to go

just_like_good_vibes’s picture

Status: Reviewed & tested by the community » Fixed

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.