Problem/Motivation

The service decorator Drupal\ui_patterns\ComponentPluginManager is incorrectly implemented.
It extends Drupal\Core\Theme\ComponentPluginManager and calls parent:: methods instead of using the decorated service.
This bypasses Drupal’s service decoration mechanism, so additional decorators or alternative implementations of component_plugin_manager are ignored.

Expected Behavior

The ComponentPluginManager in ui_patterns should call the decorated service, not the parent class. 

// Wrong (current implementation):
parent::processDefinition($definition, $plugin_id);

// Correct (expected behavior):
$this->decorated->processDefinition($definition, $plugin_id);

Proposed Resolution

  • Inject the decorated component_plugin_manager service via the constructor.
  • Replace all parent:: calls with calls to $this->decorated.
<?php

use Drupal\Core\Theme\ComponentPluginManagerInterface;

class ComponentPluginManager implements ComponentPluginManagerInterface {

  protected ComponentPluginManagerInterface $decorated;

  public function __construct(ComponentPluginManagerInterface $decorated) {
    $this->decorated = $decorated;
  }

  public function processDefinition(&$definition, $plugin_id) {
    // Delegate to the decorated service.
    $this->decorated->processDefinition($definition, $plugin_id);
    // Add ui_patterns-specific logic here if needed.
  }

  // Delegate other methods likewise...
}

Impact

  • Breaks Drupal’s service decoration mechanism.
  • Prevents other modules from properly decorating component_plugin_manager.
  • Reduces extensibility and violates expected Drupal service design.

Summary

Drupal\ui_patterns\ComponentPluginManager should delegate to the decorated service, not the parent class, to correctly follow Drupal’s service decoration pattern and maintain extensibility.

Issue fork ui_patterns-3551586

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:

About issue forks

Comments

christian.wiedemann created an issue. See original summary.

pdureau’s picture

pdureau commented
Title: Service decorator Drupal\ui_patterns\ComponentPluginManager incorrectly calls parent instead of decorated service » ComponentPluginManager decorator incorrectly calls parent instead of decorated service

christian.wiedemann opened merge request !450

christian.wiedemann’s picture

christian.wiedemann commented
Assigned: christian.wiedemann » just_like_good_vibes
Status: Active » Needs review
christian.wiedemann’s picture

christian.wiedemann commented

The decoration stuff was not straight forward as a thought. Drupal Core doesn't resolve references well for that reason loading our components fails because the validation is done before the reference loading is done. So we need to ensure that our service is the first one loads the components and cache them to the decorated service. This works well with Canvas because Canvas added it logic to setCachedDefinitions.

pdureau’s picture

pdureau commented

Drupal Core doesn't resolve references well for that reason loading our components fails because the validation is done before the reference loading is done.

It will be fixed in this Core issue: #3352063: Allow schema references in Single Directory Component prop schemas

pdureau’s picture

pdureau commented

For information, related issue on Canvas side: #3552818: Contrib compatibility: ComponentPluginManager decorator should call decorated service instead of parent

pdureau’s picture

pdureau commented
Assigned: just_like_good_vibes » Unassigned
Status: Needs review » Postponed

Let's wait to see what Canvas is doing.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: Unassigned » grimreaper
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Related issues: +#3552818: Contrib compatibility: ComponentPluginManager decorator should call decorated service instead of parent
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » christian.wiedemann
Status: Postponed » Needs review

MR created on Canvas side too :)

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Issue tags: +DevDaysAthens2026
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Needs work

Thanks so much for getting this going, @grimreaper! Especially for your Canvas MR.

However, this particular service is extraordinarily tricky to correctly decorate. See https://git.drupalcode.org/project/canvas/-/merge_requests/961/diffs?com... for the changes I pushed — would appreciate your confirmation over there 🙏

And if I'm right there, then this MR will also need adjusting.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: christian.wiedemann » grimreaper

Yes.

Updating this MR regarding your latest changes on Canvas' side.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » christian.wiedemann
Status: Needs work » Needs review

Finally, not a lot of changes.

I have the remaining failing test green on my local environment...

If someone wants to test.

I have tested both UIP2 and Canvas enabled on the same site.

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented

i have tested on my local environment, and the tests are red too.
i have a fix, i will push it in the MR if you don't mind.

i want to pinpoint a potential issue about the cacheBackend.
don't we need distinct cache keys from the decorated service?
our cached definitions contain ui_patterns annotations that the decorated service may not read back as its own under 'component_plugins' cache entries.
also, the decorated service's cache backend may be already configured by its own constructor.

what about this ? 

public function setCacheBackend(CacheBackendInterface $cache_backend, $cache_key, array $cache_tags = []): void {
    parent::setCacheBackend($cache_backend, $cache_key . '.ui_patterns', $cache_tags);
  }

just_like_good_vibes opened merge request !502

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Regarding comment 16 I don't know if it is needed but I don't think it can harm current behavior, so ok if you want to add it.

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented

i did a new mr, green and everything works fine. i also did a battery l tests with and without canvas installed.
seems ok for merge. you give it a try?

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Thanks.

I gave it a try with Canvas and it was ok for me too.

I am ok to merge.

And if it will need an adjustment after Canvas' issue is merged, let's do that in a follow-up.

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented

ok let's go :)

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Assigned: christian.wiedemann » Unassigned
Status: Needs review » Reviewed & tested by the community

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
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.

dalemoore’s picture

dalemoore commented

Would this issue be the reason why, when I've been using UI Patterns on my site fine for a while, and then install Canvas, none of my components show up in Canvas? If I add the components to separate site that doesn't use UI Patterns, they all load fine in the Canvas UI. I also, when running ddev drush cr, get complaints about prop shapes.

pdureau’s picture

pdureau commented

Hi Dale,

there are actually 2 incompatibilities between UI Patterns and Canvas, and both needs to be fixed in both sides:

dalemoore’s picture

dalemoore commented

Okay thanks Pierre, will investigate in the morning!

Status: Fixed » Closed (fixed)

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