- Issue created by @deviantintegral
- πΊπΈUnited States mherchel Gainesville, FL, US
Bumping this to Major (note it may need to be critical). Most new themes will have SDCs, and this will affect anyone who install new themes via the UI.
- First commit to issue fork.
- πΊπΈUnited States mherchel Gainesville, FL, US
Slack discussion at https://drupal.slack.com/archives/C079NQPQUEN/p1746998199858839
In it, backend framework manager @larowlan states (in regards to @deviantintegral approach in the issue summary)
that approach looks fine to me, ideally with the cache clearer injected
- πΊπΈUnited States mherchel Gainesville, FL, US
Updating the steps to reproduce
- πΊπΈUnited States mherchel Gainesville, FL, US
evidently the test case isn't as simple as I initially thought. I've run into this a lot, but need to see what the differentiator is that causes it.
Here's a failing kernel test that shows a couple issues going on.
<?php declare(strict_types=1); namespace Drupal\KernelTests\Components; use Drupal\Core\Theme\Component\ComponentValidator; use Drupal\Core\Theme\Component\SchemaCompatibilityChecker; use Drupal\Core\Theme\ComponentNegotiator; use Drupal\Core\Theme\ComponentPluginManager; /** * Tests discovery of components in a theme being installed. */ class ComponentPluginManagerDiscoveryTest extends ComponentKernelTestBase { /** * {@inheritdoc} */ protected static $modules = ['sdc_test']; /** * {@inheritdoc} */ protected static $themes = ['stark']; public function testComponentDiscoveryOnThemeInstall(): void { $definitions = \Drupal::service('plugin.manager.sdc')->getDefinitions(); $this->assertArrayNotHasKey('sdc_theme_test:bar', $definitions); \Drupal::service('theme_installer')->install(['sdc_theme_test']); $definitions = \Drupal::service('plugin.manager.sdc')->getDefinitions(); $this->assertArrayHasKey('sdc_theme_test:bar', $definitions); } }
First thing is that because there is no container rebuild or component plugin cache clear on theme install,
$definitions
get cached in the first line before the theme is installed, and the cache is not invalidated after theme install, so the last line's assert fails.However, even if we just add a plugin cache clear after the theme install like this:
public function testComponentDiscoveryOnThemeInstall(): void { $definitions = \Drupal::service('plugin.manager.sdc')->clearCachedDefinitions(); $this->assertArrayNotHasKey('sdc_theme_test:bar', $definitions); \Drupal::service('theme_installer')->install(['sdc_theme_test']); $definitions = \Drupal::service('plugin.manager.sdc')->getDefinitions(); $this->assertArrayHasKey('sdc_theme_test:bar', $definitions); }
The last line still fails, because there is no container rebuild on theme install, so the component plugin manager has not changed, and the DirectoryWithMetadataPluginDiscovery
$discovery
object property in the component plugin manager does not get its$directories
property updated to include the new theme's component directory. So as long as the component plugin definitions aren't cached again before the next request, the component plugins should be updated correctly, which can be seen if with the test rewritten to pass like this:<?php declare(strict_types=1); namespace Drupal\KernelTests\Components; use Drupal\Core\Theme\Component\ComponentValidator; use Drupal\Core\Theme\Component\SchemaCompatibilityChecker; use Drupal\Core\Theme\ComponentNegotiator; use Drupal\Core\Theme\ComponentPluginManager; /** * Tests discovery of components in a theme being installed. */ class ComponentPluginManagerDiscoveryTest extends ComponentKernelTestBase { /** * {@inheritdoc} */ protected static $modules = ['sdc_test']; /** * {@inheritdoc} */ protected static $themes = ['stark']; public function testComponentDiscoveryOnThemeInstall(): void { $definitions = \Drupal::service('plugin.manager.sdc')->getDefinitions(); $this->assertArrayNotHasKey('sdc_theme_test:bar', $definitions); \Drupal::service('theme_installer')->install(['sdc_theme_test']); \Drupal::service('plugin.manager.sdc')->clearCachedDefinitions(); // Create a new component plugin manager object to simulate a new plugin // manager object instantiated on the next request. $manager = new ComponentPluginManager( \Drupal::service('module_handler'), \Drupal::service('theme_handler'), \Drupal::service('cache.discovery'), \Drupal::service('config.factory'), \Drupal::service('theme.manager'), \Drupal::service(ComponentNegotiator::class), \Drupal::service('file_system'), \Drupal::service(SchemaCompatibilityChecker::class), \Drupal::service(ComponentValidator::class), \Drupal::getContainer()->getParameter('app.root'), ); $definitions = $manager->getDefinitions(); $this->assertArrayHasKey('sdc_theme_test:bar', $definitions); } }
So I think there are two things going on:
- If the component plugins have been cached before a new theme install, installing a new theme does not clear the cache, and the theme's plugins don't get found
- Even if the plugin cache is cleared during theme install, rebuilding the component plugin cache within the same request will not pick up the new plugins because the manager's discovery directories are not rebuilt
- πΊπΈUnited States nicxvan
I think @godot is on to something. I pushed up a test case based on the original report and could not reproduce it manually or in the test.
- Merge request !12105Resolve #3522505 "Update sdc components theme install" β (Open) created by godotislate
MR with fix and test is up. I injected the component plugin manager instead of the plugin cache clearer into the theme installer, since it seemed to be overkill to clear all the plugin caches, but should be easy enough to change if needed.
godotislate β changed the visibility of the branch 3522505-new-sdc-components to hidden.