🇧🇪Belgium @wim leers

This status probably makes more sense?

@justafish is there more to be done here, or can we close this again? 🤞

(Not working today but bringing order to persona life after XB craziness — including closing >100 tabs on my tablet 🤣🥳)

larowlan → credited wim leers → .

Per @larowlan at #3537154-24: Regression in #3529788: can publish component instances with empty required props → , the data integrity issue this issue originally caused is fixed 👍

@larowlan pointed out in chat that actually minLength: 0 (or the absence of minLength) as I did in https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... is inadequate — we should still require strings using format or pattern to be valid, and not attempt to update the preview. IOW: only "prose"-esque strings. Just did that: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

In response to #23 and #24 I think as a minimum we should be displaying an error banner and retaining HTML5 validation errors on the form using the existing styling.

+1 — but can we do that in a follow-up issue instead? 🙏

And can we let the follow-up MR in this issue be just about the tightening of what the original MR of this issue (!1305) did, to truly only handle "required prose strings should be able to be empty and still update the preview to match"? 🙏

You're right that the scenario you described in #29 currently fails. That's exactly what was happening in HEAD for empty required strings, which @lauriii wanted to improve for usability, which is what this issue did. What your STR+screenshot shows is precisely what @bnjmnm, @effulgentsia and proposed a direction for in 📌 Invalid prop errors should be detailed in Component preview Active , and it relates to the known missing designs → outlined in 🌱 [META] Robust component instance error handling Active .

penyaskito → credited wim leers → .

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

I forgot a thing in !1210 🙈

I think Nacho having addressed Lee’s review means @tedbow can RTBC & merge 😊🤞

@lauriii confirmed the backporting of this to 0.x in Slack :)

This seems critical for usability and accessibility?! 😄 Love it!

(A GIF showing it in action would be nice too. 😇)

#34: perfect, that sounds completely sensible 😊 — thanks!

Crediting Mayur for finding this.

Assigning to Auto-save maintainer @tedbow to investigate deeper, and to decide whether this requires only front-end changes or not.

I couldn't reproduce #31 + #32 every time, but almost every time.

It's just a race condition in the auto-save system. It's literally what both of the referenced issues were about. We'll just need to harden against that. Created a follow-up: 📌 Auto-save conflict upon populating an empty required `type: string` SDC prop and immediately publishing Active .

This is still ready for backporting. The data integrity issue that this fixed is still real.

wim leers → created an issue.

@justafish and I met about this MR — we both agrees that https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... is problematic.

The reason that code is in there, is because @justafish understood from talking to @lauriii and from this example in the issue summary:

{{ include('image', {
  src: './gracie.jpg',
  alt: 'Gracie is awesome',
  class: 'max-content',
  fill: true,
  sizes: '(max-width: 768px) 100vw, (max-width: 1200px) 50vw, 33vw'
})}}

… that:

• (the examples.0 in the metadata) must ALSO be able to generate a srcset
• and on top of that, must ALSO be able to generate a srcset

Those 2 bits are why this (in-progress) MR is copying local files (both inside the SDC and outside) to some location in public://: because otherwise no derivative for it can be generated, since \Drupal\image\Controller\ImageStyleDownloadController::deliver() does NOT support "shipped files" (aka files shipped as part of a module or theme), even though the logic in \Drupal\image\Entity\ImageStyle::buildUri() suggests it does (see the else branch).

