🇧🇪Belgium @wim leers

Both MRs need to get upstream changes merged in 🙏

Given FE lead @jessebaker made the FE changes, going ahead here 🚢

WFM, but given nothing is broken anymore, and this is now about enhancing, shifting to task rather than bug.

✨ [PP-2] Allow users to name components for the specific context Postponed landed!

🥳

D’oh, so simple! Wish I spotted it 😇🤓 Nice! 🙏

penyaskito → credited wim leers → .

but the actual update in gitlab file goes to this issue - https://www.drupal.org/project/experience_builder/issues/3518292 ✨ Allow searching for content in the editor navigation Active , where pipeline failing for 1 and passes for another , i'll update the gitlab file for that. Thanks

and

The failing pipeline in cypress test is not part of the changes done in this MR.

But … this one surely isn't introducing transliteration, so why is this one failing? 🤔 Anyway, because you really want this merge order, I tried to land ✨ Allow searching for content in the editor navigation Active , but couldn't: #3518292-28: Allow searching for content in the navigator → .

I know the sense of urgency y'all were under, so I spent ~30 mins reviewing this during my PTO today.

This MR is unfortunately not at all ready.

1. The tests are still hard failing on SQLite and PostgreSQL. You didn't implement what we agreed upon on Wednesday.
2. navigation.cy.js is consistently failing on the new test coverage, even after I re-tested it.
3. The information disclosure security vulnerability I pointed out on May 8 is still present, and the review thread was closed with a non-explanation 😅

… and that's just the big ones. There's plenty of smaller concerns, most of which I pointed out long ago. I have not yet re-reviewed all closed MR threads, because a bunch seem to have been closed prematurely 🙈

🏓 Initial review — looking good! A few things can be simplified, and I spotted one bug AFAICT 😇

wim leers → changed the visibility of the branch 3525601-personalization-segment-storage to hidden.

wim leers → changed the visibility of the branch 3525601-personalization-segment-storage to hidden.

Pragmatic. Let’s land it!

P.S.: there’s issues (in this same issue queue component, not linking because writing from phone) that aim to actually use Drupal’s/Symfony’s serialization formats (so: normalizers), but even then this is accurate. All that will do is create a specific subtype of the JSON format: application/json+xb or something like it.

The test you added in https://git.drupalcode.org/project/experience_builder/-/merge_requests/7... looks solid. 👍

I’ll review the newly challenges you identified in #53 on Monday.

Just move on to a different issue, I don’t want you to have nightmares about this over the weekend 🧟 😅

Thanks for 📌 Add FilteredPluginExistsConstraint constraint for validating Segment config entity Active ! That will actually be valuable in core too!

Needs rebase now that the docs MR is in 😇

Plus #11, of course.

Next up: 📌 PersonalizationSegment config entity Active .

Oh and FYI, the docs I requested aren't in this MR, but in 📌 Initial documentation draft Active , which I've also reviewed and is almost ready to land too 👍

@penyaskito addressed all my feedback from yesterday. Looks great!

Just one last request for a @todo <follow-up URL>, but other than that, this is good to go, and a very nice first step! 👏

So: RTBC'ing, to allow @penyaskito to self-merge as soon as that last bit is done!

🚢

Thanks so much for this! 🙏 It's super helpful 😄

Reviewed it from a high-level POV, avoided nitpicking.

Left clarification suggestions and a few questions, which are focused on increasing the clarity and hence practical usefulness of this document going forward.

The only thing I definitely want to see changed is `Component`: What we would refer to in technical terms as a `Component instance`. — that's too confusing, and in fact led to a lot of confusion when reviewing the designs.
Everything else is essentially clarifying. It'll probably take you <15 mins to address :)

97% ready now! Some clarifications needed. Some bits. And a few simple bits of test coverage 😇🤓

Thanks, this looks like a solid start!

Identified in review for 📌 PersonalizationSegment config entity Active .

🏓 — missing some crucial info that will be necessary for this to become committable — right now, it's not clear whether this would break contrib/custom modules 😇

This was surfaced by 📌 PersonalizationSegment config entity Active .

Added 🐛 Disabled SDC components are re-enabled after cache rebuild Active as a new beta blocker.

