From c5dec8309a4d0da1f7c6ddef07db57e0f0f34ea9 Mon Sep 17 00:00:00 2001 From: Matt Glaman <27012-mglaman@users.noreply.drupalcode.org> Date: Thu, 1 May 2025 14:01:06 +0000 Subject: [PATCH 01/38] Add source JS component cacheable metadata to build --- .../ExperienceBuilder/ComponentSource/JsComponent.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php b/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php index d624af72cb..6a748eae66 100644 --- a/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php +++ b/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Config\Entity\ConfigEntityStorageInterface; use Drupal\Core\Extension\ExtensionPathResolver; use Drupal\Core\File\FileUrlGeneratorInterface; @@ -166,6 +167,11 @@ final class JsComponent extends GeneratedFieldExplicitInputUxComponentSourceBase $component = $component->forAutoSavePreview($autoSave->data); } $valid_props = $component->getProps() ?? []; + + CacheableMetadata::createFromRenderArray($build) + ->addCacheableDependency($component) + ->applyTo($build); + return $build + [ '#type' => 'astro_island', '#uuid' => $componentUuid, -- GitLab From 44390760135d61c00cf7065d00e518a21fd82548 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net> Date: Thu, 1 May 2025 16:42:14 -0400 Subject: [PATCH 02/38] Add a test assertion --- .../ComponentSource/JsComponentTest.php | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index 14c9316db9..aab1ead40a 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -254,18 +254,34 @@ final class JsComponentTest extends ComponentSourceTestBase { if ($auto_save_exists) { $this->container->get(AutoSaveManager::class) ->save( - $source->getJavaScriptComponent(), + $js_component, // 'imported_js_components' is a value sent by the client that is used to // determine Javascript Code component dependencies and is not saved // directly on the backend. // @see \Drupal\experience_builder\Entity\JavaScriptComponent::addJavaScriptComponentsDependencies(). - $source->getJavaScriptComponent()->normalizeForClientSide()->values + ['imported_js_components' => []], + $js_component->normalizeForClientSide()->values + ['imported_js_components' => []], ); } $island = $source->renderComponent([ 'props' => $expected_component_props, ], 'some-uuid', $preview_requested); + + // The component's cacheable metadata should be in the render array. + $this->assertEmpty( + array_diff($js_component->getCacheTags(), $island['#cache']['tags']), + "The render array does not have all of the component's cache tags.", + ); + $this->assertEmpty( + array_diff($js_component->getCacheContexts(), $island['#cache']['contexts']), + "The render array does not have all of the component's cache contexts.", + ); + $this->assertLessThanOrEqual( + $js_component->getCacheMaxAge(), + $island['#cache']['max-age'], + "The render array's maximum cache age is greater than the component's.", + ); + $crawler = $this->crawlerForRenderArray($island); $element = $crawler->filter('astro-island'); -- GitLab From 5afce95130173b80b7b2060be3c6d7d27259d414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Thu, 8 May 2025 19:04:08 +0200 Subject: [PATCH 03/38] Fix tests after adding cacheability metadata to js_component --- .../Kernel/DataType/ComponentTreeHydratedTest.php | 12 ++++++++++-- .../ComponentTreeHydratedWithBlockOverrideTest.php | 4 ++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php b/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php index 41ecfb797b..e16ae970a3 100644 --- a/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php +++ b/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php @@ -884,7 +884,10 @@ HTML, '#component' => [ '#type' => 'astro_island', '#cache' => [ - 'tags' => ['config:experience_builder.component.js.my-cta'], + 'tags' => [ + 'config:experience_builder.js_component.my-cta', + 'config:experience_builder.component.js.my-cta', + ], 'contexts' => [], 'max-age' => Cache::PERMANENT, ], @@ -941,7 +944,10 @@ HTML, '#component' => [ '#type' => 'astro_island', '#cache' => [ - 'tags' => ['config:experience_builder.component.js.my-cta-with-auto-save'], + 'tags' => [ + 'config:experience_builder.js_component.my-cta-with-auto-save', + 'config:experience_builder.component.js.my-cta-with-auto-save', + ], 'contexts' => [], 'max-age' => Cache::PERMANENT, ], @@ -1118,7 +1124,9 @@ HTML, 'expected_cache_tags' => [ 'config:experience_builder.component.sdc.xb_test_sdc.props-slots', 'config:experience_builder.component.sdc.xb_test_sdc.props-no-slots', + 'config:experience_builder.js_component.my-cta-with-auto-save', 'config:experience_builder.component.js.my-cta-with-auto-save', + 'config:experience_builder.js_component.my-cta', 'config:experience_builder.component.js.my-cta', 'config:system.site', 'config:experience_builder.component.block.system_branding_block', diff --git a/tests/src/Kernel/DataType/ComponentTreeHydratedWithBlockOverrideTest.php b/tests/src/Kernel/DataType/ComponentTreeHydratedWithBlockOverrideTest.php index 860c373581..43c905471b 100644 --- a/tests/src/Kernel/DataType/ComponentTreeHydratedWithBlockOverrideTest.php +++ b/tests/src/Kernel/DataType/ComponentTreeHydratedWithBlockOverrideTest.php @@ -241,7 +241,9 @@ HTML, 'expected_cache_tags' => [ 'config:experience_builder.component.sdc.xb_test_sdc.props-slots', 'config:experience_builder.component.sdc.xb_test_sdc.props-no-slots', + 'config:experience_builder.js_component.my-cta-with-auto-save', 'config:experience_builder.component.js.my-cta-with-auto-save', + 'config:experience_builder.js_component.my-cta', 'config:experience_builder.component.js.my-cta', 'config:experience_builder.js_component.site_branding', 'config:system.site', @@ -347,7 +349,9 @@ HTML, 'expected_cache_tags' => [ 'config:experience_builder.component.sdc.xb_test_sdc.props-slots', 'config:experience_builder.component.sdc.xb_test_sdc.props-no-slots', + 'config:experience_builder.js_component.my-cta-with-auto-save', 'config:experience_builder.component.js.my-cta-with-auto-save', + 'config:experience_builder.js_component.my-cta', 'config:experience_builder.component.js.my-cta', 'config:experience_builder.js_component.site_branding', 'config:system.site', -- GitLab From 9694e0ee9b99be8544ecbe8796f6ba78c9072e0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Fri, 9 May 2025 11:34:01 +0200 Subject: [PATCH 04/38] Fix tests for JsComponent cacheability --- .../ComponentSource/JsComponentTest.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index aab1ead40a..9f992ba4dd 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -192,17 +192,22 @@ final class JsComponentTest extends ComponentSourceTestBase { 'theme', 'user.permissions', ]; - $default_cacheability = (new CacheableMetadata()) - ->setCacheContexts($default_render_cache_contexts); + $this->assertEquals([ 'js.xb_test_code_components_vanilla_image' => [ - 'cacheability' => $default_cacheability, + 'cacheability' => (new CacheableMetadata()) + ->setCacheContexts($default_render_cache_contexts) + ->setCacheTags(['config:experience_builder.js_component.xb_test_code_components_vanilla_image']), ], 'js.xb_test_code_components_with_no_props' => [ - 'cacheability' => $default_cacheability, + 'cacheability' => (new CacheableMetadata()) + ->setCacheContexts($default_render_cache_contexts) + ->setCacheTags(['config:experience_builder.js_component.xb_test_code_components_with_no_props']), ], 'js.xb_test_code_components_with_props' => [ - 'cacheability' => $default_cacheability, + 'cacheability' => (new CacheableMetadata()) + ->setCacheContexts($default_render_cache_contexts) + ->setCacheTags(['config:experience_builder.js_component.xb_test_code_components_with_props']), ], ], $rendered_without_html); } -- GitLab From 236b90acac09c6d1dd24ba1062f4eb62021b13d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Fri, 9 May 2025 12:26:50 +0200 Subject: [PATCH 05/38] Test the whole cacheability array for JsComponents --- .../ComponentSource/JsComponentTest.php | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index 9f992ba4dd..2ab9f5c3e4 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -11,6 +11,7 @@ use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\NestedArray; use Drupal\Core\Asset\AssetResolverInterface; use Drupal\Core\Asset\AttachedAssets; +use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\File\FileUrlGeneratorInterface; use Drupal\Core\Site\Settings; @@ -286,6 +287,7 @@ final class JsComponentTest extends ComponentSourceTestBase { $island['#cache']['max-age'], "The render array's maximum cache age is greater than the component's.", ); + $this->assertEquals(self::expectedJsComponentCacheabilityArray($js_component_id), $island['#cache']); $crawler = $this->crawlerForRenderArray($island); @@ -347,6 +349,43 @@ final class JsComponentTest extends ComponentSourceTestBase { } } + /** + * Return the cacheability array for a given js component id. + * + * @param string $js_component_id + * The component id. + * + * @return array + */ + public static function expectedJsComponentCacheabilityArray(string $js_component_id): array { + $expected_cacheability = [ + 'xb_test_code_components_vanilla_image' => [ + 'max-age' => Cache::PERMANENT, + 'tags' => [ + 'config:experience_builder.js_component.xb_test_code_components_vanilla_image', + ], + 'contexts' => [], + ], + 'xb_test_code_components_with_no_props' => [ + 'max-age' => Cache::PERMANENT, + 'tags' => [ + 'config:experience_builder.js_component.xb_test_code_components_with_no_props', + ], + 'contexts' => [], + ], + 'xb_test_code_components_with_props' => [ + 'max-age' => Cache::PERMANENT, + 'tags' => [ + 'config:experience_builder.js_component.xb_test_code_components_with_props', + ], + 'contexts' => [], + ], + ]; + + return $expected_cacheability[$js_component_id]; + + } + /** * @covers ::calculateDependencies() * @depends testDiscovery -- GitLab From 2c2b759df669ee5d784c4f6014d57a0ae4a447f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Fri, 9 May 2025 13:33:46 +0200 Subject: [PATCH 06/38] Fix test for js_component.my-cta cache tags. --- tests/src/Functional/PropSourceEndpointTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/src/Functional/PropSourceEndpointTest.php b/tests/src/Functional/PropSourceEndpointTest.php index 9cdc6a55ef..3ae1264ff3 100644 --- a/tests/src/Functional/PropSourceEndpointTest.php +++ b/tests/src/Functional/PropSourceEndpointTest.php @@ -60,6 +60,7 @@ class PropSourceEndpointTest extends FunctionalTestBase { 'comment_list', 'config:component_list', 'config:core.extension', + 'config:experience_builder.js_component.my-cta', 'config:search.settings', 'config:system.menu.account', 'config:system.menu.admin', -- GitLab From 8a543bc379995eb132e8e17edfac02a0336893e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Fri, 9 May 2025 14:07:46 +0200 Subject: [PATCH 07/38] Fix cache tags for XbConfigEntityHttpApiTest after adding cace tags to js_components. --- tests/src/Functional/XbConfigEntityHttpApiTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/src/Functional/XbConfigEntityHttpApiTest.php b/tests/src/Functional/XbConfigEntityHttpApiTest.php index b669546410..843559b0d9 100644 --- a/tests/src/Functional/XbConfigEntityHttpApiTest.php +++ b/tests/src/Functional/XbConfigEntityHttpApiTest.php @@ -1111,6 +1111,7 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase { 'CACHE_MISS_IF_UNCACHEABLE_HTTP_METHOD:form', 'config:component_list', 'config:core.extension', + 'config:experience_builder.js_component.test', 'config:node_type_list', 'config:system.menu.account', 'config:system.menu.admin', -- GitLab From 399944228a591ea60997480932d811223cb1024a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Mon, 12 May 2025 13:39:25 +0200 Subject: [PATCH 08/38] Fix tests for additional cache tags if new components are present. --- .../Functional/XbConfigEntityHttpApiTest.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/src/Functional/XbConfigEntityHttpApiTest.php b/tests/src/Functional/XbConfigEntityHttpApiTest.php index 843559b0d9..5279297501 100644 --- a/tests/src/Functional/XbConfigEntityHttpApiTest.php +++ b/tests/src/Functional/XbConfigEntityHttpApiTest.php @@ -873,8 +873,8 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase { // @see docs/config-management.md#3.2.1 $this->assertNotNull(Component::load('js.test')); $this->assertTrue(Component::load('js.test')->status()); - $this->assertExposedCodeComponents(['js.test'], 'MISS', $request_options); - $this->assertExposedCodeComponents(['js.test'], 'HIT', $request_options); + $this->assertExposedCodeComponents(['js.test'], 'MISS', $request_options, ['config:experience_builder.js_component.test']); + $this->assertExposedCodeComponents(['js.test'], 'HIT', $request_options, ['config:experience_builder.js_component.test']); // Confirm that there STILL is an auto-save, and its `status` was updated! $auto_save_data['status'] = TRUE; $this->assertCurrentAutoSave(200, $auto_save_data, JavaScriptComponent::ENTITY_TYPE_ID, 'test'); @@ -1090,7 +1090,7 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase { ], json_decode((string) $response->getBody(), TRUE)); } - private function assertExposedCodeComponents(array $expected, string $expected_dynamic_page_cache, array $request_options): void { + private function assertExposedCodeComponents(array $expected, string $expected_dynamic_page_cache, array $request_options, array $additional_expected_cache_tags = []): void { assert(in_array($expected_dynamic_page_cache, ['HIT', 'MISS'], TRUE)); $expected_contexts = [ 'languages:language_content', @@ -1107,11 +1107,10 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase { // @see \Drupal\experience_builder\Controller\ApiComponentsController::getCacheableClientSideInfo() 'user.roles:anonymous', ]; - $body = $this->assertExpectedResponse('GET', Url::fromUri('base:/xb/api/v0/config/component'), $request_options, 200, $expected_contexts, [ + $expected_cache_tags = [ 'CACHE_MISS_IF_UNCACHEABLE_HTTP_METHOD:form', 'config:component_list', 'config:core.extension', - 'config:experience_builder.js_component.test', 'config:node_type_list', 'config:system.menu.account', 'config:system.menu.admin', @@ -1128,7 +1127,13 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase { 'user:1', 'user:2', 'user_list', - ], 'UNCACHEABLE (request policy)', $expected_dynamic_page_cache); + ]; + // If expected adds new components, those components add additional cache tags. If those cache tags are not + // present, the test will fail. This array is used to add those additional expected cache tags. + if (!empty($additional_expected_cache_tags)) { + $expected_cache_tags = array_merge($expected_cache_tags, $additional_expected_cache_tags); + } + $body = $this->assertExpectedResponse('GET', Url::fromUri('base:/xb/api/v0/config/component'), $request_options, 200, $expected_contexts, $expected_cache_tags, 'UNCACHEABLE (request policy)', $expected_dynamic_page_cache); self:self::assertNotNull($body); $component_config_entity_ids = array_keys($body); self::assertSame( -- GitLab From 59e81c2144642f9ef2350d5b813a404143f39b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Mon, 12 May 2025 13:44:47 +0200 Subject: [PATCH 09/38] Remove tests that are already covered by new tests --- .../ComponentSource/JsComponentTest.php | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index 2ab9f5c3e4..b860255152 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -273,20 +273,6 @@ final class JsComponentTest extends ComponentSourceTestBase { 'props' => $expected_component_props, ], 'some-uuid', $preview_requested); - // The component's cacheable metadata should be in the render array. - $this->assertEmpty( - array_diff($js_component->getCacheTags(), $island['#cache']['tags']), - "The render array does not have all of the component's cache tags.", - ); - $this->assertEmpty( - array_diff($js_component->getCacheContexts(), $island['#cache']['contexts']), - "The render array does not have all of the component's cache contexts.", - ); - $this->assertLessThanOrEqual( - $js_component->getCacheMaxAge(), - $island['#cache']['max-age'], - "The render array's maximum cache age is greater than the component's.", - ); $this->assertEquals(self::expectedJsComponentCacheabilityArray($js_component_id), $island['#cache']); $crawler = $this->crawlerForRenderArray($island); -- GitLab From e8a9ae56e0fae09d3a3d5fc14bd02cd34a0f80eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Mon, 12 May 2025 15:57:44 +0200 Subject: [PATCH 10/38] Test AutoSave cache tags for JsComponents. --- .../ExperienceBuilder/ComponentSource/JsComponent.php | 3 +++ .../ExperienceBuilder/ComponentSource/JsComponentTest.php | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php b/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php index 6a748eae66..f37bab2d79 100644 --- a/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php +++ b/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php @@ -165,6 +165,9 @@ final class JsComponent extends GeneratedFieldExplicitInputUxComponentSourceBase if ($isPreview && !$autoSave->isEmpty()) { \assert($autoSave->data !== NULL); $component = $component->forAutoSavePreview($autoSave->data); + CacheableMetadata::createFromRenderArray($build) + ->addCacheTags([AutoSaveManager::CACHE_TAG]) + ->applyTo($build); } $valid_props = $component->getProps() ?? []; diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index b860255152..d32362c673 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -273,7 +273,11 @@ final class JsComponentTest extends ComponentSourceTestBase { 'props' => $expected_component_props, ], 'some-uuid', $preview_requested); - $this->assertEquals(self::expectedJsComponentCacheabilityArray($js_component_id), $island['#cache']); + $expected_cache = self::expectedJsComponentCacheabilityArray($js_component_id); + if ($preview_requested && $auto_save_exists) { + $expected_cache['tags'][] = AutoSaveManager::CACHE_TAG; + } + $this->assertEqualsCanonicalizing($expected_cache, $island['#cache']); $crawler = $this->crawlerForRenderArray($island); -- GitLab From 575bc16aaf4c60c8f926392b64bc08d2a70bad46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Mon, 12 May 2025 16:43:03 +0200 Subject: [PATCH 11/38] Fix auto-save cache tags tests in the ComponentTreeHydratedTest --- .../DataType/ComponentTreeHydratedTest.php | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php b/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php index e16ae970a3..c1cca2bc9f 100644 --- a/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php +++ b/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php @@ -1148,8 +1148,26 @@ HTML, ...self::overwriteRenderableExpectations( self::setIsPreviewPropertyRecursively($component_tree_with_complex_nesting, TRUE), [ - ['parents' => [...$path_to_auto_saved_js_component, '#name'], 'value' => 'My Code Component with Auto-Save - Draft'], - ['parents' => [...$path_to_auto_saved_js_component, '#component_url'], 'value' => '/xb/api/v0/auto-saves/js/js_component/my-cta-with-auto-save'], + [ + 'parents' => [...$path_to_auto_saved_js_component, '#name'], + 'value' => 'My Code Component with Auto-Save - Draft', + ], + [ + 'parents' => [...$path_to_auto_saved_js_component, '#component_url'], + 'value' => '/xb/api/v0/auto-saves/js/js_component/my-cta-with-auto-save', + ], + [ + 'parents' => [...$path_to_auto_saved_js_component, '#cache'], + 'value' => [ + 'tags' => [ + 'experience_builder__auto_save', + 'config:experience_builder.js_component.my-cta-with-auto-save', + 'config:experience_builder.component.js.my-cta-with-auto-save', + ], + 'contexts' => [], + 'max-age' => Cache::PERMANENT, + ], + ], ], ), 'expected_html' => <<<HTML @@ -1216,6 +1234,16 @@ HTML, <!-- xb-end-uuid-in-root --> HTML, 'isPreview' => TRUE, + 'expected_cache_tags' => [ + 'config:experience_builder.component.sdc.xb_test_sdc.props-slots', + 'config:experience_builder.component.sdc.xb_test_sdc.props-no-slots', + 'experience_builder__auto_save', + 'config:experience_builder.js_component.my-cta-with-auto-save', + 'config:experience_builder.component.js.my-cta-with-auto-save', + 'config:experience_builder.js_component.my-cta', + 'config:experience_builder.component.js.my-cta', + 'config:experience_builder.component.block.system_branding_block', + ], ]; } -- GitLab From d766b774af7f68cf95840d5a7bbc548302365962 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Mon, 12 May 2025 16:45:25 +0200 Subject: [PATCH 12/38] Fix Auto-save cache tag for ComponentTreeHydratedWithBlockOverrideTest --- .../DataType/ComponentTreeHydratedWithBlockOverrideTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/src/Kernel/DataType/ComponentTreeHydratedWithBlockOverrideTest.php b/tests/src/Kernel/DataType/ComponentTreeHydratedWithBlockOverrideTest.php index 43c905471b..f01f1d216a 100644 --- a/tests/src/Kernel/DataType/ComponentTreeHydratedWithBlockOverrideTest.php +++ b/tests/src/Kernel/DataType/ComponentTreeHydratedWithBlockOverrideTest.php @@ -349,6 +349,7 @@ HTML, 'expected_cache_tags' => [ 'config:experience_builder.component.sdc.xb_test_sdc.props-slots', 'config:experience_builder.component.sdc.xb_test_sdc.props-no-slots', + 'experience_builder__auto_save', 'config:experience_builder.js_component.my-cta-with-auto-save', 'config:experience_builder.component.js.my-cta-with-auto-save', 'config:experience_builder.js_component.my-cta', -- GitLab From 1df355ecd73517623707f3aee7798b354af2890d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Mon, 12 May 2025 18:28:23 +0200 Subject: [PATCH 13/38] Issue #3522217: Add auto_save cache tags to fix tests --- tests/src/Functional/XbConfigEntityHttpApiTest.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/src/Functional/XbConfigEntityHttpApiTest.php b/tests/src/Functional/XbConfigEntityHttpApiTest.php index 5279297501..75afabc46f 100644 --- a/tests/src/Functional/XbConfigEntityHttpApiTest.php +++ b/tests/src/Functional/XbConfigEntityHttpApiTest.php @@ -873,8 +873,14 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase { // @see docs/config-management.md#3.2.1 $this->assertNotNull(Component::load('js.test')); $this->assertTrue(Component::load('js.test')->status()); - $this->assertExposedCodeComponents(['js.test'], 'MISS', $request_options, ['config:experience_builder.js_component.test']); - $this->assertExposedCodeComponents(['js.test'], 'HIT', $request_options, ['config:experience_builder.js_component.test']); + $this->assertExposedCodeComponents(['js.test'], 'MISS', $request_options, [ + 'experience_builder__auto_save', + 'config:experience_builder.js_component.test', + ]); + $this->assertExposedCodeComponents(['js.test'], 'HIT', $request_options, [ + 'experience_builder__auto_save', + 'config:experience_builder.js_component.test', + ]); // Confirm that there STILL is an auto-save, and its `status` was updated! $auto_save_data['status'] = TRUE; $this->assertCurrentAutoSave(200, $auto_save_data, JavaScriptComponent::ENTITY_TYPE_ID, 'test'); -- GitLab From c61aa3d3edba0868e8d84f2360dd895b50b92d16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Tue, 13 May 2025 14:18:19 +0200 Subject: [PATCH 14/38] Issue #3522217: Improve a bit the visibility of the expectedCacheabilityArrayForJsComponentRender method --- .../ComponentSource/JsComponent.php | 7 ++--- .../ComponentSource/JsComponentTest.php | 27 ++++++++----------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php b/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php index f37bab2d79..92ce9e4290 100644 --- a/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php +++ b/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php @@ -165,10 +165,11 @@ final class JsComponent extends GeneratedFieldExplicitInputUxComponentSourceBase if ($isPreview && !$autoSave->isEmpty()) { \assert($autoSave->data !== NULL); $component = $component->forAutoSavePreview($autoSave->data); - CacheableMetadata::createFromRenderArray($build) - ->addCacheTags([AutoSaveManager::CACHE_TAG]) - ->applyTo($build); } + if ($isPreview) { + $build['#cache']['tags'][] = AutoSaveManager::CACHE_TAG; + } + $valid_props = $component->getProps() ?? []; CacheableMetadata::createFromRenderArray($build) diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index d32362c673..26e4208303 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -224,11 +224,12 @@ final class JsComponentTest extends ComponentSourceTestBase { */ public function testRenderJsComponent(bool $preview_requested, bool $auto_save_exists, string $expected_result, array $component_ids): void { $this->generateComponentConfig(); - foreach ($this->componentStorage->loadMultiple($component_ids) as $component) { + $expected_cache = self::expectedCacheabilityArrayForJsComponentRender(); + foreach ($this->componentStorage->loadMultiple($component_ids) as $component_id => $component) { assert($component instanceof Component); $source = $component->getComponentSource(); \assert($source instanceof JsComponent); - $this->assertRenderedAstroIsland($component, $preview_requested, $auto_save_exists, $expected_result); + $this->assertRenderedAstroIsland($component, $preview_requested, $auto_save_exists, $expected_result, $expected_cache[$component_id]); } } @@ -247,6 +248,7 @@ final class JsComponentTest extends ComponentSourceTestBase { bool $preview_requested, bool $auto_save_exists, string $expected_result, + array $expected_cache, ): void { $source = $component->getComponentSource(); \assert($source instanceof JsComponent); @@ -273,8 +275,7 @@ final class JsComponentTest extends ComponentSourceTestBase { 'props' => $expected_component_props, ], 'some-uuid', $preview_requested); - $expected_cache = self::expectedJsComponentCacheabilityArray($js_component_id); - if ($preview_requested && $auto_save_exists) { + if ($preview_requested) { $expected_cache['tags'][] = AutoSaveManager::CACHE_TAG; } $this->assertEqualsCanonicalizing($expected_cache, $island['#cache']); @@ -340,30 +341,27 @@ final class JsComponentTest extends ComponentSourceTestBase { } /** - * Return the cacheability array for a given js component id. - * - * @param string $js_component_id - * The component id. + * Return the cacheability array for all the available JsComponents. * * @return array */ - public static function expectedJsComponentCacheabilityArray(string $js_component_id): array { - $expected_cacheability = [ - 'xb_test_code_components_vanilla_image' => [ + public static function expectedCacheabilityArrayForJsComponentRender(): array { + return [ + 'js.xb_test_code_components_vanilla_image' => [ 'max-age' => Cache::PERMANENT, 'tags' => [ 'config:experience_builder.js_component.xb_test_code_components_vanilla_image', ], 'contexts' => [], ], - 'xb_test_code_components_with_no_props' => [ + 'js.xb_test_code_components_with_no_props' => [ 'max-age' => Cache::PERMANENT, 'tags' => [ 'config:experience_builder.js_component.xb_test_code_components_with_no_props', ], 'contexts' => [], ], - 'xb_test_code_components_with_props' => [ + 'js.xb_test_code_components_with_props' => [ 'max-age' => Cache::PERMANENT, 'tags' => [ 'config:experience_builder.js_component.xb_test_code_components_with_props', @@ -371,9 +369,6 @@ final class JsComponentTest extends ComponentSourceTestBase { 'contexts' => [], ], ]; - - return $expected_cacheability[$js_component_id]; - } /** -- GitLab From b4b3681cef76d7f7407ea190653f9ca467c2c325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Tue, 13 May 2025 14:18:55 +0200 Subject: [PATCH 15/38] Issue #3522217: Fix ComponentTreeHydrated tests --- .../DataType/ComponentTreeHydratedTest.php | 30 +++++++++++++++++-- ...onentTreeHydratedWithBlockOverrideTest.php | 4 ++- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php b/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php index c1cca2bc9f..6bca3fb7e7 100644 --- a/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php +++ b/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php @@ -11,6 +11,7 @@ use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Render\Markup; use Drupal\Core\Render\RendererInterface; use Drupal\Core\TypedData\TypedDataManagerInterface; +use Drupal\experience_builder\AutoSave\AutoSaveManager; use Drupal\experience_builder\Element\RenderSafeComponentContainer; use Drupal\experience_builder\Entity\Page; use Drupal\experience_builder\Plugin\DataType\ComponentTreeStructure; @@ -1133,6 +1134,7 @@ HTML, ], ]; yield 'component tree with complex nesting' => [...$component_tree_with_complex_nesting, 'isPreview' => FALSE]; + $path_to_auto_saved_js_component = [ ComponentTreeStructure::ROOT_UUID, 'uuid-in-root', @@ -1144,6 +1146,17 @@ HTML, 'uuid-js-component-auto-save', '#component', ]; + $path_to_js_component = [ + ComponentTreeStructure::ROOT_UUID, + 'uuid-in-root', + '#component', '#slots', 'the_body', + 'uuid-level-1', + '#component', '#slots', 'the_body', + 'uuid-level-2', + '#component', '#slots', 'the_body', + 'uuid-js-component', + '#component', + ]; yield 'component tree with complex nesting in preview' => [ ...self::overwriteRenderableExpectations( self::setIsPreviewPropertyRecursively($component_tree_with_complex_nesting, TRUE), @@ -1160,7 +1173,7 @@ HTML, 'parents' => [...$path_to_auto_saved_js_component, '#cache'], 'value' => [ 'tags' => [ - 'experience_builder__auto_save', + AutoSaveManager::CACHE_TAG, 'config:experience_builder.js_component.my-cta-with-auto-save', 'config:experience_builder.component.js.my-cta-with-auto-save', ], @@ -1168,6 +1181,18 @@ HTML, 'max-age' => Cache::PERMANENT, ], ], + [ + 'parents' => [...$path_to_js_component, '#cache'], + 'value' => [ + 'tags' => [ + AutoSaveManager::CACHE_TAG, + 'config:experience_builder.js_component.my-cta', + 'config:experience_builder.component.js.my-cta', + ], + 'contexts' => [], + 'max-age' => Cache::PERMANENT, + ], + ], ], ), 'expected_html' => <<<HTML @@ -1237,7 +1262,7 @@ HTML, 'expected_cache_tags' => [ 'config:experience_builder.component.sdc.xb_test_sdc.props-slots', 'config:experience_builder.component.sdc.xb_test_sdc.props-no-slots', - 'experience_builder__auto_save', + AutoSaveManager::CACHE_TAG, 'config:experience_builder.js_component.my-cta-with-auto-save', 'config:experience_builder.component.js.my-cta-with-auto-save', 'config:experience_builder.js_component.my-cta', @@ -1245,6 +1270,7 @@ HTML, 'config:experience_builder.component.block.system_branding_block', ], ]; + } /** diff --git a/tests/src/Kernel/DataType/ComponentTreeHydratedWithBlockOverrideTest.php b/tests/src/Kernel/DataType/ComponentTreeHydratedWithBlockOverrideTest.php index f01f1d216a..257934df6b 100644 --- a/tests/src/Kernel/DataType/ComponentTreeHydratedWithBlockOverrideTest.php +++ b/tests/src/Kernel/DataType/ComponentTreeHydratedWithBlockOverrideTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\experience_builder\Kernel\DataType; // cspell:ignore ttxgk xpzur use Drupal\Core\Render\Element; +use Drupal\experience_builder\AutoSave\AutoSaveManager; use Drupal\experience_builder\Entity\JavaScriptComponent; use Drupal\experience_builder\Plugin\DataType\ComponentTreeStructure; use Drupal\Component\Utility\NestedArray; @@ -140,6 +141,7 @@ HTML, 'config:experience_builder.js_component.site_branding', 'config:system.site', 'config:experience_builder.component.block.system_branding_block', + AutoSaveManager::CACHE_TAG, ], ] + $original_test_case, @@ -349,7 +351,7 @@ HTML, 'expected_cache_tags' => [ 'config:experience_builder.component.sdc.xb_test_sdc.props-slots', 'config:experience_builder.component.sdc.xb_test_sdc.props-no-slots', - 'experience_builder__auto_save', + AutoSaveManager::CACHE_TAG, 'config:experience_builder.js_component.my-cta-with-auto-save', 'config:experience_builder.component.js.my-cta-with-auto-save', 'config:experience_builder.js_component.my-cta', -- GitLab From b449eb170839527e7d1d3f8c8e792f2784c6450b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Tue, 13 May 2025 14:25:18 +0200 Subject: [PATCH 16/38] Issue #3522217: Simplify cacheability tests in jsComponents --- .../ComponentSource/JsComponentTest.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index 26e4208303..f1085ba09b 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -194,20 +194,20 @@ final class JsComponentTest extends ComponentSourceTestBase { 'user.permissions', ]; + $default_cacheability = (new CacheableMetadata()) + ->setCacheContexts($default_render_cache_contexts); + $this->assertEquals([ 'js.xb_test_code_components_vanilla_image' => [ - 'cacheability' => (new CacheableMetadata()) - ->setCacheContexts($default_render_cache_contexts) + 'cacheability' => (clone $default_cacheability) ->setCacheTags(['config:experience_builder.js_component.xb_test_code_components_vanilla_image']), ], 'js.xb_test_code_components_with_no_props' => [ - 'cacheability' => (new CacheableMetadata()) - ->setCacheContexts($default_render_cache_contexts) + 'cacheability' => (clone $default_cacheability) ->setCacheTags(['config:experience_builder.js_component.xb_test_code_components_with_no_props']), ], 'js.xb_test_code_components_with_props' => [ - 'cacheability' => (new CacheableMetadata()) - ->setCacheContexts($default_render_cache_contexts) + 'cacheability' => (clone $default_cacheability) ->setCacheTags(['config:experience_builder.js_component.xb_test_code_components_with_props']), ], ], $rendered_without_html); -- GitLab From 6204466b87026cd4e1eba7fcd1ccf657e361b0f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Tue, 13 May 2025 16:03:00 +0200 Subject: [PATCH 17/38] Issue #3522217: Fix tests --- tests/src/Functional/PropSourceEndpointTest.php | 2 ++ tests/src/Kernel/DataType/ComponentTreeHydratedTest.php | 1 + 2 files changed, 3 insertions(+) diff --git a/tests/src/Functional/PropSourceEndpointTest.php b/tests/src/Functional/PropSourceEndpointTest.php index 3ae1264ff3..f9ac34dd7e 100644 --- a/tests/src/Functional/PropSourceEndpointTest.php +++ b/tests/src/Functional/PropSourceEndpointTest.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Drupal\Tests\experience_builder\Functional; use Drupal\Component\Serialization\Json; +use Drupal\experience_builder\AutoSave\AutoSaveManager; use Drupal\experience_builder\Entity\Component; use Drupal\node\Entity\Node; use Drupal\Tests\system\Functional\Cache\AssertPageCacheContextsAndTagsTrait; @@ -78,6 +79,7 @@ class PropSourceEndpointTest extends FunctionalTestBase { 'user:0', 'user:1', 'user_list', + AutoSaveManager::CACHE_TAG, ]; $expected_contexts = [ diff --git a/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php b/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php index 6bca3fb7e7..390d0ff7f7 100644 --- a/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php +++ b/tests/src/Kernel/DataType/ComponentTreeHydratedTest.php @@ -1267,6 +1267,7 @@ HTML, 'config:experience_builder.component.js.my-cta-with-auto-save', 'config:experience_builder.js_component.my-cta', 'config:experience_builder.component.js.my-cta', + 'config:system.site', 'config:experience_builder.component.block.system_branding_block', ], ]; -- GitLab From 3553bb99ed8de4df20e75e2213491e68a852b5ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Tue, 13 May 2025 16:20:23 +0200 Subject: [PATCH 18/38] Issue #3522217: Use constant instead of literal string for autosave cache tag. --- tests/src/Functional/XbConfigEntityHttpApiTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/Functional/XbConfigEntityHttpApiTest.php b/tests/src/Functional/XbConfigEntityHttpApiTest.php index 75afabc46f..dd55af7971 100644 --- a/tests/src/Functional/XbConfigEntityHttpApiTest.php +++ b/tests/src/Functional/XbConfigEntityHttpApiTest.php @@ -874,11 +874,11 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase { $this->assertNotNull(Component::load('js.test')); $this->assertTrue(Component::load('js.test')->status()); $this->assertExposedCodeComponents(['js.test'], 'MISS', $request_options, [ - 'experience_builder__auto_save', + AutoSaveManager::CACHE_TAG, 'config:experience_builder.js_component.test', ]); $this->assertExposedCodeComponents(['js.test'], 'HIT', $request_options, [ - 'experience_builder__auto_save', + AutoSaveManager::CACHE_TAG, 'config:experience_builder.js_component.test', ]); // Confirm that there STILL is an auto-save, and its `status` was updated! -- GitLab From 02fe0950b438a4c1300a7a7aca3d4b228487b788 Mon Sep 17 00:00:00 2001 From: Wim Leers <wim.leers@acquia.com> Date: Tue, 13 May 2025 18:03:51 +0200 Subject: [PATCH 19/38] Address my feedback at https://git.drupalcode.org/project/experience_builder/-/merge_requests/972#note_514876. --- .../ComponentSource/JsComponentTest.php | 55 ++++--------------- 1 file changed, 12 insertions(+), 43 deletions(-) diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index f1085ba09b..6061638863 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -12,6 +12,7 @@ use Drupal\Component\Utility\NestedArray; use Drupal\Core\Asset\AssetResolverInterface; use Drupal\Core\Asset\AttachedAssets; use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\File\FileUrlGeneratorInterface; use Drupal\Core\Site\Settings; @@ -217,19 +218,21 @@ final class JsComponentTest extends ComponentSourceTestBase { * For JavaScript components, auto-saves create an extra testing dimension! * * @depends testDiscovery - * @testWith [false, false, "live"] - * [false, true, "live"] - * [true, false, "live"] - * [true, true, "draft"] + * @testWith [false, false, "live", []] + * [false, true, "live", []] + * [true, false, "live", ["experience_builder__auto_save"]] + * [true, true, "draft", ["experience_builder__auto_save"]] */ - public function testRenderJsComponent(bool $preview_requested, bool $auto_save_exists, string $expected_result, array $component_ids): void { + public function testRenderJsComponent(bool $preview_requested, bool $auto_save_exists, string $expected_result, array $additional_expected_cache_tags, array $component_ids): void { $this->generateComponentConfig(); - $expected_cache = self::expectedCacheabilityArrayForJsComponentRender(); foreach ($this->componentStorage->loadMultiple($component_ids) as $component_id => $component) { assert($component instanceof Component); $source = $component->getComponentSource(); \assert($source instanceof JsComponent); - $this->assertRenderedAstroIsland($component, $preview_requested, $auto_save_exists, $expected_result, $expected_cache[$component_id]); + $expected_cacheability = (new CacheableMetadata()) + ->addCacheTags($additional_expected_cache_tags) + ->addCacheableDependency($source->getJavaScriptComponent()); + $this->assertRenderedAstroIsland($component, $preview_requested, $auto_save_exists, $expected_result, $expected_cacheability); } } @@ -248,7 +251,7 @@ final class JsComponentTest extends ComponentSourceTestBase { bool $preview_requested, bool $auto_save_exists, string $expected_result, - array $expected_cache, + CacheableDependencyInterface $expected_cacheability, ): void { $source = $component->getComponentSource(); \assert($source instanceof JsComponent); @@ -275,10 +278,7 @@ final class JsComponentTest extends ComponentSourceTestBase { 'props' => $expected_component_props, ], 'some-uuid', $preview_requested); - if ($preview_requested) { - $expected_cache['tags'][] = AutoSaveManager::CACHE_TAG; - } - $this->assertEqualsCanonicalizing($expected_cache, $island['#cache']); + $this->assertEquals($expected_cacheability, CacheableMetadata::createFromRenderArray($island)); $crawler = $this->crawlerForRenderArray($island); @@ -340,37 +340,6 @@ final class JsComponentTest extends ComponentSourceTestBase { } } - /** - * Return the cacheability array for all the available JsComponents. - * - * @return array - */ - public static function expectedCacheabilityArrayForJsComponentRender(): array { - return [ - 'js.xb_test_code_components_vanilla_image' => [ - 'max-age' => Cache::PERMANENT, - 'tags' => [ - 'config:experience_builder.js_component.xb_test_code_components_vanilla_image', - ], - 'contexts' => [], - ], - 'js.xb_test_code_components_with_no_props' => [ - 'max-age' => Cache::PERMANENT, - 'tags' => [ - 'config:experience_builder.js_component.xb_test_code_components_with_no_props', - ], - 'contexts' => [], - ], - 'js.xb_test_code_components_with_props' => [ - 'max-age' => Cache::PERMANENT, - 'tags' => [ - 'config:experience_builder.js_component.xb_test_code_components_with_props', - ], - 'contexts' => [], - ], - ]; - } - /** * @covers ::calculateDependencies() * @depends testDiscovery -- GitLab From 614d076a2091cd5c449ebbe6418206a79ad3eed9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Tue, 13 May 2025 18:41:53 +0200 Subject: [PATCH 20/38] Issue #3522217: fix phpcs --- .../ExperienceBuilder/ComponentSource/JsComponentTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index 6061638863..05154993ae 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -11,7 +11,6 @@ use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\NestedArray; use Drupal\Core\Asset\AssetResolverInterface; use Drupal\Core\Asset\AttachedAssets; -use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\File\FileUrlGeneratorInterface; @@ -225,7 +224,7 @@ final class JsComponentTest extends ComponentSourceTestBase { */ public function testRenderJsComponent(bool $preview_requested, bool $auto_save_exists, string $expected_result, array $additional_expected_cache_tags, array $component_ids): void { $this->generateComponentConfig(); - foreach ($this->componentStorage->loadMultiple($component_ids) as $component_id => $component) { + foreach ($this->componentStorage->loadMultiple($component_ids) as $component) { assert($component instanceof Component); $source = $component->getComponentSource(); \assert($source instanceof JsComponent); -- GitLab From 23eb1ea538079a2f896b42e9878c355f84e342ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Wed, 14 May 2025 13:56:33 +0200 Subject: [PATCH 21/38] Issue #3522217: Update getCacheTags fro JavaScriptComponent to add the dependencies. --- src/Entity/JavaScriptComponent.php | 10 ++++++++++ tests/src/Functional/XbConfigEntityHttpApiTest.php | 2 ++ 2 files changed, 12 insertions(+) diff --git a/src/Entity/JavaScriptComponent.php b/src/Entity/JavaScriptComponent.php index e9d72dbd1d..2b1169ef75 100644 --- a/src/Entity/JavaScriptComponent.php +++ b/src/Entity/JavaScriptComponent.php @@ -60,6 +60,8 @@ final class JavaScriptComponent extends ConfigEntityBase implements XbAssetInter public const string ENTITY_TYPE_ID = 'js_component'; public const string ADMIN_PERMISSION = 'administer code components'; + + // @phpstan-ignore-next-line private const string ASSETS_DIRECTORY = 'assets://astro-island/'; /** @@ -437,4 +439,12 @@ final class JavaScriptComponent extends ConfigEntityBase implements XbAssetInter return self::loadMultiple($js_component_ids); } + public function getCacheTags() { + $cache_tags = parent::getCacheTags(); + if ($dependencies = $this->getDependencies()) { + $cache_tags = array_merge($cache_tags, array_map(fn($dependency) => "config:$dependency", $dependencies['config'] ?? [])); + } + return $cache_tags; + } + } diff --git a/tests/src/Functional/XbConfigEntityHttpApiTest.php b/tests/src/Functional/XbConfigEntityHttpApiTest.php index dd55af7971..4822d7f89c 100644 --- a/tests/src/Functional/XbConfigEntityHttpApiTest.php +++ b/tests/src/Functional/XbConfigEntityHttpApiTest.php @@ -875,10 +875,12 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase { $this->assertTrue(Component::load('js.test')->status()); $this->assertExposedCodeComponents(['js.test'], 'MISS', $request_options, [ AutoSaveManager::CACHE_TAG, + 'config:experience_builder.js_component.another_component', 'config:experience_builder.js_component.test', ]); $this->assertExposedCodeComponents(['js.test'], 'HIT', $request_options, [ AutoSaveManager::CACHE_TAG, + 'config:experience_builder.js_component.another_component', 'config:experience_builder.js_component.test', ]); // Confirm that there STILL is an auto-save, and its `status` was updated! -- GitLab From 56dd61c660ba79b772cece759d170d79b4b24f59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Wed, 14 May 2025 15:56:21 +0200 Subject: [PATCH 22/38] Issue #3522217: Remove unnecessary phpstan ignore. --- src/Entity/JavaScriptComponent.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Entity/JavaScriptComponent.php b/src/Entity/JavaScriptComponent.php index 2b1169ef75..819ea8d975 100644 --- a/src/Entity/JavaScriptComponent.php +++ b/src/Entity/JavaScriptComponent.php @@ -60,8 +60,6 @@ final class JavaScriptComponent extends ConfigEntityBase implements XbAssetInter public const string ENTITY_TYPE_ID = 'js_component'; public const string ADMIN_PERMISSION = 'administer code components'; - - // @phpstan-ignore-next-line private const string ASSETS_DIRECTORY = 'assets://astro-island/'; /** -- GitLab From 975ee2ae54dafe9fadcf057130cb02f819260cca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Wed, 14 May 2025 16:13:00 +0200 Subject: [PATCH 23/38] Issue #3522217: add test coverage to getCacheTags to JavaScriptComponent. --- tests/src/Kernel/Config/JavascriptComponentTest.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/src/Kernel/Config/JavascriptComponentTest.php b/tests/src/Kernel/Config/JavascriptComponentTest.php index b5628ec268..02e6af00ec 100644 --- a/tests/src/Kernel/Config/JavascriptComponentTest.php +++ b/tests/src/Kernel/Config/JavascriptComponentTest.php @@ -40,6 +40,9 @@ class JavascriptComponentTest extends KernelTestBase { $js_component = JavaScriptComponent::createFromClientSide($client_data); $this->assertSame(SAVED_NEW, $js_component->save()); $this->assertCount(0, $js_component->getDependencies()); + $this->assertSame([ + 'config:experience_builder.js_component.test', + ], $js_component->getCacheTags()); // Create another component that will be imported by the first one. $client_data_2 = $client_data; @@ -48,6 +51,9 @@ class JavascriptComponentTest extends KernelTestBase { $js_component2 = JavaScriptComponent::createFromClientSide($client_data_2); $this->assertSame(SAVED_NEW, $js_component2->save()); $this->assertCount(0, $js_component2->getDependencies()); + $this->assertSame([ + 'config:experience_builder.js_component.test2', + ], $js_component2->getCacheTags()); // Adding a component to `imported_js_components` should add this component // to the dependencies. @@ -60,6 +66,10 @@ class JavascriptComponentTest extends KernelTestBase { ], $js_component->getDependencies() ); + $this->assertSame([ + 'config:experience_builder.js_component.test', + 'config:experience_builder.js_component.test2', + ], $js_component->getCacheTags()); // Ensure missing components are will throw a validation error. $client_data['imported_js_components'] = [$js_component2->id(), 'missing']; @@ -99,6 +109,9 @@ class JavascriptComponentTest extends KernelTestBase { $js_component->updateFromClientSide($client_data); $this->assertSame(SAVED_UPDATED, $js_component->save()); $this->assertSame([], $js_component->getDependencies()); + $this->assertSame([ + 'config:experience_builder.js_component.test', + ], $js_component->getCacheTags()); } } -- GitLab From 87f7f6058b81d58611a775ccee9412724de35d04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Wed, 14 May 2025 17:29:32 +0200 Subject: [PATCH 24/38] Issue #3522217: Assert entities are valid after importing new js components. --- src/Entity/JavaScriptComponent.php | 22 +++++++-- .../JavaScriptComponentValidationTest.php | 45 +++++++++++++++++-- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/src/Entity/JavaScriptComponent.php b/src/Entity/JavaScriptComponent.php index 819ea8d975..326508d03f 100644 --- a/src/Entity/JavaScriptComponent.php +++ b/src/Entity/JavaScriptComponent.php @@ -168,9 +168,9 @@ final class JavaScriptComponent extends ConfigEntityBase implements XbAssetInter ]); } + $violation_list = new EntityConstraintViolationList($this); if (array_key_exists('source_code_js', $data) || array_key_exists('compiled_js', $data)) { if (!array_key_exists('imported_js_components', $data)) { - $violation_list = new EntityConstraintViolationList($this); $violation_list->add(new ConstraintViolation( "The 'imported_js_components' field is required when 'source_code_js' or 'compiled_js' is provided", "The 'imported_js_components' field is required when 'source_code_js' or 'compiled_js' is provided", @@ -181,6 +181,22 @@ final class JavaScriptComponent extends ConfigEntityBase implements XbAssetInter )); throw new ConstraintViolationException($violation_list); } + foreach ($data['imported_js_components'] as $key => $js_component_name) { + // Test that the imported_js_components are valid names. + if (!preg_match('/^[a-z0-9_-]+$/', $js_component_name)) { + $violation_list->add(new ConstraintViolation( + "The 'imported_js_components' contains an invalid component name.", + "The 'imported_js_components' contains an invalid component name.", + [], + NULL, + "imported_js_components", + NULL + )); + } + } + if ($violation_list->count() > 0) { + throw new ConstraintViolationException($violation_list); + } // The client calculates imported JavaScript components dependencies. This // value is never returned to the client as it will always recalculate it // based off source_code_js. @@ -358,8 +374,8 @@ final class JavaScriptComponent extends ConfigEntityBase implements XbAssetInter $js_component = JavaScriptComponent::load($js_component_name); if (!$js_component) { $violation_list->add(new ConstraintViolation( - "The JavaScript component with machine name '$js_component_name' does not exist.", - "The JavaScript component with machine name '$js_component_name' does not exist.", + "The JavaScript component with the machine name '$js_component_name' does not exist.", + "The JavaScript component with the machine name '$js_component_name' does not exist.", [], NULL, "imported_js_components.$key", diff --git a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php index 71cb9fd3bd..acfa3e382a 100644 --- a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php +++ b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php @@ -7,6 +7,8 @@ namespace Drupal\Tests\experience_builder\Kernel\Config; // cspell:ignore sofie use Drupal\experience_builder\Entity\JavaScriptComponent; +use Drupal\experience_builder\Exception\ConstraintViolationException; +use Drupal\Tests\experience_builder\Traits\BetterConfigDependencyManagerTrait; /** * Tests validation of JavaScriptComponent entities. @@ -17,6 +19,8 @@ use Drupal\experience_builder\Entity\JavaScriptComponent; */ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTestBase { + use BetterConfigDependencyManagerTrait; + /** * {@inheritdoc} */ @@ -56,9 +60,7 @@ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTest */ protected function setUp(): void { parent::setUp(); - - $this->entity = JavaScriptComponent::create([ - 'machineName' => 'test', + $javascript_component_base = [ 'name' => 'Test', 'status' => TRUE, 'props' => [ @@ -86,8 +88,11 @@ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTest 'original' => '.test { display: none; }', 'compiled' => '.test{display:none;}', ], - ]); + ]; + $this->entity = JavaScriptComponent::create([...$javascript_component_base, 'machineName' => 'test']); $this->entity->save(); + $this->other_entity = JavaScriptComponent::create([...$javascript_component_base, 'machineName' => 'test2']); + $this->other_entity->save(); } /** @@ -164,6 +169,38 @@ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTest $this->assertValidationErrors($expected_validation_errors); } + /** + * @testWith ["unexisting", "The JavaScript component with the machine name 'unexisting' does not exist."] + * ["", "The 'imported_js_components' contains an invalid component name."] + * ["🚀", "The 'imported_js_components' contains an invalid component name."] + */ + public function testUnexistingJsDependencies(string $component_id, string $expected_exception_message): void { + $this->expectException(ConstraintViolationException::class); + $this->expectExceptionMessage($expected_exception_message); + + $client_values = $this->entity->normalizeForClientSide()->values; + $client_values['imported_js_components'] = [$component_id]; + $this->entity->updateFromClientSide($client_values); + } + + public function testEntityIsValid(): void { + $client_values = $this->entity->normalizeForClientSide()->values; + $client_values['imported_js_components'] = [$this->other_entity->id()]; + $this->entity->updateFromClientSide($client_values); + $this->entity->save(); + $this->assertSame( + [ + 'config' => [ + 'experience_builder.js_component.test2', + ], + 'module' => [ + 'experience_builder', + ], + ], + $this->getAllDependencies($this->entity) + ); + } + public static function providerInvalidEnumsAndExamples(): array { return [ 'Invalid string' => [ -- GitLab From 9880573c29303475ef071eebf49f61c4c8cda0ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Wed, 14 May 2025 17:47:53 +0200 Subject: [PATCH 25/38] Issue #3522217: fix text --- tests/src/Functional/XbConfigEntityHttpApiTest.php | 2 +- tests/src/Kernel/Config/JavascriptComponentTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/Functional/XbConfigEntityHttpApiTest.php b/tests/src/Functional/XbConfigEntityHttpApiTest.php index 4822d7f89c..e96b9b07b9 100644 --- a/tests/src/Functional/XbConfigEntityHttpApiTest.php +++ b/tests/src/Functional/XbConfigEntityHttpApiTest.php @@ -583,7 +583,7 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase { $this->assertSame([ 'errors' => [ [ - 'detail' => "The JavaScript component with machine name 'missing' does not exist.", + 'detail' => "The JavaScript component with the machine name 'missing' does not exist.", 'source' => ['pointer' => 'imported_js_components.0'], ], ], diff --git a/tests/src/Kernel/Config/JavascriptComponentTest.php b/tests/src/Kernel/Config/JavascriptComponentTest.php index 02e6af00ec..9618b5af23 100644 --- a/tests/src/Kernel/Config/JavascriptComponentTest.php +++ b/tests/src/Kernel/Config/JavascriptComponentTest.php @@ -84,7 +84,7 @@ class JavascriptComponentTest extends KernelTestBase { $this->assertCount(1, $violations); $violation = $violations->get(0); $this->assertSame('imported_js_components.1', $violation->getPropertyPath()); - $this->assertSame("The JavaScript component with machine name 'missing' does not exist.", $violation->getMessage()); + $this->assertSame("The JavaScript component with the machine name 'missing' does not exist.", $violation->getMessage()); } // Ensure not sending `imported_js_components` will throw an error. -- GitLab From 8fabffcc17a6f96084598c09d0bcf64f6f0ea3fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Wed, 14 May 2025 17:59:55 +0200 Subject: [PATCH 26/38] Issue #3522217: fix phpcs and phpstan errors. --- .../src/Kernel/Config/JavaScriptComponentValidationTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php index acfa3e382a..814252be02 100644 --- a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php +++ b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php @@ -21,6 +21,8 @@ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTest use BetterConfigDependencyManagerTrait; + protected JavaScriptComponent $other_entity; + /** * {@inheritdoc} */ @@ -175,15 +177,18 @@ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTest * ["🚀", "The 'imported_js_components' contains an invalid component name."] */ public function testUnexistingJsDependencies(string $component_id, string $expected_exception_message): void { + \assert($this->entity instanceof JavaScriptComponent); $this->expectException(ConstraintViolationException::class); $this->expectExceptionMessage($expected_exception_message); + \assert($this->entity instanceof JavaScriptComponent); $client_values = $this->entity->normalizeForClientSide()->values; $client_values['imported_js_components'] = [$component_id]; $this->entity->updateFromClientSide($client_values); } public function testEntityIsValid(): void { + \assert($this->entity instanceof JavaScriptComponent); $client_values = $this->entity->normalizeForClientSide()->values; $client_values['imported_js_components'] = [$this->other_entity->id()]; $this->entity->updateFromClientSide($client_values); -- GitLab From f81d5fe54276fb8a8642086899f7ada8df8c2960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Wed, 14 May 2025 18:16:40 +0200 Subject: [PATCH 27/38] Issue #3522217: Fix phpcs with a correct word --- tests/src/Kernel/Config/JavaScriptComponentValidationTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php index 814252be02..62604b8df7 100644 --- a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php +++ b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php @@ -172,11 +172,11 @@ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTest } /** - * @testWith ["unexisting", "The JavaScript component with the machine name 'unexisting' does not exist."] + * @testWith ["missing", "The JavaScript component with the machine name 'missing' does not exist."] * ["", "The 'imported_js_components' contains an invalid component name."] * ["🚀", "The 'imported_js_components' contains an invalid component name."] */ - public function testUnexistingJsDependencies(string $component_id, string $expected_exception_message): void { + public function testNonExistingJsDependencies(string $component_id, string $expected_exception_message): void { \assert($this->entity instanceof JavaScriptComponent); $this->expectException(ConstraintViolationException::class); $this->expectExceptionMessage($expected_exception_message); -- GitLab From 7f40daf3f2183645992b4481036cb574d7993a8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Fri, 16 May 2025 11:23:14 +0200 Subject: [PATCH 28/38] Issue #3522217: add extra non-ascii components. --- tests/src/Kernel/Config/JavaScriptComponentValidationTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php index 62604b8df7..6c023617a6 100644 --- a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php +++ b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php @@ -175,6 +175,8 @@ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTest * @testWith ["missing", "The JavaScript component with the machine name 'missing' does not exist."] * ["", "The 'imported_js_components' contains an invalid component name."] * ["🚀", "The 'imported_js_components' contains an invalid component name."] + * ["componente_extraño", "The 'imported_js_components' contains an invalid component name."] + * [";", "The 'imported_js_components' contains an invalid component name."] */ public function testNonExistingJsDependencies(string $component_id, string $expected_exception_message): void { \assert($this->entity instanceof JavaScriptComponent); -- GitLab From 4c658f35e67c562de19e8f1aac5382e756ccc01f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Fri, 16 May 2025 11:23:53 +0200 Subject: [PATCH 29/38] Issue #3522217: Test component that imports 2 extra components. --- ....xb_test_code_components_using_imports.yml | 14 +++ ....xb_test_code_components_using_imports.yml | 88 +++++++++++++++++++ .../ComponentSource/JsComponentTest.php | 14 ++- 3 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 tests/modules/xb_test_code_components/config/install/experience_builder.component.js.xb_test_code_components_using_imports.yml create mode 100644 tests/modules/xb_test_code_components/config/install/experience_builder.js_component.xb_test_code_components_using_imports.yml diff --git a/tests/modules/xb_test_code_components/config/install/experience_builder.component.js.xb_test_code_components_using_imports.yml b/tests/modules/xb_test_code_components/config/install/experience_builder.component.js.xb_test_code_components_using_imports.yml new file mode 100644 index 0000000000..185f06f147 --- /dev/null +++ b/tests/modules/xb_test_code_components/config/install/experience_builder.component.js.xb_test_code_components_using_imports.yml @@ -0,0 +1,14 @@ +uuid: dd1e8922-118a-4ff7-b83c-bc8543efdc5a +langcode: en +status: true +dependencies: + config: + - experience_builder.js_component.xb_test_code_components_using_imports +label: 'Component using imports' +id: js.xb_test_code_components_using_imports +provider: null +source: js +category: '@todo' +settings: + local_source_id: xb_test_code_components_using_imports + prop_field_definitions: { } diff --git a/tests/modules/xb_test_code_components/config/install/experience_builder.js_component.xb_test_code_components_using_imports.yml b/tests/modules/xb_test_code_components/config/install/experience_builder.js_component.xb_test_code_components_using_imports.yml new file mode 100644 index 0000000000..66c15c7aa9 --- /dev/null +++ b/tests/modules/xb_test_code_components/config/install/experience_builder.js_component.xb_test_code_components_using_imports.yml @@ -0,0 +1,88 @@ +uuid: 82d79d93-c4b8-4413-9b76-f89ffe432bc1 +langcode: en +status: true +dependencies: + enforced: + config: + - experience_builder.js_component.xb_test_code_components_with_no_props + - experience_builder.js_component.xb_test_code_components_with_props +machineName: xb_test_code_components_using_imports +name: 'Component using imports' +block_override: null +required: { } +props: { } +slots: { } +js: + original: | + import WithNoProps from '@/components/xb_test_code_components_with_no_props'; + import WithProps from '@/components/xb_test_code_components_with_props'; + import FormattedText from "@/lib/FormattedText"; + import { cn } from "@/lib/utils"; + + const UsingImports = ({ + title = "<h3>Component using imports</h3>", + }) => { + return ( + <div className="test-component-using-specific-imports"> + <h2>This component imports specific components</h2> + <div className="imported-components"> + <div className="no-props-component"> + <h3>Component with no props:</h3> + <WithNoProps /> + </div> + <div className="props-component"> + <h3>Component with props:</h3> + <WithProps name="Imported Name" age={25} /> + </div> + </div> + </div> + ); + }; + + export default UsingImports; + compiled: | + import { jsx as _jsx, jsxs as _jsxs } from "react/jsx-runtime"; + import WithNoProps from '@/components/xb_test_code_components_with_no_props'; + import WithProps from '@/components/xb_test_code_components_with_props'; + import FormattedText from "@/lib/FormattedText"; + import { cn } from "@/lib/utils"; + const UsingImports = ({ title = "<h3>Component using imports</h3>" })=>{ + return /*#__PURE__*/ _jsxs("div", { + className: "test-component-using-specific-imports", + children: [ + /*#__PURE__*/ _jsx("h2", { + children: "This component imports specific components" + }), + /*#__PURE__*/ _jsxs("div", { + className: "imported-components", + children: [ + /*#__PURE__*/ _jsxs("div", { + className: "no-props-component", + children: [ + /*#__PURE__*/ _jsx("h3", { + children: "Component with no props:" + }), + /*#__PURE__*/ _jsx(WithNoProps, {}) + ] + }), + /*#__PURE__*/ _jsxs("div", { + className: "props-component", + children: [ + /*#__PURE__*/ _jsx("h3", { + children: "Component with props:" + }), + /*#__PURE__*/ _jsx(WithProps, { + name: "Imported Name", + age: 25 + }) + ] + }) + ] + }) + ] + }); + }; + export default UsingImports; +css: + original: '' + compiled: '' diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index 05154993ae..7e951f57c4 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -124,6 +124,10 @@ final class JsComponentTest extends ComponentSourceTestBase { public static function getExpectedSettings(): array { return [ + 'js.xb_test_code_components_using_imports' => [ + 'local_source_id' => 'xb_test_code_components_using_imports', + 'prop_field_definitions' => [], + ], 'js.xb_test_code_components_vanilla_image' => [ 'local_source_id' => 'xb_test_code_components_vanilla_image', 'prop_field_definitions' => [ @@ -184,7 +188,7 @@ final class JsComponentTest extends ComponentSourceTestBase { // rendering. // @see ::testRenderComponent() $rendered_without_html = array_map( - fn ($expectations) => array_diff_key($expectations, ['html' => NULL]), + fn($expectations) => array_diff_key($expectations, ['html' => NULL]), $rendered, ); @@ -198,6 +202,14 @@ final class JsComponentTest extends ComponentSourceTestBase { ->setCacheContexts($default_render_cache_contexts); $this->assertEquals([ + 'js.xb_test_code_components_using_imports' => [ + 'cacheability' => (clone $default_cacheability) + ->setCacheTags([ + 'config:experience_builder.js_component.xb_test_code_components_using_imports', + 'config:experience_builder.js_component.xb_test_code_components_with_no_props', + 'config:experience_builder.js_component.xb_test_code_components_with_props', + ]), + ], 'js.xb_test_code_components_vanilla_image' => [ 'cacheability' => (clone $default_cacheability) ->setCacheTags(['config:experience_builder.js_component.xb_test_code_components_vanilla_image']), -- GitLab From 056aaa2b64365ce7d013131fa3da3da58f03b156 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez=20Holgueras?= <18324-isholgueras@users.noreply.drupalcode.org> Date: Fri, 16 May 2025 09:26:59 +0000 Subject: [PATCH 30/38] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Wim Leers <44946-wimleers@users.noreply.drupalcode.org> --- tests/src/Functional/XbConfigEntityHttpApiTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/Functional/XbConfigEntityHttpApiTest.php b/tests/src/Functional/XbConfigEntityHttpApiTest.php index e96b9b07b9..ca91e12c1c 100644 --- a/tests/src/Functional/XbConfigEntityHttpApiTest.php +++ b/tests/src/Functional/XbConfigEntityHttpApiTest.php @@ -1139,7 +1139,7 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase { // If expected adds new components, those components add additional cache tags. If those cache tags are not // present, the test will fail. This array is used to add those additional expected cache tags. if (!empty($additional_expected_cache_tags)) { - $expected_cache_tags = array_merge($expected_cache_tags, $additional_expected_cache_tags); + $expected_cache_tags = Cache::mergeTags($expected_cache_tags, $additional_expected_cache_tags); } $body = $this->assertExpectedResponse('GET', Url::fromUri('base:/xb/api/v0/config/component'), $request_options, 200, $expected_contexts, $expected_cache_tags, 'UNCACHEABLE (request policy)', $expected_dynamic_page_cache); self:self::assertNotNull($body); -- GitLab From 5f992fd9ae003ddfb0bbb05197ab3e575dc05674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Fri, 16 May 2025 11:29:18 +0200 Subject: [PATCH 31/38] Issue #3522217: remove unnecessary if. --- tests/src/Functional/XbConfigEntityHttpApiTest.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/src/Functional/XbConfigEntityHttpApiTest.php b/tests/src/Functional/XbConfigEntityHttpApiTest.php index ca91e12c1c..dde73fb6f6 100644 --- a/tests/src/Functional/XbConfigEntityHttpApiTest.php +++ b/tests/src/Functional/XbConfigEntityHttpApiTest.php @@ -4,9 +4,10 @@ declare(strict_types=1); namespace Drupal\Tests\experience_builder\Functional; +use Drupal\Core\Cache\Cache; +use Drupal\Core\Url; use Drupal\experience_builder\Audit\ComponentAudit; use Drupal\experience_builder\AutoSave\AutoSaveManager; -use Drupal\Core\Url; use Drupal\experience_builder\Entity\AssetLibrary; use Drupal\experience_builder\Entity\Component; use Drupal\experience_builder\Entity\ComponentInterface; @@ -1138,9 +1139,7 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase { ]; // If expected adds new components, those components add additional cache tags. If those cache tags are not // present, the test will fail. This array is used to add those additional expected cache tags. - if (!empty($additional_expected_cache_tags)) { - $expected_cache_tags = Cache::mergeTags($expected_cache_tags, $additional_expected_cache_tags); - } + $expected_cache_tags = Cache::mergeTags($expected_cache_tags, $additional_expected_cache_tags); $body = $this->assertExpectedResponse('GET', Url::fromUri('base:/xb/api/v0/config/component'), $request_options, 200, $expected_contexts, $expected_cache_tags, 'UNCACHEABLE (request policy)', $expected_dynamic_page_cache); self:self::assertNotNull($body); $component_config_entity_ids = array_keys($body); -- GitLab From 4e840565f6c12efe439674aac6d6e965cb606856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Fri, 16 May 2025 11:41:52 +0200 Subject: [PATCH 32/38] Issue #3522217: Fix cspell --- tests/src/Kernel/Config/JavaScriptComponentValidationTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php index 6c023617a6..181ea117da 100644 --- a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php +++ b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Drupal\Tests\experience_builder\Kernel\Config; -// cspell:ignore sofie +// cspell:ignore sofie componente extraño use Drupal\experience_builder\Entity\JavaScriptComponent; use Drupal\experience_builder\Exception\ConstraintViolationException; -- GitLab From 8f174525e950549d5158ca5f7168a2eb74000858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Fri, 16 May 2025 12:35:41 +0200 Subject: [PATCH 33/38] Issue #3522217: Add imported dependencies to JsComponent. --- .../ComponentSource/JsComponent.php | 6 +++++- .../ComponentSource/JsComponentTest.php | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php b/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php index 92ce9e4290..ac03a706a6 100644 --- a/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php +++ b/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php @@ -88,7 +88,11 @@ final class JsComponent extends GeneratedFieldExplicitInputUxComponentSourceBase public function calculateDependencies(): array { $dependencies = parent::calculateDependencies(); // @todo Add the global asset library in https://www.drupal.org/project/experience_builder/issues/3499933. - $dependencies['config'][] = $this->getJavaScriptComponent()->getConfigDependencyName(); + $dependencies['config'] = array_merge( + $dependencies['config'] ?? [], + [$this->getJavaScriptComponent()->getConfigDependencyName()], + $this->getJavaScriptComponent()->getDependencies()['config'] ?? [] + ); return $dependencies; } diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index 7e951f57c4..da2eefb198 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -357,6 +357,13 @@ final class JsComponentTest extends ComponentSourceTestBase { */ public function testCalculateDependencies(array $component_ids): void { self::assertSame([ + 'js.xb_test_code_components_using_imports' => [ + 'config' => [ + 'experience_builder.js_component.xb_test_code_components_using_imports', + 'experience_builder.js_component.xb_test_code_components_with_no_props', + 'experience_builder.js_component.xb_test_code_components_with_props', + ], + ], 'js.xb_test_code_components_vanilla_image' => [ 'module' => [ 'image', @@ -636,6 +643,17 @@ final class JsComponentTest extends ComponentSourceTestBase { */ public static function getExpectedClientSideInfo(): array { return [ + 'js.xb_test_code_components_using_imports' => [ + 'expected_output_selectors' => [ + 'astro-island[opts*="using imports"]', + 'script[blocking="render"][src*="/ui/lib/astro-hydration/dist/client.js"]', + ], + 'source' => 'Code component', + 'metadata' => ['slots' => []], + 'propSources' => [ ], + 'dynamic_prop_source_candidates' => [], + 'transforms' => [], + ], 'js.xb_test_code_components_vanilla_image' => [ 'expected_output_selectors' => [ 'astro-island[opts*="Vanilla Image"][props*="placehold.co"]', -- GitLab From 9aab93a25579e71ed43901557c39e5586f87e5fb Mon Sep 17 00:00:00 2001 From: Wim Leers <wim.leers@acquia.com> Date: Fri, 16 May 2025 16:28:49 +0200 Subject: [PATCH 34/38] Fix `::assertRenderedAstroIsland()` generation of an auto-save entry for a code component that imports other code components. --- .../ComponentSource/JsComponentTest.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index da2eefb198..65f5afd410 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -280,8 +280,15 @@ final class JsComponentTest extends ComponentSourceTestBase { // 'imported_js_components' is a value sent by the client that is used to // determine Javascript Code component dependencies and is not saved // directly on the backend. + // Ensure that the current set of imported JS components continues to + // be respected. // @see \Drupal\experience_builder\Entity\JavaScriptComponent::addJavaScriptComponentsDependencies(). - $js_component->normalizeForClientSide()->values + ['imported_js_components' => []], + $js_component->normalizeForClientSide()->values + [ + 'imported_js_components' => array_map( + fn (string $config_name): string => str_replace('experience_builder.js_component.', '', $config_name), + $js_component->toArray()['dependencies']['enforced']['config'] ?? [] + ), + ], ); } -- GitLab From 8713108f90344f694806c2d3543cf8b38c6a0c48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez=20Holgueras?= <18324-isholgueras@users.noreply.drupalcode.org> Date: Fri, 16 May 2025 14:35:51 +0000 Subject: [PATCH 35/38] Apply 2 suggestion(s) to 2 file(s) Co-authored-by: Wim Leers <44946-wimleers@users.noreply.drupalcode.org> --- .../ExperienceBuilder/ComponentSource/JsComponent.php | 6 +----- .../src/Kernel/Config/JavaScriptComponentValidationTest.php | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php b/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php index ac03a706a6..92ce9e4290 100644 --- a/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php +++ b/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php @@ -88,11 +88,7 @@ final class JsComponent extends GeneratedFieldExplicitInputUxComponentSourceBase public function calculateDependencies(): array { $dependencies = parent::calculateDependencies(); // @todo Add the global asset library in https://www.drupal.org/project/experience_builder/issues/3499933. - $dependencies['config'] = array_merge( - $dependencies['config'] ?? [], - [$this->getJavaScriptComponent()->getConfigDependencyName()], - $this->getJavaScriptComponent()->getDependencies()['config'] ?? [] - ); + $dependencies['config'][] = $this->getJavaScriptComponent()->getConfigDependencyName(); return $dependencies; } diff --git a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php index 181ea117da..002d8d1d92 100644 --- a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php +++ b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php @@ -175,8 +175,8 @@ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTest * @testWith ["missing", "The JavaScript component with the machine name 'missing' does not exist."] * ["", "The 'imported_js_components' contains an invalid component name."] * ["🚀", "The 'imported_js_components' contains an invalid component name."] - * ["componente_extraño", "The 'imported_js_components' contains an invalid component name."] - * [";", "The 'imported_js_components' contains an invalid component name."] + * ["componente_extraño", "The 'imported_js_components' contains an invalid component name."] + * [";", "The 'imported_js_components' contains an invalid component name."] */ public function testNonExistingJsDependencies(string $component_id, string $expected_exception_message): void { \assert($this->entity instanceof JavaScriptComponent); -- GitLab From 2aeced3f42146d798be8b5c77e34b82384f491a6 Mon Sep 17 00:00:00 2001 From: Wim Leers <wim.leers@acquia.com> Date: Fri, 16 May 2025 16:36:03 +0200 Subject: [PATCH 36/38] =?UTF-8?q?Make=20`JavaScriptComponentValidationTest?= =?UTF-8?q?`=20more=20like=20`PatternValidationTest`=20and=20others=20?= =?UTF-8?q?=E2=80=94=20no=20very=20atypical=20`::testEntityIsValid()`=20an?= =?UTF-8?q?ymore.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../JavaScriptComponentValidationTest.php | 61 +++++++++++-------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php index 002d8d1d92..9f6221e260 100644 --- a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php +++ b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php @@ -21,8 +21,6 @@ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTest use BetterConfigDependencyManagerTrait; - protected JavaScriptComponent $other_entity; - /** * {@inheritdoc} */ @@ -91,10 +89,44 @@ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTest 'compiled' => '.test{display:none;}', ], ]; - $this->entity = JavaScriptComponent::create([...$javascript_component_base, 'machineName' => 'test']); + JavaScriptComponent::create([...$javascript_component_base, 'machineName' => 'other'])->save(); + $this->entity = JavaScriptComponent::create([ + ...$javascript_component_base, + 'machineName' => 'test', + 'dependencies' => [ + 'enforced' => [ + 'config' => [ + JavaScriptComponent::load('other')->getConfigDependencyName(), + ], + ], + ], + ]); $this->entity->save(); - $this->other_entity = JavaScriptComponent::create([...$javascript_component_base, 'machineName' => 'test2']); - $this->other_entity->save(); + } + + /** + * {@inheritdoc} + */ + public function testEntityIsValid(): void { + parent::testEntityIsValid(); + + // Beyond validity, validate config dependencies are computed correctly. + $this->assertSame( + [ + 'config' => [ + 'experience_builder.js_component.other', + ], + ], + $this->entity->getDependencies() + ); + $this->assertSame([ + 'config' => [ + 'experience_builder.js_component.other', + ], + 'module' => [ + 'experience_builder', + ], + ], $this->getAllDependencies($this->entity)); } /** @@ -189,25 +221,6 @@ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTest $this->entity->updateFromClientSide($client_values); } - public function testEntityIsValid(): void { - \assert($this->entity instanceof JavaScriptComponent); - $client_values = $this->entity->normalizeForClientSide()->values; - $client_values['imported_js_components'] = [$this->other_entity->id()]; - $this->entity->updateFromClientSide($client_values); - $this->entity->save(); - $this->assertSame( - [ - 'config' => [ - 'experience_builder.js_component.test2', - ], - 'module' => [ - 'experience_builder', - ], - ], - $this->getAllDependencies($this->entity) - ); - } - public static function providerInvalidEnumsAndExamples(): array { return [ 'Invalid string' => [ -- GitLab From 745eceb7830bcf6c93c8238a7e85a31fca3771df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Fri, 16 May 2025 17:04:08 +0200 Subject: [PATCH 37/38] Issue #3522217: Fix phpstan and phpcs --- tests/src/Kernel/Config/JavaScriptComponentValidationTest.php | 1 + .../ExperienceBuilder/ComponentSource/JsComponentTest.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php index 9f6221e260..3a27df11d2 100644 --- a/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php +++ b/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php @@ -96,6 +96,7 @@ class JavaScriptComponentValidationTest extends BetterConfigEntityValidationTest 'dependencies' => [ 'enforced' => [ 'config' => [ + // @phpstan-ignore-next-line JavaScriptComponent::load('other')->getConfigDependencyName(), ], ], diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index 65f5afd410..d68927b6de 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -657,7 +657,7 @@ final class JsComponentTest extends ComponentSourceTestBase { ], 'source' => 'Code component', 'metadata' => ['slots' => []], - 'propSources' => [ ], + 'propSources' => [], 'dynamic_prop_source_candidates' => [], 'transforms' => [], ], -- GitLab From cd80036c127c8c1256440c30a87d3dbbd497f21b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20S=C3=A1nchez?= <nacho@isholgueras.com> Date: Fri, 16 May 2025 17:20:41 +0200 Subject: [PATCH 38/38] Issue #3522217: Revert tests. --- .../ExperienceBuilder/ComponentSource/JsComponentTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php index d68927b6de..f7746354b9 100644 --- a/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php +++ b/tests/src/Kernel/Plugin/ExperienceBuilder/ComponentSource/JsComponentTest.php @@ -367,8 +367,6 @@ final class JsComponentTest extends ComponentSourceTestBase { 'js.xb_test_code_components_using_imports' => [ 'config' => [ 'experience_builder.js_component.xb_test_code_components_using_imports', - 'experience_builder.js_component.xb_test_code_components_with_no_props', - 'experience_builder.js_component.xb_test_code_components_with_props', ], ], 'js.xb_test_code_components_vanilla_image' => [ -- GitLab