(For debugging: install XB, then navigate to /sites/default/files/styles/xb_parametrized_width--640//core/tests/fixtures/files/image-1.png.webp to generate a 640px-wide derivative of core/tests/fixtures/files/image-1.png. Put a breakpoint in the aforementioned methods and observe: it'll try to use core as the scheme.)

This is why @justafish resorted to copying shipped files into public:// — because core's ImageStyleDownloadController does not support generating derivatives for shipped files.

But precisely that is the very worrying piece here: a Twig extension writing to the file system as a side effect. 😱

This opens a can of worms:

1. What if the default image is updated (its contents are changed but its filename is not): that should then be reflected too in the public:// copy. How do we efficiently detect such a change?
2. What about multiple SDCs with the same filename? Multiple extensions with the same SDC name? In core, the File entity does the deduplicating of identically named files; here we would become responsible. A naming scheme is conceivable (public://sdc-default-images/<extension name>/<SDC name>/<filename>) to avoid that, but it's not great.
3. What about the extra disk space consumption? Wouldn't it be better to just use the actual shipped files instead?

Conclusion

1. IMHO the value of generating a srcset for a shipped default image in an SDC is questionable — it is unlikely to be enormous anyway. @justafish understood @lauriii to want this, but based on the issue summary, I'm not confident that is the case. Doubly so because we don't currently do that in HEAD either and this was jointly agreed in ✨ Add automated image optimization to image component Active as a reasonable trade-off.
   EDIT: I found the original comment covering this → .
2. If there is sufficient value in that in Lauri's eyes, then the correct (and maintainable) solution would be:
   1. Rewrite the relative-to-the-SDC URLs to relative-to-Drupal-root URLs, aka exactly what \Drupal\experience_builder\PropSource\DefaultRelativeUrlPropSource + \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponent::rewriteExampleUrl() do in HEAD.
   2. Subclass core's ImageStyleDownloadController and add support for shipped files.
3. 99% of the value is NOT in making the "shipped images in an SDC have srcset derivatives" bit work, but in what the issue title says: . So: if @lauriii thinks this should happen despite all of the above concerns, then it'll need to be in a non-beta blocking follow-up.

Assigning to @lauriii to get feedback on this; meanwhile @justafish will push ahead with everything else.

Thanks!

Reproduced:

Investigated that and:

1. the first message is coming from ui/src/features/editor/ConflictWarning.tsx, introduced in 📌 [PP-1] Enforce conflict enforcement outside of tests and e2e tests Postponed
2. the second message is coming from \Drupal\experience_builder\Controller\UnmatchedItemInPublishRequest, introduced in 🐛 Messages in \Drupal\experience_builder\Controller\ErrorCodesEnum are not helpful Active

Digging deeper now.

IOW this was a bug in 📌 Ensure predictable config export order of config-defined component trees Active — the test coverage that that added and this issue expanded has now been generalized using a new \Drupal\Tests\experience_builder\Kernel\Config\ConfigWithComponentTreeTestBase.

I expect linting errors but tests to pass — will fix after lunch 👍

After writing #30, one thing didn't sit right: we're only adding test coverage for Pattern, but per @larowlan in #20, this affects ContentTemplates too, and per my #30, PageRegions too.

We should test all 3, generically. Made it so.

AFAICT this could also affect PageRegions, because that's the third config entity type that relies on \Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItemListInstantiatorTrait::generateComponentTreeKeys()

This boils down to data loss, so … tagging .

Thanks!

Do we need another follow up for the "unpublished pages appear in the search navigator" or is that working as intended?

That is working as intended, and was introduced in ✨ Allow searching for content in the editor navigation Active .

Mayur explained in chat:

My response:

@Mayur Sose that’s expected — because it’s only been named "xyz" in auto-save — the actual underlying SAVED entity is still stored with "Untitled page"

That means this now just needs a simple direct backport. 👍

Thanks everyone!

AHHHHHHHHHH! So you developed on 0.x, the MR targeted 0.x and then you changed it to 1.x
That explains everything.

Except … HOW F'ING STUPID GitLab is being here, for not showing such a significant change (different target branch) in the "activity" entries:

It lists zero-impact activity there, but this it does not.

Made me lose 20 minutes of my day. 😬

But the diff is correct, so it should be fine to just squash-merge it.

Can't do that due to A) it not applying to 1.x, B) hence not running tests, C) the d.o-GitLab integration. Need to use the "merge" button on d.o, and then this won't work.

Rebased to be able to land this 👍

Also, the issue summary is very outdated, because it says

See https://www.drupal.org/project/experience_builder/issues/3532718 📌 Improve the front-end DX of Active for relevant upcoming changes to how srcset is generated, however some of the work here can be started before that's completed e.g. creating a "hidden" component, the card component, and a skeleton for the new image component

and I believe that the thing that concerns me most is something worth calling out in the issue summary.

📌 Improve the front-end DX of Active landed 9 days ago.

Did an initial review of the other MR, and raised 2 major concerns 😅

Per #22.

Discussed in chat and we decided to move forward with this and create a follow-up for #13. @jessebaker's availability is limited today.

WTAF happened to this MR? It's still @fago's commit from 10:43, so I don't think he pushed again? There was 1 commit, now there's 8?!

I've been intermittently seeing this kind of problem on GitLab. Seems like it's having some integrity issues…

Was going to merge, will now wait a bit for this to re-settle 🙃

Please provide the corresponding Component config entity at each stage. Specifically, any change to the code component that is *saved* (not auto-saved, but actually saved) SHOULD result in an update to the code component's Component config entity.

(That would be experience_builder.component.js.[machine name of code component].)

Export it either using drush cget or at /admin/config/development/configuration/single/export

Remove component from page and then from components list

This is very hard to interpret. I think the first time "component" really means "component instance", and the second time you're removing the code component from

Please always use the term "component instance" when referring to an instantiation of any component (no matter whether it's an SDC, code component or block plugin).
"component" refers to the generic concept that can be instantiated, represented inside XB itself by the Component config entity — which is how XB knows about a component coming from some source (SDC is a source, code components are a source, block plugins are a source).

Thanks for the very fast turn-around, so shortly after #3503412, @hooroomoo! 🙏

It's not that we specified the wrong property, it's that the inherited \Drupal\Core\Field\FieldItemBase::mainPropertyName() does not make sense for XB's field type.

(I bet there will be more such things — few field types in the Drupal ecosystem have as many field properties as XB's field type…)

Thanks for the bug report!

OMG LOL!

Per @larowlan's approval.

Reverting those test changes passed just fine after locally I was convinced they were not needed — yay, zero concerns left! 🥳

@balintbrews is doing the cherry-picking to 0.x. This should pick cleanly.

Investigating the sole lingering concern: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

Thanks, @larowlan! 😊

I'm very relieved that this simple follow-up MR addresses the data integrity pieces — whew! 😇

I do very much agree we should do what you proposed at #3529788-29: Required `type: string, minLength: 0` props should fall back to empty string (even though invalid) while Content Author is typing, to allow matching live preview in most cases → ! 👍

For #13 — thanks Mayur!

Whoops — pretty evident now that I just threw up an MR without actually testing it! 😇

#9: RE: subdir — how come this worked fine until now?! 😳

I bet this fails because

    // Case 3: Route with _xb_use_template_draft should use regular asset
    // library if there is no auto-saved version.
    $assert_library_global_library('/xb/api/v0/layout/xb_page/' . $page->id(), TRUE);

does not negotiate the default theme.

TIL ✨ Allow SDCs to be marked to be excluded from UI Active happened!

RTBC'd the "pure infra" MR https://git.drupalcode.org/project/experience_builder/-/merge_requests/1307 — not the other MR.

Needs follow-up issue to remove the work-around once XB requires >=11.3.

Back to and to Sally for the other MR on this issue :) Thanks, @penyaskito!

Failing test-only CI 👍

There's no change record yet 😅

I'll review tomorrow — it's possible @larowlan will beat me to it, so not yet self-assigning.

This can break the admin theme! 😅🙈

Reported by @mglaman in chat.

wim leers → created an issue.

AFAICT \Drupal\Core\Entity\EntityChangedTrait::setChangedTime() is only called by tests, except for in one main spot and 3 spots in total:

1. \Drupal\Core\Entity\ContentEntityForm::updateChangedTime() (which is what we're hitting)
2. \Drupal\Core\Entity\Form\RevisionRevertForm::prepareRevision()
3. \Drupal\Core\Action\Plugin\Action\SaveAction::execute()

Related: shouldn't we remove all our explicit checks for 'changed' the field NAME and switch it over to checking 'changed' the field TYPE? 😅

The mistake I made in 🐛 Hero component failed to render after removing text Active (besides it not being sufficiently narrow in special-casing allowing the empty string — follow-up MR on that issue to fix that) is that while StaticPropSource::withValue() would complain hard and loud upon trying to load/edit a component tree with a required type: string, minLength: 0 SDC prop, it SHOULD have prevented it from being saved in the first place.

@tedbow's test coverage nicely reproduces that, and it's the test coverage I should've added in #3529788. My bad. 🙈

The IMHO correct solution was actually pretty simple, and what I suggested ~24 hours ago worked fine:

1. commit 1: port Ted's test coverage — fails hard 👍
2. commit 2: do an extra check in ::isMinimalRepresentation(), similar to what #3529788 did for ::withValue(), which SHOULD work because ::buildPreviewRenderable() already is ignoring any validation errors precisely because it's for previewing — reduces the massive fail to a narrow one: a message mismatch
3. commit 3: make the message match the expected one by making the check in StaticPropSource::isMinimalRepresentation() throw a different exception that ::validateComponentInput() can catch, to allow this in turn to trigger the exceptions from SDCs themselves — thus replacing the hideous/confusing validation error linked above with the one we've been seeing all this time: the one from \Drupal\Core\Theme\Component\ComponentValidator::validateProps() — should be green

Thoughts? :)

Thanks!

Thanks!

This is IMHO the bit we originally missed.

WDYT about #24 + the draft MR? 😊

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

Responded in-depth at #3529788-24: Required `type: string` props should fall back to empty string (even though invalid) while Content Author is typing, to allow matching live preview in most cases → why I think the alternative MR is the wrong direction — tl;dr: this is a usability regression for the reasons @lauriii cited at #3529788-23: Required `type: string` props should fall back to empty string (even though invalid) while Content Author is typing, to allow matching live preview in most cases → .

I agree with @lauriii on this one — the point is indeed that while the Content Author is doing their thing, it's possible that both:

1. neither the prop sources themselves would be considered invalid (i.e. the field types do not contain valid non-empty field item values)
2. nor would the evaluated results (i.e. the values returned by a static prop source getting evaluated wouldn't pass SDC's JSON Schema validation)

Yet we'd still want:

1. the preview to be updated using the invalid data
2. the invalid data to be auto-saved

Because without the first, it'd result in a (real-time) jarring user experience: the preview wouldn't match what the Content Author sees in the component instance form.

Because without the second, it'd result in a (upon return) jarring user experience: the component instance form wouldn't be in the exact state that the Content Author left it — which is a perhaps typical kind of data loss, but it's data loss nonetheless.

In terms of revisiting here, I think we should prevent PATCHing/autosave/preview from happening if the form is invalid.

That is literally what @effulgentsia, @bnjmnm and I proposed, but @lauriii was very strongly against this, for the UX reasons cited above.

Without that, we're basically rendering our use of ajv/clientside validation redundant - if a user can edit another field to dismiss validation - there's no point to it and it doesn't give us any protection at all.

I agree, partially: I don't think it's entirely moot. I think this is about strategically allowing certain technically invalid values when they degrade the UX too much. For example:

• foobar is never a string, so client-side validation informing the Content Author of that and not even trying to update the preview is good
• https:/ (author mid-typing and then pausing to find the URL copy/paste) is not a valid URL, so not even trying to update the preview is once again good
• '' (the empty string) is semantically inadequate for meeting the "required string" JSON schema prop shape, but it is (usually — some code components or SDCs might very well have logic that crashes if they are given the empty string, but that is likely a minority) 👈 this issue!
• etc.

That's why I handled it so narrowly in this MR and included the rationale in long comments. But … I do realize now that on the client side it's handled much more (too) broadly: that needs to be narrowed to only allow it for type: string without a minLength.

1. The test changes here to make the above change pass

I sure was somewhat nervous about this change. My rationale (which I should've posted 😬🙈) is that it actually never made sense for empty strings to get stored for those in the first place! The reason this test worked at all until that commit you cited is because we never actually prevented storing useless data (a field item value that would've been rejected if we'd been calling ::filterEmptyItems()). That made the stored JSON blobs unnecessarily big.

🥳

Removing the prefix, because that's already the reality of XB anyway :)

LGTM — only needs addressing of @larowlan's edge case, and just have 2 minor questions.

Mentioned today, while we are here let's ensure we have a plan (fix here or follow-up) for Attributes prop marked as required works, even if that doesn't make any sense.

👆 This is not yet addressed, but the MR is ready, so tagging .

Also linking the core issues that @penyaskito found in his research at https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

Per #8, let's ask Mayur or Neha to manually test this 🙏

Failing test-only CI 👍

I had only nits. Over to @tedbow for final approval given this is firmly in the component :)

Looks great — thanks!

Thanks!

+1 that tests are overkill here.

As I previously described in the place that originally reported this: this is NOT a security concern because XB's config entity access control handlers special-cased the view and delete operations and deferred ALL OTHER operations (including non-existent ones) to the same logic: check if the current user has the config entity type's "admin" permission. 👍

Test-only CI job failed 👍

Requested a few clarifications, specifically around the continue — I'm not convinced yet it is necessary. It's better to fail explicitly than silently ignore problems — unless there's a good reason, and then that should be documented 🙏

Ship it!

Thanks for your testing, @heyyo!

IMHO this should be backported to 0.x, but deferring to @lauriii.

LGTM! Note that there's many known issues with GeneratedFieldExplicitInputUxComponentSourceBase — see https://www.drupal.org/project/experience_builder/issues/3520484#stable 🌱 [META] Production-ready ComponentSource plugins Active

This in and of itself seems fine, and definitely won't get in the way of that :)

🏓 Responded to @f.mazeikis' most pressing question; didn't do a full review, but … the scope here is now no longer so narrow, but generic, thanks to ✨ Define JSON Schema $refs for linking/embedding videos and linking documents Active + ✨ Add a Video prop type to the Code Component editor Active .

I very strongly disagree with #20. 😅

1. Layout Builder does NOT render the entire page, XB does. (At the technical level: XB implements \Drupal\Core\Display\PageVariantInterface, LB does not.) LB is used to render static(ish) content (the special "Field" block plus reusable content blocks)
2. While 📌 Support auto-placeholdering for blocks placed in Layout Builder Needs work may indeed have only 22 followers, the ambition you've expressed for XB is for it to take over much functionality of Drupal core. If that's the case, then we cannot delay getting this right until a distant future. If this is not a stable/1.0 blocker, that would mean that it's irresponsible and hence impossible to make ComponentSource plugins a public API by that time.

@lauriii, please see the proposed resolution.

Marked #3513406 as a duplicate per #3513406-13: ValueError: "Drupal\Core\Template\Attribute" is not a valid backing value for enum → . Crediting the contributors there that helped get clarity there, which allowed marking it as a duplicate 😊

Per #3513406-8: ValueError: "Drupal\Core\Template\Attribute" is not a valid backing value for enum → , this affects the quite popular https://www.drupal.org/project/radix → theme significantly.

Basically this just needs to be generalized:
\Drupal\experience_builder\ComponentMetadataRequirementsChecker::check()

    foreach ($metadata->schema['properties'] as $prop_name => $prop) {
      if ($prop_name === 'attributes') {
        continue;
      }

… which is the direction @penyaskito's in-progress MR already is taking 👍 Let's revive & land it!

It needs more investigation to figure what exactly causes this error.

The answer is AFAICT surprisingly simple: it's not valid JSON Schema! SDC uses a superset — see @e0ipso's comment at #3352063-14: Allow schema references in Single Directory Component prop schemas → .

The solution is ✨ How to pass content_attributes to SDC without it being shown in the XB UI Active , so marking this as a duplicate and tagged that as a beta blocker + contributed project blocker.

Note that this strongly relates to 📌 Only allow deleting Code Components if there's 0 usages Active , which is a stable blocker. @lauriii's at #3530058-8: Only allow deleting Code Components if there's 0 usages → connects very closely to what this issue is about!

It seems this is kind of a duplicate of ✨ Usage info for code components with unpublished changes Postponed (because it's hard to find).

@lauriii in #8:

What would be extremely useful is having a list of usages of the component.

We already have that infrastructure and information, since 📌 Calculate field and component dependencies on save and store them in an easy to retrieve format Active . We just don't have it in the modal dialog, which is what I suspect you're hinting at?

If so, that makes this even more closely related to ✨ Usage info for code components with unpublished changes Postponed !

Could we make the soft delete as an explicit feature?

Yes, but what does "explicit feature" mean? 👈 confused me even more, perhaps you meant "components"?

* **Links:** If the link's format is `uri-reference`, use `{ "uri": value, "options": [] }`; otherwise, use `href: "#"` unless the user explicitly requests a specific URL.

This is not actually correct: that's simply the set of properties used by the link field type, which is picked by these lines in \Drupal\experience_builder\JsonSchemaInterpreter\JsonSchemaStringFormat::computeStorablePropShape():

      // TRICKY: Drupal core does not support RFC3987 aka IRIs, but it's a superset of RFC3986.
      // TRICKY: the `uri` and `iri` prop types will only pass validation with absolute paths, so we
      // instead use the link widget which is more permissive about the URI/IRI content.
      // @see \Drupal\Core\Field\Plugin\Field\FieldType\UriItem
      // @see \Drupal\link\Plugin\Field\FieldType\LinkItem::defaultFieldSettings()
      static::URI_REFERENCE, static::URI, static::IRI_REFERENCE, static::IRI => new StorablePropShape(
        shape: $shape,
        fieldTypeProp: new FieldTypePropExpression('link', 'url'),
        // @see \Drupal\link\Plugin\Field\FieldType\LinkItem::defaultFieldSettings()
        fieldInstanceSettings: [
          // This shape only needs the URI, not a title.
          'title' => 0,
        ],
        fieldWidget: 'link_default',
      ),

There's absolutely no guarantee that this field type is actually used: modules/sites can choose to deviate from XB's default choices for a particular prop shape — see hook_storage_prop_shape_alter(). They could switch to the uri field type or something else altogether.

Changing status for creating the follow-up.

Thanks, @mayur-sose!

Feels like a major milestone to say goodbye to these SDCs that we've all interacted with a million times — but they're not gone, they're just boxed up now 🤓

📌 Allow CMS Author to set site's homepage from navigation Postponed is in. That's the last big one.

Merged in upstream, this should still be green 🤞

tedbow → credited wim leers → .

🚀

Time to create those 3 follow-ups (or do some of them in a follow-up MR on this issue, whichever is easiest, @hooroomoo!) — hence instead of .

Thanks all, and especially @mglaman! 🙏😊

The bold bits of #71 which required changes from me have passed automated tests, and work fine during manual testing 🥳

Then simplified, and now this is IMHO ready to go from a BE POV. Approved.

Favoring progress over perfection, let's handle the identified follow-ups in follow-up issues:
mine (#71)

• @jessebaker's (#70)
• @larowlan's (https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... + https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...)

Theirs are optimizations/clean-ups, mine is a data integrity issue that cannot yet occur in HEAD 👍

@jessebaker agreed, so … merging as soon as CI (pipeline #100!!!) is green 😊

• Thanks @tedbow for creating the follow-up 📌 Create StagedConfigUpdateValidationTest for StagedConfigUpdate config entity type Active 🙏
• Thanks @mglaman for adding the missing docs in docs/config-management.md — Succinct, and beautifully explains the rationale 😊
• 📌 Add validation constraints to system.site Active would allow us removing one hack from XB now, so tagged that issue. (Thanks @mglaman for linking!)
• @tedbow's move to own controller to make it clear it acts differently then other auto-save config entities commit was a great call 👍
• Thanks both of you for the clarifications to the code, and for addressing @lauriii's & @larowlan's remarks.
• I don't understand why we'd require StagedConfigUpdate::ADMIN_PERMISSION to be able to set the homepage at all? Is it simply because of pragmatism, because \Drupal\Core\Entity\EntityAccessControlHandler::checkCreateAccess() is called with zero context?

  Turns out … that that "contextless" thing turns out to be the reason @mglaman did it this way, per his MR comment, but turns out that there is a way! 😄 Yay for core's super abstracted nature 🤓

  Raised this with @lauriii. He didn't realize that/missed that in his manual testing in #63. He agrees what's in HEAD is very confusing for users.

  So: implemented a solution 😊👍

Only one concern, not commit-blocking:

1. We currently use this only for updating the system.site simple config. But the StagedConfigUpdate infrastructure supports targeting any config entity. system.site will always exist, because the System module is one of the modules Drupal cannot run without. But if other simple config is targeted and the providing module is uninstalled, or a target config entity is deleted … then the garbage values in AutoSaveManager will continue to exist!
   So: we need another follow-up to expand what \Drupal\experience_builder\AutoSave\AutoSaveManager::onXbConfigEntitySave() does (in the case of target config entities getting deleted) and we need to implement hook_modules_uninstalled() to delete StagedConfigUpdates targeting uninstalled modules' simple config.