Just yesterday I happen to have worked on this code in 📌 Version component prop definitions for SDC and Code components Active — the place to update is src/Plugin/ExperienceBuilder/ComponentSource/SingleDirectoryComponent.php.

Great find!

We're definitely lacking test coverage for this. I think a new ComponentSourceTestBase::testUpdateDiscovery() would probably make sense?

👍Agreed on both counts!

Although, actually … I can think of a use case for consecutive exposed slots. Imagine you always want 2 columns. But you want content authors to populate them freely. That’s 2 slots in the same component instance that you’re both exposing.

Landed beta blocker ✨ Content templates, part 3b: store exposed slot subtrees on individual entities Active ! 🥳

Added 📌 Spike: explore merits of one XB field per exposed slot in content templates Active as stable blocker.

✨ Content templates, part 3b: store exposed slot subtrees on individual entities Active is in — crediting @nicxvan here for his comment about this issue's scope at #3519352-46: Content templates, part 3b: store exposed slot subtrees on individual entities → .

#4: AFAICT that's been solved for XB in 📌 [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table Active . The updating of that usage/dependency metadata DB table is atomic.

@larowlan Thanks for #56 Thanks for the issue summary update!

@catch in #57: RE 12 vs 20: There could easily be 100 slots in a given component tree. Most of them would indeed be "fixed" in the ContentTemplate: the site builder populates them as they see fit. The ones that are exposed by the site builder to content creators for them to populate are required to be empty (see ValidExposedSlotConstraintValidator). So, yes, 12 exposed slots already seems pretty much the upper bound in real-world terms. (In fact, we could even limit it — for either or both content author UX and performance/scalability reasons!)

So: AFAICT you were just talking about how many actual component instance slots exist in total in a content template ("distinct", 20) vs which ones are exposed (12). 👍

I see that both @larowlan in #56 and @effulgentsia in #58 basically agreed with what I wrote in #55, so tightening scope, and landing this first.

Met with @f.mazeikis and dug in to the remaining challenges. We agreed there's 2 remaining steps:

1. Add 2 test cases to \Drupal\Tests\experience_builder\Kernel\Plugin\Field\FieldType\ComponentTreeItemListTest::provider():
   

       yield 'component tree with a page title block component' => [...self::removePrefixSuffix($component_tree_with_page_title_block_component), 'isPreview' => FALSE];
       yield 'component tree with a page title block component in preview' => [...self::modifyExpectationFromLiveToPreview($component_tree_with_page_title_block_component, TRUE), 'isPreview' => TRUE];
   

   — essentially reusing $component_tree_with_single_block_component but changing it to use the page title block instead.

   This will require an addition like this to \Drupal\Tests\experience_builder\Kernel\Plugin\Field\FieldType\ComponentTreeItemListTest::testHydrationAndRendering():

   diff --git a/tests/src/Kernel/Plugin/Field/FieldType/ComponentTreeItemListTest.php b/tests/src/Kernel/Plugin/Field/FieldType/ComponentTreeItemListTest.php
   index ba8df6349..76d51f06a 100644
   --- a/tests/src/Kernel/Plugin/Field/FieldType/ComponentTreeItemListTest.php
   +++ b/tests/src/Kernel/Plugin/Field/FieldType/ComponentTreeItemListTest.php
   @@ -18,6 +18,7 @@ use Drupal\experience_builder\Element\RenderSafeComponentContainer;
    use Drupal\experience_builder\Entity\AssetLibrary;
    use Drupal\experience_builder\Entity\Page;
    use Drupal\experience_builder\Exception\SubtreeInjectionException;
   +use Drupal\experience_builder\Plugin\DisplayVariant\XbPageVariant;
    use Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItemList;
    use Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItemListInstantiatorTrait;
    use Drupal\experience_builder\Render\ImportMapResponseAttachmentsProcessor;
   @@ -118,6 +119,7 @@ class ComponentTreeItemListTest extends KernelTestBase {
        });
        $this->assertEquals($expected_renderable, $renderable);
    
   +    XbPageVariant::finalTitle = 'Funny string chosen by Felix :D';
        $html = (string) $this->container->get(RendererInterface::class)->renderInIsolation($renderable);
        // Strip trailing whitespace to make heredocs easier to write.
        $html = preg_replace('/ +$/m', '', $html);
   

