Starting with #2005716: Promote EntityType to a domain object we no longer require plugin definitions to be arrays. However, \Drupal\Component\Plugin\PluginManagerBase::processDefinition() expects them to be arrays in order to merge in defaults.

Personally, I'd empty the body of this method, remove the defaults property, and move the method to DefaultPluginManager (which is the only place that calls the method), but that proposal was met with some resistance on IRC.

Instead, we can simply check if the definition is an array and don't do anything if it isn't.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Xano’s picture

Assigned: Xano » Unassigned
Status: Active » Needs review
FileSize
626 bytes
EclipseGc’s picture

FileSize
2.47 KB

Ok, per our IRC conversation, let's try something more like this. I double checked all the grep-able instances of this use and didn't see any reason this wouldn't work. It's also not a part of the interface, so I think this is kosher.

Eclipse

neclimdul’s picture

since NestedArray isn't used anymore in the component manager we should remove the NestedArray use.

EclipseGc’s picture

And make sure it's added in DefaultPluginManager... yeah my bad.

Eclipse

Xano’s picture

FileSize
415 bytes
2.63 KB
neclimdul’s picture

This seems like the right thing to do.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me. See #2170775: Remove array typehint from $plugin_definition for the (hopefully) last of these issues.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

tstoeckler’s picture

Can someone explain why the is_array($definition) was dropped? I would have expected that to part of the if as well.

Xano’s picture

I don't know, but I also don't know if that's too bad, as it may cause silent failures. With the current solution, if you want defaults and you configure those as an array in the defaults property, and if your definition is not an array, you have done something wrong and you'll need to get an error anyway.

tstoeckler’s picture

Hmm... yeah that makes sense. Thanks!

Status: Fixed » Closed (fixed)

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