As stated in the documentation the $base_plugin_definition parameter is an array but this is not always true. For instance, definition for layout plugins is stored in an object (LayoutDefinition).

I would change the array to mixed or array|object since it can be both array and object.

CommentFileSizeAuthor
#21 drupal_wrong_documentation-2900883-21.patch870 byteskalyansamanta
#19 2900883-wrong-documentation-8.patch876 byteslarskhansen
#10 2900883-wrong-documentation-7.patch870 byteslarskhansen
#6 doc_fix_for_deriverinterface-2900883-6.patch863 bytesgaëlg
#3 2900883-wrong-documentation.patch725 byteslarskhansen

Comments

Chi created an issue. See original summary.

larskhansen’s picture

larskhansen commented
Issue tags: -Novice +Novice Vienna2017

I'll have a patch for this.

larskhansen’s picture

larskhansen commented
Issue summary: View changes
StatusFileSize
new 2900883-wrong-documentation.patch725 bytes
larskhansen’s picture

larskhansen commented
Status: Active » Needs review
gaëlg’s picture

gaëlg
he/him
Primary language French
Location Lille, France
commented
Issue tags: -Novice Vienna2017 +Novice, +Vienna2017

Minor change: just fixed the issue tags

gaëlg’s picture

gaëlg
he/him
Primary language French
Location Lille, France
commented
StatusFileSize
new doc_fix_for_deriverinterface-2900883-6.patch863 bytes

Thanks for the patch. The doc text still mentioned an array. I copy-pasted some part of the getDerivativeDefinition() method which explained that it can be an array or an object.

The last submitted patch, 3: 2900883-wrong-documentation.patch, failed testing. View results

chi’s picture

chi commented
Issue summary: View changes

Would this be better to use array|object instead of mixed?

larskhansen’s picture

larskhansen commented

If it only receives either an array or a object - then yes. It would be better with array|object.

larskhansen’s picture

larskhansen commented
StatusFileSize
new 2900883-wrong-documentation-7.patch870 bytes
chi’s picture

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

Looks good for me.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Needs review

Do we know which kind of object / which interface it would implement? That would be tremendously more useful than just saying "object".

chi’s picture

chi commented

As far as I know LayoutDefinition is the only non-array definition for this moment.

larskhansen’s picture

larskhansen commented

If LayoutDefinition is the only object - then let us write that.

If there are more than two objects - then let us keep the array|object.

chi’s picture

chi commented

LayoutDefinition is the only object definition in Drupal core. Since it it technically possible contributed and custom modules may also implement definitions as objects.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Version: 8.4.x-dev » 8.5.x-dev

array|\Drupal\Component\Plugin\Definition\PluginDefinitionInterface would be the correct approach.

Object-based plugin definitions MUST implement this interface.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

And LayoutDefinition is the second object-based definition.
The first was ConfigEntityType/ContentEntityType (\Drupal\Core\Entity\EntityTypeInterface)

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

larskhansen’s picture

larskhansen commented
Issue tags: -Vienna2017 +Lissabon2018, +DrupalDeveloperDays
StatusFileSize
new 2900883-wrong-documentation-8.patch876 bytes

Hi,

I'm at Drupal Developer Days in Lissabon 2018 and are giving this another go.

I've created a patch as Tim Plunkett describes.

But shouldn't getDerivativeDefinition only accept a "PluginDefinitionInterface" and not an array of "PluginDefinitionInterface"?
I think getDerivativeDefinitions should be used for an array of "PluginDefinitionInterface". Or am I way off here?

But for now - the patch follows Tims suggestion.

BR Lars Hansen

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

kalyansamanta’s picture

kalyansamanta commented
StatusFileSize
new drupal_wrong_documentation-2900883-21.patch870 bytes
joachim’s picture

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

LGTM.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 6429dc64ba to 8.7.x and c66a0b71a7 to 8.6.x. Thanks!

Backported to 8.6.x as a docs fix.

  • alexpott committed 6429dc6 on 8.7.x 
    Issue #2900883 by larskhansen, GaëlG, kalyansamanta, Chi, tim.plunkett,...

  • alexpott committed c66a0b7 on 8.6.x 
    Issue #2900883 by larskhansen, GaëlG, kalyansamanta, Chi, tim.plunkett,...

Status: Fixed » Closed (fixed)

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