2. Make sure the page title block also works when not using XbPageVariant:

   1. final class XbBlockPageVariant extends BlockPageVariant {
        public function setTitle($title) {
          XbPageVariant::finalTitle = $title;
          return parent::setTitle($title);
        }
      }
      

   2. usie hook_display_variant_plugin_info_alter() to override the class used for the block_page page variant to XbBlockPageVariant::class

   Once those 2 things are done, let's land this!

   Pre-emptively RTBC'ing because I'll be out in the next 2 days.

Great!

IMHO this is RTBC, and only needs that one test resolved.

@tedbow, could you please drive this across the finish line while @larowlan and I are out the next 2 days? 🙏 The sole remaining test failure is in TranslationTest, which you know best 😊

@tedbow: can you please respond to @mglaman in #38? 🙏

Based on meeting just now, can we get:

• the failing tests to explicitly skipped on SQLite, and have a comment pointing to the relevant core issue?
• an update to .gitlabci.yml to use MariaDB by default instead of SQLite? (Or MySQLite, whichever is fastest.)

This is not yet passing tests. 😇 Left a review with pointers.

📌 Tighten validation of `parent_uuid` and `slot` on XB fields to match the strictness of config Active landed.

• ✨ [PP-2] Allow users to name components for the specific context Postponed was +1'd by @larowlan.
• 📌 Version component prop definitions for SDC and Code components Active massive progress by @larowlan and I both.

Will try to push a solution for that, too.

Done:

(I'm aware of the clean-up potential. I'll let @larowlan refactor from this — AFAICT — working state to his liking 😊)

To enable @larowlan to focus on the actual tricky bits, and to avoid him having to spend time fixing the remaining test failures I left in #23 (8 days ago), I got that out of the way. I also addressed the @todo for adding validation.

All PHPUnit tests are passing, except for one. That one actually reveals a severe flaw in what I did 😇😬

The last failure:

Drupal\Tests\experience_builder\Kernel\Config\JavascriptComponentStorageTest::testComponentEntityCreation
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for experience_builder.component.js.bxnge13p with the following errors: 0 [versioned_properties.46d5d8c16712f873.settings.prop_field_definitions] 'noodles' is a required key.

… this is because the way Component config entities are currently validated per the current config schema, it is required for all past versions of a Component to have valid source-specific settings.

However … in the case of code components, it's possible that new props/slots are added! 😅 (More to come on that front in 🌱 Plan to allow editing props and slots for exposed code components Active .) Which then fails due to

…
    prop_field_definitions:
      constraints:
        # There must be a key for every `prop` in the corresponding \Drupal\experience_builder\Entity\JavaScriptComponent::$props.
        SequenceKeysMustMatch:
          configPrefix: experience_builder.js_component.
          configName: '%parent.%parent.%parent.%parent.source_local_id'
          propertyPathToSequence: props

And that is the big flaw: that actually only the active version MUST be valid, older versions MAY be valid. It's impossible to perfectly validate them, given the old implementation is no longer around. Will try to push a solution for that, too.

penyaskito → credited wim leers → .

Cool, then maybe this issue will be just about test coverage :)

Cleaning up 😇

Thanks! I'm afraid there's lots of out-of-scope code reformatting, even causing the phpcs CI job to fail 😇

Besides that, the other next step: update the UI :)

🏓

Kinda relieved to read my concerns about third-party settings are (seemingly) confirmed by you.

The static prop source defaults work can be combined with Wim's versioned config entity and we can add a version column to the field item that will apply to all components not just specific sources. That will give us the consolidation of the inputs column we're chasing here

I read that as "versions for all Component config entities, not just some, and stored as a new component_version field property for content entities and a same-name key-value pair for config-defined component instances" — which is exactly where I thought this was going last week 😄👍

Can't wait to see what you cook up for me by your EOD/my start! 🤓

🐛 SDC prop of `type: string` with empty string listed in its `enum` results in broken input UX Active is in!

🕺

Reviewed this 34 files, +2899, -166 diff at a high level.

