Update XB to require Drupal 11.2

#3480805: CI: update to >=1.6.3 of the GitLab CI Template also pins to 11.1.x, maybe worth doing that instead as Stylelint is also failing but upgrading the template will eventually unblock that via #3492697: Update Stylelint formatter for Stylelint 16

I can reproduce the fail locally with

$ ddev xb-set-core-version 11.x-dev
$ ddev test tests/src/Kernel/Plugin/Field/FieldType/ComponentTreeItemTest.php --filter testInvalidField@valid.*props

1) Drupal\Tests\experience_builder\Kernel\Plugin\Field\FieldType\ComponentTreeItemTest::testInvalidField with data set "valid values using static props" (['{"a548b48d-58a8-4077-aa04-da9...ts"}}}', '{"dynamic-static-card2df":{"h...ue"}}}'], [])
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
-Array &0 []
+Array &0 [
+    'field_xb_demo.0' => Array &1 [
+        0 => 'The array must contain a "tree" key.',
+        1 => 'The array must contain a "props" key.',
+    ],
+]

/var/www/html/web/modules/contrib/experience_builder/tests/src/Kernel/Plugin/Field/FieldType/ComponentTreeItemTest.php:233

At a guess, this might be why:

 Drupal\experience_builder\Plugin\ComponentPluginManager:
    decorates: Drupal\Core\Theme\ComponentPluginManager
    parent: Drupal\Core\Theme\ComponentPluginManager
    arguments: ['@entity_type.manager', '@state']

Might be worth trying with #3492705: Install modules with container_rebuild_true set by themselves applied.

#3492705: Install modules with container_rebuild_true set by themselves indeed does fix it.

Why is the writing component info to state done as a decorator? Could this not be a separate service that operates on e.g. hook_rebuild()? Would mean no need for the recursion protection then.

#3492705: Install modules with container_rebuild_true set by themselves landed so this should no longer be necessary - @tedbow can you confirm?

We should still do #3480805: CI: update to >=1.6.3 of the GitLab CI Template.

Spotted this too yesterday, thanks for jumping in! 👍🙏

+1

I think we should do #3480805: CI: update to >=1.6.3 of the GitLab CI Template which will test against 11.1 and 10.4 and is currently green, then we can enable OPT_IN_TEST_NEXT_MINOR to add 11.2 but that's not critical to complete until closer to the release of 11.2.0.

Oh, even better! Looking 👀

(The "+1" was referring to "let's unblock this ASAP".)

Just reviewed #3480805 in depth, and pleased to say I was able to RTBC it! 🚀 Yay for one temporary work-around avoided, and the correct/long-term solution being ready at the same time! 🥳

#3480805: CI: update to >=1.6.3 of the GitLab CI Template is in — is there anything left to do here? 😊

Well, we should now opt in to "next minor" in our CI config and then fix the failure.

👍 — retitling.

Nice, that seems to be uncovering either not previously observed bugs or 11.2.x has changed in some way — because tests are failing with unmet config dependencies! 😬

Also:

Warning: Undefined array key "label" in /builds/project/experience_builder/web/core/lib/Drupal/Core/Field/FormatterBase.php on line 80

… this seems an upstream bug in core?

This is less urgent than many other MRs, so unassigning @longwave.

Thanks to @longwave, closed #3497866: CI: also test against next minor: 11.2.x aka Drupal 11.x HEAD as a duplicate.

@penyaskito found that FieldTypeUninstallValidatorTest fails in HEAD as well

1) Drupal\Tests\experience_builder\Kernel\FieldTypeUninstallValidatorTest::testUninstall
Drupal\Core\Config\UnmetDependenciesException: Configuration objects provided by <em class="placeholder">xb_test_config_node_article</em> have unmet dependencies: <em class="placeholder">field.field.node.article.field_xb_test (experience_builder.component.sdc.experience_builder.image, experience_builder.component.sdc.experience_builder.my-hero, experience_builder.component.sdc.experience_builder.two_column)</em>

/var/www/html/web/core/lib/Drupal/Core/Config/UnmetDependenciesException.php:100
/var/www/html/web/core/lib/Drupal/Core/Config/ConfigInstaller.php:581
/var/www/html/web/core/lib/Drupal/Core/ProxyClass/Config/ConfigInstaller.php:132
/var/www/html/web/core/lib/Drupal/Core/Extension/ModuleInstaller.php:209
/var/www/html/web/core/lib/Drupal/Core/ProxyClass/Extension/ModuleInstaller.php:83
/var/www/html/web/modules/contrib/experience_builder/tests/src/Kernel/FieldTypeUninstallValidatorTest.php:60

