New SDC components are not available when enabling themes through the UI

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.

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

Updating the steps to reproduce

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:

1. 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
2. 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

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.

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.

Thanks for following up!

Heck yes! This totally fixes the issue for me. I'm attaching two videos:

1. Part 1: The video showing the issue existing
2. Part 2: The video showing patch installed, and the issue solved

Still leaving a NR, since we need a code review and I'm not super qualified for this level of PHP. But confirmed to fix the issue!

acbramley → changed the visibility of the branch 3490787-remove-exception-when to hidden.

I read the comments in the MR and everything reads well. I updated credit.

One comment about the deprecation notice

Fine to self RTBC after that change

Committed to 11.x and 11.2.x

Published change record, thanks folks

larowlan → closed merge request !12105

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