1. My go-to test for making changes to the Component config entity is unsurprisingly ComponentValidationTest. But unfortunately that currently fails hard 😅
2. ComponentTreeItemListTest nicely illustrates the simplification for the stored data 👍
3. I have concerns about ::applyComponentTreeItemDefaults(), and I'm especially concerned about how it's used by ClientServerConversionTrait::clientModelToInput(). Left a lengthy comment there.
4. 👏 ComponentInputsEvolutionTest is fantastic! But … see the aforementioned comment for why that actually doesn't cover the full gamut of possible input evolutions.

AFAICT there are more remaining tasks:

1. multiple-cardinality support
2. explain the rationale behind the (auto-generated) naming scheme of the new config entities
3. provide an overview of which files are >=90% copy-pasted-and-renamed from field_union 🙏 I bet that >2K of the new lines are coming from field_union?

good point to stop and decide if we should keep going with this approach

I still am optimistic! So, I'd say: start with my main concerns (explained in detail on the MR), because if you can overcome/explain away those, then I think we're on track here 🤓

Only one test failure:

Drupal\Tests\experience_builder\Functional\TranslationTest::testTranslation with data set "not translatable"
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'Starring … Drupal as the hero! 🤩'
+'Drupal, c'est magnifique !'

I bet @larowlan will quickly find the root cause while reviewing 😇 🤓

Per #3526818-5: Tighten validation of `parent_uuid` and `slot` on XB fields to match the strictness of config → .

Turns out to be an easy fix 🥳

Root cause: Symfony magic: the Optional constraint implies NotBlank apparently 🤷‍♂️

This means we can downgrade the priority of 📌 Default content exports are invalid and hence are not correct after importing Active to 👍

This somehow introduced a regression for default_content's export functionality: 📌 Default content exports are invalid and hence are not correct after importing Active

Which would've been far less painful for @justafish to run into if the validation this MR introduced caught the problematic bits, created 📌 Tighten validation of `parent_uuid` and `slot` on XB fields to match the strictness of config Active for that.

Added 📌 Default content exports are invalid and hence are not correct after importing Active to stable blockers and 📌 Tighten validation of `parent_uuid` and `slot` on XB fields to match the strictness of config Active to beta blockers — thanks @justafish!

@phenaproxima: I suspect that the NULL values stored in the DB for parent_uuid and slot are actually cast to string at some point (either the DB layer, or SQL content entity storage, or Entity Field API, or …), and that's how they end up being exported wrong?

Note that I actually can get validation errors to occur, for example doing

diff --git a/tests/fixtures/xb-page-with-data.yml b/tests/fixtures/xb-page-with-data.yml
index 97b35785c..216b7041e 100644
--- a/tests/fixtures/xb-page-with-data.yml
+++ b/tests/fixtures/xb-page-with-data.yml
@@ -19,7 +19,7 @@ default:
       inputs:
         width:
           sourceType: 'static:field_item:list_integer'
-          value: 50
+          value: 40
           expression: ℹ︎list_integer␟value
           sourceTypeSettings:
             storage:

results in:

1) Drupal\Tests\experience_builder\Functional\DefaultContentImportTest::testImportDefaultContentWithXbData
Drupal\Core\DefaultContent\InvalidEntityException: /Users/wim.leers/core/modules/contrib/experience_builder/tests/src/Functional/../../fixtures/xb-page-with-data.yml: components.0.inputs.16176e0b-8197-40e3-ad49-48f1b6e9a7f9.width=Does not have a value in the enumeration [25,33,50,66,75]. The provided value is: "40".

So that means

Worse: the Recipes default content import functionality apparently did not report a validation error?! 😱

is a different problem, and it's a beta-blocking one.

Extracted that to 📌 Tighten validation of `parent_uuid` and `slot` on XB fields to match the strictness of config Active .

wim leers → created an issue.

Ironically I surfaced something closely related in reviewing that Recipes MR a ~year ago: #3439923-31: Add recipes api as experimental API to core → + @phenaproxima's response at -32, where he says:

it's exported by the default_content module, which is what generates these YAML files. IMHO they should not be changed at this time, but if (which, once this is committed, is really when) core adds the ability to export content entities as YAML […]

😭😬 — no related issues linked from #3439923 that are doing that, and search engines aren't finding such an issue either…

OMG, AFAICT \Drupal\Core\DefaultContent\Importer exists but not \Drupal\Core\DefaultContent\Exporter?! CR: https://www.drupal.org/node/3445169 → .