2) Drupal\Tests\experience_builder\Kernel\FieldTypeUninstallValidatorTest::testUninstallXbFieldMultipleEntityTypes
Drupal\Core\Config\UnmetDependenciesException: Configuration objects provided by <em class="placeholder">xb_test_config_node_article</em> have unmet dependencies: <em class="placeholder">field.field.node.article.field_xb_test (experience_builder.component.sdc.experience_builder.image, experience_builder.component.sdc.experience_builder.my-hero, experience_builder.component.sdc.experience_builder.two_column)</em>

/var/www/html/web/core/lib/Drupal/Core/Config/UnmetDependenciesException.php:100
/var/www/html/web/core/lib/Drupal/Core/Config/ConfigInstaller.php:581
/var/www/html/web/core/lib/Drupal/Core/ProxyClass/Config/ConfigInstaller.php:132
/var/www/html/web/core/lib/Drupal/Core/Extension/ModuleInstaller.php:209
/var/www/html/web/core/lib/Drupal/Core/ProxyClass/Extension/ModuleInstaller.php:83
/var/www/html/web/modules/contrib/experience_builder/tests/src/Kernel/FieldTypeUninstallValidatorTest.php:103

probably due to #3416522: Add the ability to install multiple modules and only do a single container rebuild to ModuleInstaller

Disclaimer: I usually work with 11.x HEAD, so was surprised I didn't see this before.

The problem is that if installing several modules, after experience_builder is installed no rebuild is happening, so it's not triggering the discovery of sdc components/blocks and its related config creation, and xb_test_config_node_article depends on that.

I thought we might need to flush caches on hook_install, but we are actually doing that already.
That's why the issue only happens in (some) kernel tests.

So we have two options:

1) Include the config in xb_test_config_node_article (meh 😔)
2) Document and workaround it in kernel tests (☺️)

e.g Never use

$this->container->get('module_installer')->install(['experience_builder', 'xb_test_config_node_article', 'sdc_test']);

but instead use

$this->container->get('module_installer')->install(['experience_builder', 'sdc_test']);
$this->container->get('module_installer')->install(['xb_test_config_node_article']);

container_rebuild_required = TRUE in xb.info.yml might be enough. See #3492235: Default container_rebuild_required to FALSE.

Even better, thanks!

#32 onwards is a core bug in the module installer since #3416522: Add the ability to install multiple modules and only do a single container rebuild to ModuleInstaller that means that modules that depend on XB components cannot be installed at the same time that XB itself is installed. This is because XB creates config objects on the fly, that do not exist until after XB is actually installed, but ::install() does this (simplified):

// Gather full set of modules to be installed
...

// Check the validity of the default configuration. This will throw
// exceptions if the configuration is not valid.
$config_installer->checkConfigurationToInstall('module', $module_list);

// Group modules based on container_rebuild_required flag
...

// Install modules in groups
foreach ($module_groups as $modules) {
  $this->doInstall($modules, $installed_modules, $sync_status);
}

Because we check the config dependencies of the entire set before anything is installed, the XB config entities can never exist by this point.

Locally, moving checkConfigurationToInstall() into the later loop makes the tests pass for me, so I will open a core issue to change this.

Wow, #36 is quite the find! 🤯 Thanks, @longwave!

Wrote all this up and was about to open a new issue, then found that I opened #3497173: Modules that create config cannot be installed alongside modules that depend on that config back in January and completely forgot about it!

Alternatively though: XB can ship experience_builder.component.sdc.experience_builder.* as default config to work around this, at least for the XB-provided set of components - given this will be created at install time anyway, why can't we ship with it?

I had the same question as #36 at #33, and catch responded in #34.

given this will be created at install time anyway, why can't we ship with it?

Can't really argue with that.

Won't that be a pain to maintain though? 🤔

Thanks to \Drupal\Tests\experience_builder\Functional\DefaultConfigTest it should be no pain at all; our tests would tell us. Or they should. If not, then we would need an explicit test for that.

Per #39, did we try @catch's #34?

Meanwhile: testing #40's assumptions.

Yes I tried #34, it doesn't help because container_rebuild_required is not considered until after checkConfigurationToInstall() is called, which is where the error is triggered - see the pseudocode in #36.

Not sure 30291899 is a valid test because SingleDirectoryComponent::updateConfigEntity() only updates prop_field_definitions, not labels - if we don't have a child of #3520484: [META] Production-ready ComponentSource plugins to deal with this we need one.

All green! Unsure if we want to make next minor fail CI or if we should just keep an eye on it manually; the problem is that next minor is a moving target so what passes today may not pass tomorrow and not be the fault of anything we have done, which might slow our progress.

#43++, tagging.

#44: how about we mark this CI job to allow failures? Then we can at least see where we're at. Manually triggered on MRs, automatically on commit?

PHPUnit (next minor) already allows failures. Pushed a change to disable _FOR_EVERY_MR_COMMIT for the next minor matrix so we can run them manually.

Opened #3522164: Handle update and delete of SingleDirectoryComponent `Component`s, plus missing config dependencies as a sister issue of #3484682: Handle update and delete of Block `Component`s, plus react to dependency removal.

