Unable to uninstall base theme and subtheme via config sync at the same time

Actually noticed this bug while prepping a site for D10

@trigger_error('Calling ' . __METHOD__ . ' without the $extension_list_theme argument is deprecated in drupal:9.4.0 and will be required in drupal:10.0.0

1. Should be updated for 10.1.x and 11 now I think.

Testing out the functionality by editing a contrib theme I'm using to base theme off stark.
Before installing I exported my config
Installed theme which installed stark
Ran config import and got the error

Applied fix
Ran config import and both uninstalled.

So this looks good just needs that 1 update.

Addressed the pointer in #23.
Please review.

Issue has been addressed

+++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
@@ -448,9 +463,24 @@ protected function createExtensionChangelist() {
+    // Set the actual module weights.

Should this say theme weights?

Also is there no API method for that anywhere?

Yes it should say theme weights... it's about setting them in the array we're creating to do the ordering... so it's not setting the theme weights in the system... will fix this to be clearer. It's a copy/pasta so fixing it above too.

Leaving at RTBC as this is only fixing a comment as @catch requested.

@alexpott found 📌 Provide canonical extension sorting by dependencies in the extension system Active which covers my remaining concern here, which was apparently a remaining concern on a similar issue 9 years ago.

Committed/pushed to 10.1.x, cherry-picked to 10.0.x and 9.5.x, thanks!

I agree that we should fix this in 9.5 and 10.0 so here's a patch to remove the deprecation message as we should only be adding them in 10.1

I agree that we should fix this in 9.5 and 10.0 so here's a patch to remove the deprecation message as we should only be adding them in 10.1

Hmm fair enough, this is the tricky thing about best effort bc vs. backporting bugfixes with internal API changes. Committed/pushed to 10.0.x and 9.5.x, thanks! I don't think anyone's going to be overriding either class so hopefully this is moot.

Just a couple of micronits

1. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -166,6 +167,13 @@ class ConfigImporter {
   +  protected $themeExtensionList;
   
   +++ b/core/modules/config/src/Form/ConfigSingleImportForm.php
   @@ -104,6 +105,13 @@ class ConfigSingleImportForm extends ConfirmFormBase {
   +  protected $themeExtensionList;
   
   +++ b/core/modules/config/src/Form/ConfigSync.php
   @@ -121,6 +122,13 @@ class ConfigSync extends FormBase {
   +  protected $themeExtensionList;
   

   Let's add a property type-hint here for the D10 version.

2. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -448,9 +463,24 @@ protected function createExtensionChangelist() {
   +    // Set the actual module weights.
   

   %s/module/theme

3. +++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php
   @@ -687,6 +687,37 @@ public function testRequiredModuleValidation() {
   +   * Tests installing a base themes and sub themes during configuration import.
   

   %s/base themes/base theme

4. +++ b/core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php
   @@ -687,6 +687,37 @@ public function testRequiredModuleValidation() {
   +    $this->assertTrue($this->container->get('theme_handler')->themeExists('test_basetheme'));
   +    $this->assertTrue($this->container->get('theme_handler')->themeExists('test_subsubtheme'));
   +    $this->assertTrue($this->container->get('theme_handler')->themeExists('test_subtheme'));
   ...
   +    $this->assertFalse($this->container->get('theme_handler')->themeExists('test_basetheme'));
   +    $this->assertFalse($this->container->get('theme_handler')->themeExists('test_subsubtheme'));
   +    $this->assertFalse($this->container->get('theme_handler')->themeExists('test_subtheme'));
   

   micronit: is it worth putting the theme_handler service into a local variable ?

   would probably need to assign it twice to allow for any container rebuild as a result of updating core.extension

Ah crosspost

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

Published the change record.