I'm hoping that @phenaproxima can push this forward 😇

wim leers → created an issue. See original summary → .

The untranslatable_inputs sample diff for post-stable was incomplete.

Sooooooooooo very close! 😄

(Title change in #44 to reflect this won't be doing the UI pieces — and the UI pieces are not beta-blocking. This is about the back end leaping ahead of the front end.)

📌 [PP-1] Consider not storing the ComponentTreeStructure data type as a JSON blob Postponed made this really easy: add a new name field property, and add a corresponding new key-value pair to type: experience_builder.component_tree_node.

I'd like a review from @larowlan given he led the work on 📌 [PP-1] Consider not storing the ComponentTreeStructure data type as a JSON blob Postponed 🙏

wim leers → changed the visibility of the branch 3460958-edit-name-tag-component to hidden.

Let's get this issue on track, since it's a beta blocker.

Re-read this issue.

The trade-offs I described in #2.2 are irrelevant after 📌 [PP-1] Consider not storing the ComponentTreeStructure data type as a JSON blob Postponed :)

The concerns I raised in #2.1 and elaborated on in #6 are easily solvable:

1. when there's asymmetrical translations (potentially everything different per translation), there already was no challenge
2. when there's symmetrical translations (different inputs per translation but locked tree), we'd initially show 2 label "chip"s when hovering a component, and 2 labels in the layers panel: first the (translated) Component name, then the default translation's component instance name with a language indicator. As soon as the content author assigns a name to the component instance in this language, that assigned name would be the only thing we'd show.

(That last bit is what I suspect @lauriii was imagining when he wrote #39.)

Pleased to see I got this back on track in #34 even though I do not recall writing that — thanks past self 😅

#32 failed to update from 🟢 to ✅ for the cited reasons → .

Per #37, I triaged all config management issues this morning.

Didn't find anything.

So I can move to 🥳 Note that #3526127-5: Ensure predictable config export order of config-defined component trees → might become a beta blocker.

did not have an indicator yet — the sole issue there is well on its way, so marked it 🟢.

Here there is not, and perhaps we should introduce it.

Here it is indeed the sequence order (which implicitly has "deltas" as keys: a PHP "list" array, so 0, 1 etc.) that ensures the correct ordering.

I'm contemplating whether this should be a beta blocker or not. It feels like it'd be safer to make this a beta blocker. Do you agree?

Having reviewed #3519352 and written #3519352-55: Content templates, part 3b: store exposed slot subtrees on individual entities → (specifically the part), I'm bumping from 🟡 to 🟢.

Was the component already placed within experience builder and saved?

My bet too :)

@thoward216 Just confirming … you are unblocked, right? 🤞

wim leers → changed the visibility of the branch 0.x to hidden.

Update for #29:

• 🐛 Component trees with dynamic prop sources that introspect field values don't add the relevant fields to their dependencies Active is fixed
• #3519352 is unblocked, and is in need of review by @phenaproxima at #3519352-55: Content templates, part 3b: store exposed slot subtrees on individual entities →

Bumping this to because of the security concerns raised by @catch. It's important that we land on how to handle the storage needs of content templates before 🌱 Milestone 1.0.0-beta1: Start creating non-throwaway sites Active , even if the end-user facing functionality does not exist yet.

Discussion

@catch in #44: no, ✨ [later phase] Provide API for finding and UI for surfacing dangling/dead component subtrees — aka garbage clean-up Postponed will be necessary even if we do that. Check it out — there's other ways to end up with dangling subtrees 😬

@larowlan in #45: 👏 Yes!

@larowlan in #48:

1. This is the part that this issue could make simple 👍 That would keep the scope of ✨ [later phase] Provide API for finding and UI for surfacing dangling/dead component subtrees — aka garbage clean-up Postponed smaller, but #3524406 would still be necessary.
2. 💯 — that's why I was excited about that insight by you and @effulgentsia! 😄

@catch in #51:
Where you wrote

So either actually using field purge (because field-per-slot), or expanding 'field delta purge by component' would avoid that. Although field delta purge by component obviously doesn't exist as a concept so it might not be a small undertaking to implement.

I do understand .

But I don't understand the other. We'd have to purge all component instances within an exposed slot that was deleted. So it'd be basically:

1. UPDATE {field_table} SET deleted=1 WHERE parent_uuid IS NULL AND slot LIKE 'my_deleted_exposed_slot'; for the content entity type+bundle whose ContentTemplate had an exposed slot deleted
2. the same for the {field_revision_table} revision table
3. … then gradually purging just those rows, not all fields. The challenge there is AFAICT the need to call \Drupal\file\Plugin\Field\FieldType\FileFieldItemList::deleteRevision() and friends.

@catch in #54:

But also 12 different exposed slots would imply non-exposed slots in between, I can't see why there would be adjacent exposed slots, that could just be one slot. So we're talking about well over 20 distinct slots at that point? To me this seems like it would be an extreme upper bound, and 1-3 is more likely unless I'm missing something with the use case.

How do you go from "12 exposed slots" to "20 distinct slots"? 🤔

MR review + issue/MR scope

👏 @larowlan for #49 and #50 🤩

The issue summary is now wildly out-of-date, given the new 📌 [PP-1] Consider not storing the ComponentTreeStructure data type as a JSON blob Postponed -powered reality, so let's fix that.

The current data model (specifically the allowed values for the parent_uuid and slot field properties in \Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem::propertyDefinitions()) clearly are sufficient to make this functionality work — because the post-3468272-updated MR is passing tests.

The current MR chooses to not store one field per exposed slot. If we were to do one-field-per-subtree-for-an-exposed-slot, we'd need to do the following on top of this MR:

1. Add a exposed_slot storage setting to ComponentTreeItem.
2. Add a validation constraint that enforces that storage setting: it must disallow both parent_uuid and slot being NULL, because those would be component instances targeting the root of a component tree.
3. Update \Drupal\experience_builder\Entity\ContentTemplate::build() and the ComponentTreeLoader service not load the sole XB field, but all of the XB fields on the given content entity type + bundle, and call ::mergeSubTreeItemList() on all of them.
4. Tests

I don't see any reason why we cannot do that in a follow-up, nor do I think that that introduces any additional risk at this time.

That would allow @phenaproxima to get started on next steps (such as ✨ Content templates, the boss battle: create a UI for editing templates Active ).

Thoughts? 😊

Triaged the issue queue component, as well as the @todos in XB's config schema.

Added many newly created issues — all of them stable blockers unless marked otherwise:

1. 📌 [11.2-only] Adopt `AtLeastOneOf` validation constraint for cardinality validation Postponed
2. ✨ Stop assuming default Field Widget settings suffice — add Field Widget settings support to `experience_builder.generated_field_explicit_input_ux: prop_field_definitions` Active
3. 📌 Tighten validation of `experience_builder.generated_field_explicit_input_ux: prop_field_definitions` Active
4. 📌 Tighten validation of `experience_builder.generated_field_explicit_input_ux: prop_field_definitions.[%key].expression` Active
5. 📌 [>=11.1.8] Remove `type: field.value.language` work-around once Postponed
6. 📌 [PP-1] Remove BetterConfigExistsContraint and move back to ConfigExistsContraint Active → post-stable priority
7. 📌 [later phase] [PP-1] Gracefully handle deleted regions in PageTemplate config entities" Postponed → post-stable priority
8. 📌 JavaScriptComponent config entities should have mutable machineNames Active
9. 📌 Allow XB to be used on all node types Active
10. ✨ [PP-1] Validate that content templates are attached to entity bundles that fulfill XB's requirements Postponed
11. ✨ Implement `::onDependencyRemoval()` for `Pattern` and `PageRegion` config entities: when a module providing a field type is uninstalled, replace it with the default StaticPropSource Active → post-stable priority
12. 📌 [PP-2] Add ComponentAuditabilityTest Active
13. 🐛 SdcPropKeysConstraintValidator::validate() should complain about extraneous keys too, not just missing keys Active

A lot of progress has happened on this front!

I used 📌 [PoC] Introduce a `ContentTypeTemplate` config entity Active to create a PoC, to align with @lauriii on the product side and @callumharrod on the design side. We now have actual designs!

Then … @phenaproxima stepped in and took the PoC I did in #3511366, and started really building this:

1. ✨ Content type templates, part 1: introduce a ContentTemplate config entity which always renders an empty component tree for a particular entity display Active
2. ✨ Content templates, part 2: Using the templates for rendering Active
3. ✨ Content templates, part 2: store a component tree in the template, and allow individual entities to fill in specific slots in that tree Active
4. (blocked/long detour via 📌 [PP-1] Consider not storing the ComponentTreeStructure data type as a JSON blob Postponed , which now landed)
5. currently being worked on: ✨ Content templates, part 3b: store exposed slot subtrees on individual entities Active

#48:

Now we need to add a new block to section 3 for legal or compliance reasons.

This new block is not going to appear on any of the overridden nodes, so we need to write a complex update hook to add it everywhere.

This is exactly the problem we wanted to avoid! 👍 And we did. See ✨ Content type templates, part 1: introduce a ContentTemplate config entity which always renders an empty component tree for a particular entity display Active + ✨ Content templates, part 2: store a component tree in the template, and allow individual entities to fill in specific slots in that tree Active + ✨ Content templates, part 3b: store exposed slot subtrees on individual entities Active .

This means the editors lose some control over what parts of the page they can edit (Only the middle section), but allows us to add new content to the top or bottom blocks that will automatically apply to all nodes, regardless on if they're overriden or not.

Yep, this is exactly what exposed slots are about. See ✨ Content templates, part 2: store a component tree in the template, and allow individual entities to fill in specific slots in that tree Active :)

