Issue #3464388 by vidorado, mogtofu33, smustgrave, wim leers: SDC...

Issue #3464388 by vidorado, mogtofu33, smustgrave, wim leers: SDC *.component.yml metadata is cached aggressively, gets in the way of component development

core/core.services.yml

@@ -1887,6 +1887,7 @@ services:
       - '@file_system'
       - '@Drupal\Core\Theme\Component\SchemaCompatibilityChecker'
       - '@Drupal\Core\Theme\Component\ComponentValidator'
+      - '@keyvalue'
       - '%app.root%'
   Drupal\Core\Theme\ComponentPluginManager: '@plugin.manager.sdc'
   Drupal\Core\Template\Loader\ComponentLoader:

core/lib/Drupal/Core/Theme/ComponentPluginManager.php

@@ -12,6 +12,7 @@
 use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\Extension\ThemeHandlerInterface;
 use Drupal\Core\File\FileSystemInterface;
+use Drupal\Core\KeyValueStore\KeyValueFactoryInterface;
 use Drupal\Core\Plugin\CategorizingPluginManagerTrait;
 use Drupal\Core\Plugin\DefaultPluginManager;
 use Drupal\Core\Plugin\Factory\ContainerFactory;
@@ -62,6 +63,8 @@ class ComponentPluginManager extends DefaultPluginManager implements Categorizin
    *   The compatibility checker.
    * @param \Drupal\Core\Theme\Component\ComponentValidator $componentValidator
    *   The component validator.
+   * @param \Drupal\Core\KeyValueStore\KeyValueFactoryInterface $keyValueFactory
+   *   The key value factory.
    * @param string $appRoot
    *   The application root.
    */
@@ -75,6 +78,7 @@ public function __construct(
     protected FileSystemInterface $fileSystem,
     protected SchemaCompatibilityChecker $compatibilityChecker,
     protected ComponentValidator $componentValidator,
+    protected KeyValueFactoryInterface $keyValueFactory,
     protected string $appRoot,
   ) {
     // We are skipping the call to the parent constructor to avoid initializing
@@ -124,6 +128,19 @@ public function createInstance($plugin_id, array $configuration = []): Component
     }
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getDefinitions(): array {
+    $development_settings = $this->keyValueFactory->get('development_settings');
+    $twig_debug = $development_settings->get('twig_debug', FALSE);
+    $twig_cache_disable = $development_settings->get('twig_cache_disable', FALSE);
+    if ($twig_debug || $twig_cache_disable) {
+      return $this->findDefinitions();
+    }
+    return parent::getDefinitions();
+  }
+
   /**
    * Gets a component for rendering.
    *

core/tests/Drupal/KernelTests/Components/ComponentPluginManagerTest.php

@@ -5,6 +5,7 @@
 namespace Drupal\KernelTests\Components;
 
 use Drupal\Core\Render\Component\Exception\ComponentNotFoundException;
+use Drupal\Core\Cache\CacheBackendInterface;
 
 /**
  * Tests the component plugin manager.
@@ -49,4 +50,47 @@ public function testMismatchingFolderName(): void {
     $this->manager->find('sdc_theme_test:mismatching-folder-name');
   }
 
+  /**
+   * Test component definitions caching depending on twig debug/cache settings.
+   *
+   * @param bool $twigDebug
+   *   Whether twig debug is enabled.
+   * @param bool $cacheEnabled
+   *   Whether cache is enabled.
+   * @param bool $expectCacheGet
+   *   Whether we expect the cache to be called.
+   *
+   * @dataProvider providerTestComponentCachingDependingOnDevelopmentSettings
+   */
+  public function testComponentCachingDependingOnDevelopmentSettings(bool $twigDebug, bool $cacheEnabled, bool $expectCacheGet): void {
+    // Set the development settings.
+    $developmentSettings = $this->keyValue->get('development_settings');
+    $developmentSettings->set('twig_debug', $twigDebug);
+    $developmentSettings->set('twig_cache_disable', !$cacheEnabled);
+
+    // Set the cache backend as a spy mock.
+    $cacheBackend = $this->createMock(CacheBackendInterface::class);
+    $cacheBackend->expects($expectCacheGet ? $this->once() : $this->never())
+      ->method('get')
+      ->with('cache_key');
+    $this->manager->setCacheBackend($cacheBackend, 'cache_key');
+
+    // Make two calls to getDefinitions() to ensure the
+    // cache is/isn't called if it should/shouldn't be.
+    $this->manager->getDefinitions();
+    $this->manager->getDefinitions();
+  }
+
+  /**
+   * Data provider for testComponentCachingDependingOnDevelopmentSettings().
+   */
+  public static function providerTestComponentCachingDependingOnDevelopmentSettings(): array {
+    return [
+      'Debug enabled, cache enabled' => [TRUE, TRUE, FALSE],
+      'Debug enabled, cache disabled' => [TRUE, FALSE, FALSE],
+      'Debug disabled, cache enabled' => [FALSE, TRUE, TRUE],
+      'Debug disabled, cache disabled' => [FALSE, FALSE, FALSE],
+    ];
+  }
+
 }