I wanted to merge this, but:

We want only 1 of the 4 for the next minor, too.

Marked #3523759: [PP-1] Cypress tests include invalid menu config on 11.x HEAD as a duplicate of this. See how the default menu block plugin configuration changed. We'll need to port that change here most likely.

https://www.drupal.org/project/drupal/releases/11.2.0 was created ~7 hours ago while I was sleeping :)

So: soon "next minor" will become "current minor", at which point our CI will break unless we force CI back to considering 11.1 current minor.

Discussed with @effulgentsia and @lauriii. We agreed that XB 1.0.0-beta1 should require 11.2, meaning we'll drop 11.1 support.

That will simplify things that may be different between different minors, such as #3493070: SDC `enum` props should have translatable labels: use `meta:enum`.

It will also simplify getting to a green PHPStan — because in the past we've had different minors' PHPStan rules conflict, requiring awkward exotic dances 🕺 😅

Paired with @isholgueras to accelerate getting this on track, per #56.

Turns out that @longwave's #38 aka #3497173: Modules that create config cannot be installed alongside modules that depend on that config is no longer occurring, because @isholgueras and I removed all the config/install/experience_builder.component.*.yml files and tests pass 🥳

PHPUnit CI job failure is caused by a PHP warning:

    1 test triggered 1 PHP warning:
    
    1) /builds/project/experience_builder/src/ComponentSource/ComponentSourceBase.php:75
    Undefined array key "title"
    
    Triggered by:
    
    * Drupal\Tests\experience_builder\Kernel\Config\JavaScriptComponentValidationTest::testInvalidSlotIdentifiedByConfigSchema
      /builds/project/experience_builder/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php:796

AFAICT that's the only thing that still needs to be resolved? 🤞

Investigating #57.

Figured out #57. ComponentSourceBase::normalizeSlotDefinitions() assumes every slot has a title. And that's actually pretty reasonable to expect. Because it's equally crucial for usability as props having titles. So: made that a proper requirement now 👍

but also … LOL, I was wildly wrong in #57 😇 The "next minor" CI job wasn't running at all; I was looking at the current PHPUnit CI job … 🫣

It crashes very early on:

PHP Fatal error:  Uncaught Error: Interface "Drupal\TestSite\TestSetupInterface" not found in /builds/project/experience_builder/tests/src/TestSite/XBTestSetup.php:31

Just tweaked our GitLab CI to only ever test against 11.2. That provides a clearer target: all the CI jobs we're used to looking at must pass 👍

.

.

Crediting @penyaskito for https://git.drupalcode.org/project/experience_builder/-/merge_requests/4...

We're green and have PHPUnit running around the 5 min mark.
https://git.drupalcode.org/project/experience_builder/-/jobs/5771862

4 mins 11 https://git.drupalcode.org/project/experience_builder/-/jobs/5771963 😎

🥳🥳🥳

What a lovely bit of news to wake up to! 😊

First dropping Jack off at daycare, then can’t wait to review and land this one 🤓

Awesome work here, thanks everyone! 🙏

Did a complete review to make sure I 100% understand what this MR is changing and why.

Observations (for future reference for everyone, including myself):

1. The changes to XB's config schema support to match the SDC JSON schema for its "code components" were too broad. In the near future, meta:enum and x-translation-context will be added in #3516602: SDC `enum` props should have human-readable labels: use `meta:enum`. default may be added in the distant future, in #3514672: [Needs design] DX & authoring experience: support `default` in addition to `examples[0]`, enables updating a component input across all existing instances.
2. This actually includes the fix for #3520923: [PP-1] Use `json` schema type for SQLite and remove `text` workaround.
3. Bumped #3530351 to critical at #3530351-4: Decouple image+video (URI) shape matching from specific image+video file types/extensions because of the stuff this MR does to support AVIF (which makes sense to follow the latest & greatest in 11.2 👍).
4. The PHPUnit slowness appears to have been caused by a combination of factors in 11.2 which mean that @larowlan's fix for it (removing ui/node_modules 🤪) is suddenly necessary whereas it been necessary during Drupal 10, 11.0 nor 11.1 🤷‍♂️ Likely culprit: #3249443: drupal_phpunit_find_extension_directories() uses infinite recursion ⇒ more directories = slower tests, which ironically I opened and did a lot of work on 3.5 years ago 🤣
5. The pretty crazy work-around this MR adds to tests/src/TestSite/XBTestSetup.php will be removed in #3531679: Decouple kernel tests from `CanvasTestSetup`: painful to work on, and VERY slow 👍
6. The #3524738: Fatal error when passing NULL to Renderer::render() regression in Drupal core 11.2 forced the addition of temporary work-arounds. Issue created to ensure we revert those: #3534128: [upstream] Revert work-arounds for 11.2 regression #3524738.

See y'all on all other beta blockers in the next week 😊