This leads us to the second problem, how do we add new Sections to the default template and have them flow through to the overridden nodes in the correct place.

This, too, is addressed in the above. Because a ContentTemplate for article Nodes literally controls every article node entity, with each individual article node only being able to populate exposed slots.

@callumharrod has designs. I'll ask him to share those here.

Per #5.

Re-scoping to #7 per @balintbrews in #8.

Also, folding the original scope of 📌 Consider renaming javascript component ID to 'id' rather than machineName Active into this:

  $component = JavaScriptComponent::create([
      'machineName' => $this->randomMachineName(),

Camel case here has tripped me up twice now, this should probably just be id or machine_name.

— @longwave at https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

We either need to:

1. make this a beta blocker
2. or make this a stable blocker, but then we'll need an update path

For now, assuming the latter.

Re-parenting to 🌱 [META] Production-ready data storage Active . Related: 🌱 Plan to allow editing props and slots for exposed code components Active .

Two big changes since we last commented on this issue, which impacts what this issue would need to solve:

1. 📌 Calculate field and component dependencies on save and store them in an easy to retrieve format Active happened ⇒ we can now fully reliably query not just the existence of a Component config entity for a code component (i.e. a JavaScriptComponent config entity), but also for instances of (i.e. usages of) that Component config entity!
2. ✨ Store and calculate dependencies in `JavaScriptComponent` Active happened (which is what @effulgentsia described in #12) ⇒ changing a JavaScriptComponent's machineName would now also have to update both the enforced_dependencies of all JavaScriptComponents that depend on the one being renamed, as well as updating the source code JS + compiled JS

That last one I know that @balintbrews has actually provided an outline of a solution for in #14.

I think I partially understand now! 🥳

Doing this issue would mean storing not

dependencies:
  enforced:
    config:
      - experience_builder.js_component.xb_test_code_components_with_no_props
      - experience_builder.js_component.xb_test_code_components_with_props

but

dependencies:
  enforced:
    config:
      - experience_builder.js_component.56133d44-1556-4408-a225-4e00a22d6998
      - experience_builder.js_component.dbba4ea1-b2b2-4f48-8ae5-a70efdedb3f3

Plus, at code editor time, transform

import Button from '@/components/56133d44-1556-4408-a225-4e00a22d6998'

to

import Button from '@/components/button'

So that to the person using the code editor, the UUIDs are never visible, only the (mutable!) machine name. Which means that if somebody changes the machine name from button to fancybutton, I would automatically see:

import Button from '@/components/fancybutton'

Cool! But, … what if the code component is modified to no longer export Button? 🤔

MR with test coverage up. 👍

But … does this even work? Doesn't \Drupal\Core\Template\ComponentsTwigExtension::mergeAdditionalRenderContext() only support a single attributes prop? 😅

So that's definitely an upstream bug in SDC.