Problem/Motivation

See #2818653: Allow object-based plugin definitions to be processed in DefaultPluginManager::findDefinitions() for similar. Object base definitions cannot use PluginDependencyTrait and need to be able to.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#8 2821191-pdt-8-PASS.patch11.66 KBtim.plunkett
#8 2821191-pdt-8-FAIL.patch3.03 KBtim.plunkett
#8 2821191-pdt-8-interdiff.txt3.61 KBtim.plunkett
#5 2821191-pdt-5.patch8.05 KBtim.plunkett
#5 2821191-pdt-5-interdiff.txt6.19 KBtim.plunkett
#3 2821191-pdt-3.patch6.94 KBtim.plunkett

Comments

tim.plunkett created an issue. See original summary.

Version: 8.3.x-dev » 8.4.x-dev

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

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Version: 8.4.x-dev » 8.3.x-dev
Status: Active » Needs review
StatusFileSize
new 2821191-pdt-3.patch6.94 KB
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs review » Needs work 
+++ b/core/lib/Drupal/Component/Plugin/Definition/PluginDefinitionInterface.php
@@ -51,4 +51,24 @@ public function getClass();
+   * @see \Drupal\Core\Plugin\PluginDependencyTrait::calculatePluginDependencies()

I realize that this is just a @see and not runtime code, but nevertheless we should not be introducing knowledge about Drupal's config system to the plugin component. That is just unnecessary coupling. If we want this, it should be in a separate interface in \Drupal\Core\Config.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Needs review
StatusFileSize
new 2821191-pdt-5-interdiff.txt6.19 KB
new 2821191-pdt-5.patch8.05 KB

Honestly this is such a niche concept, IMO it's really only useful for derivative when the deriver knows things the plugin doesn't.
Otherwise, \Drupal\Component\Plugin\DependentPluginInterface::calculateDependencies() should be used instead.

Therefore, moving this to a trait and new interface.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Therefore, moving this to a trait and new interface.

This makes sense.

xjm’s picture

xjm
she/her
Primary language English
commented
Version: 8.3.x-dev » 8.4.x-dev

Note that issues like this should be targeted against 8.4.x, but can be considered for backport once they are committed to that branch. See the updated alpha release policy. Thanks!

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Category: Task » Bug report
StatusFileSize
new 2821191-pdt-8-interdiff.txt3.61 KB
new 2821191-pdt-8-FAIL.patch3.03 KB
new 2821191-pdt-8-PASS.patch11.66 KB

@effulgenstia pointed out this was missing explicit test coverage, and that it is a bug (since the code in HEAD would fatal due to no is_array() check).

The last submitted patch, 8: 2821191-pdt-8-FAIL.patch, failed testing.

effulgentsia’s picture

effulgentsia commented

Ticking credit boxes for reviewers.

  • effulgentsia committed 2fd6026 on 8.4.x 
    Issue #2821191 by tim.plunkett, tstoeckler, jibran: Allow object-based...

  • effulgentsia committed f8652e6 on 8.3.x 
    Issue #2821191 by tim.plunkett, tstoeckler, jibran: Allow object-based...
effulgentsia’s picture

effulgentsia commented
Version: 8.4.x-dev » 8.3.x-dev
Status: Reviewed & tested by the community » Fixed

Thanks! Pushed to 8.4.x. And cherry picked to 8.3.x, because this is a low-disruption bug fix.

Status: Fixed » Closed (fixed)

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