Revisit storage of dependencies in separate table now we have separate deltas per component

larowlan → credited catch → .

Thanks, @catch and @larowlan!

🤔

I was thinking this would be less preferable, because it would require to query not 1 table when a dependency is removed (config deleted, module uninstalled …), but N tables.
With N being easily dozens of tables:

1. F (number of XB fields), which is currently up to 1 per content entity type, bundle pair
2. D (DB tables per field): 2 aka current data table + revision table
3. E (currently none, but if we end up doing one field per exposed slot too, then it could easily become 10 times as many — assuming 10 exposed slots)

I would like nicely normalized, self-contained data too, but it seems like the consequences here are quite big?

@catch Is your concern still that the the data in the current single stand-alone “dependencies” table can get out of sync?

When looking into this I realised we missed 🐛 \Drupal\experience_builder\Plugin\DataType\ComponentInputs::getPropSources needs to take \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\GeneratedFieldExplicitInputUxComponentSourceBase::rawInputValueToPropSourceArray into account Active and will fix that tomorrow

I think because prop sources can have dependencies, we might not be able to do this and may need to retain the database table.

So for \Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem::calculateFieldItemValueDependencies we call \Drupal\experience_builder\Plugin\DataType\ComponentInputs::calculateDependencies which in turn calls \Drupal\experience_builder\PropSource\ContentAwareDependentInterface::calculateDependencies on these prop sources.

Now we know the inputs column on field items cannot contain dynamic or adapted prop sources, so we don't have to consult them for their dependencies with content entities. The static prop sources we know about from the prop field definitions on the config entity and can hence consult them (but currently do not).

But they can contain default-relative-url prop sources however at present these just depend on the component, so we already have that.

With config entities, we can call ::calculateDependencies and it will compute this (i.e what catch proposed in the issue).

So I think in theory this is possible but it probably warrants a spike to see what the performance difference/impact is.

@catch Is your concern still that the the data in the current single stand-alone “dependencies” table can get out of sync?

Multiple concerns to be honest:

1. Even if it's updated in a transaction when the entity is saved, the dependencies of configuration entities can change in the background. One example would be a field plugin moving from one module to another. No-one is going to manually update the XB dependencies table if they move a field plugin between two custom modules. Another example would be a plugin rename. Think of the groups 1/2/3 update where group_content was renamed to group_relationship for one high usage contrib module that did major surgery to entity type and plugin names.

2. The denormalization means duplication of data, which means bigger databases.

3. Any other change to how config dependencies are calculated (like something in XB itself, or even in the core config system if we found a bug with dependencies not being added correctly, which we have before) would require updating the table too.

4. Longer term, but I think it will eventually conflict with ✨ [meta] Add database driver for MongoDB to Core as experimental Active (and the contrib driver), whereas entity queries would not.

On the multiple tables:

#4.D isn't an issue, because revision tables always also include the values for the current revision - so for this kind of audit operation, only the revision tables need to be checked.

#4.E I think multiple fields on a bundle could be done with an OR condition in a single query.

If we only need to prevent uninstall of a module or deletion of a configuration entity, we can return early as soon as a single record is found without querying additional tables anyway, although it would limit the detail in the validation message.

Also it does not seem hugely likely to me that many, many different entity bundles would use the xb field, more like 2-3 usually? XB for layouts/content templates obviously a lot more than that, but that doesn't require the field.

Even if it's 50 database queries, if those are well indexed, that's 50 or 100ms, so for rare audit operations, it should be OK - but I really think we're talking about 1-10 queries on the vast majority of sites. We're talking about something that linearly scales with the number of fields, but should still be very efficient regardless of the amount of content - as long as there's an index on the columns we need to check like component (this would be something else to check regardless of this issue, for update paths etc. too)

I think if 📌 [PP-1] Consider not storing the ComponentTreeStructure data type as a JSON blob Postponed had been done earlier, we'd already be doing entity queries for this validation and 📌 [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table Active would never have come up as a proposal.

@catch in #7:

1. field plugin moving from one module to another

   Never even thought of this edge case 🤯

2. denormalization means duplication of data, which means bigger databases.

   There's very little duplication here? Only the Component config entity dependency plus identifiers (entity type ID, bundle, ID, revision ID, language).

3. True. But that would be true both with a separate DB table (HEAD) and if it's extra columns in the XB field rows (this issue)?
4. 🤔 I don't understand this one. Can you elaborate?

RE: multiple tables #4.D+E → makes sense, I stand corrected 🙈

it does not seem hugely likely to me that many, many different entity bundles would use the xb field, more like 2-3 usually

AFAICT @lauriii expects/envisions there being no more EntityViewDisplay admin UI anymore, and they'd instead each get an XB ContentTemplate — although many would likely have zero exposed slots, which … means I think you're right! 😄

That on its own changes the equation a lot. 🤔 That means #4.F would never be very high.

@larowlan in #6:

cannot contain dynamic or adapted prop sources,

dynamic only really makes sense in ContentTemplates, but I think @lauriii has expressed he wants to keep this possible in the future
adapted definitely was intended to become supported, both for

• adapting a static prop source (for example a type: integer plus UnixTimestampToDateAdapter to result in a type: string, format: date — see that class in HEAD),
• adapting 2 static prop sources (for example a type: string, $ref: "json-schema-definitions://experience_builder.module/image" plus something like 'type' => 'string', '$ref' => 'json-schema-definitions://experience_builder.module/config-entity-id' to represent an image style config entity, which are then combined in ImageAndStyleAdapter — see that class in HEAD)

  I think because prop sources can have dependencies, we might not be able to do this and may need to retain the database table.

  I don't understand why it would not be technically possible? I expressed in #4 it'd be , but I do think we'd be able. The stored prop sources must be able to be reconstructed, so we should be able to call the methods to compute the dependencies again as you described.

  What am I missing? 🤔

  @larowlan in #5: Nice catch! 😅

Per the above discussion, and having a call with @catch: this is already tagged , but now I'm going to bump this to as well.

Because if we were to do this late(r), this would require a painful update path. So, as @catch wrote at the end of #7: this is kind of going back to 📌 [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table Active , and undoing 📌 [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table Active 😅, but this time there'll be more existing test coverage to make that refactor likely painless 😊 (And let's make sure to add the right DB indexes too.)

Opened a draft MR to show what this might look like.

I think it works fine for 'find which content uses this component' which is the changes to ComponentAudit class (tests are passing here).

I think it falls over for field type uninstall validator with #6

The things that are ugly are:

• config entity queries that know the shape of settings (prop_field_definitions) inside component sources that extend from \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\GeneratedFieldExplicitInputUxComponentSourceBase, the changes to ::calculateDependencies on that class and its child. And the changes to ComponentFieldItem in that test.

But I think the issue there might be the way we're implementing that uninstall validator - specifically the requirement to have it tell you which fields are being used.

The uninstall validator interface runs off module name (not field name/type) so we can simplify a lot of that code to just look for components that have dependencies on that module _of any type_ not necessarily from a field type POV

If we can accept that simplification and just let it prevent uninstalling the module (which is the root reason) rather than saying which fields are in use - and just say

$reasons[] = new TranslatableMarkup(
          'Is required for component type %component, that is in use in the content of the following entities: %entity_type id=%entity_id revision=%revision_id',
          [
           '%component' => $component_id,
            '%entity_type' => $entity_type,
            '%revision_id' => $revision_id,
            '%entity_id' => $entity_id,
          ],
        );

Rather than the current message

$reasons[] = new TranslatableMarkup(
          'Provides a field type, %used_field, that is in use in the content of the following entities: %entity_type id=%entity_id revision=%revision_id',
          [
            '%used_field' => $field_definition['id'],
            '%entity_type' => $entity_type,
            '%revision_id' => $revision_id,
            '%entity_id' => $entity_id,
          ],
        );

I think that ugliness will go away.

Thoughts?

Edit I also think that lets us get rid of the concept of collecting dependencies of type plugin which core doesn't support. So that would also be a win in my opinion. Unless we want to keep that to allow for the edge case of a plugin going missing, but core doesn't have any handling for that and it could easily happen, I think that scenario we can use the render-safe-container to prevent breakage.

Thanks! Will review 😊

The plugin dependency information is for more than just field types IIRC. It’s also for anything else plugin-based, such as SDCs.

IIRC it is indeed about being able to inform the Drupal user precisely what the problem is: not just “module X” is gone (which might be meaningless), but “plugin X is gone” (which more precisely describes the impact).

But I’ll need to check by reviewing + reading the code.

I definitely agree it’s less than ideal that XB pushes beyond what core offers here 😔 I remember feeling strongly that this is the right call for the end user, but perhaps … enough has changed, or I was wrong! 😇

Have a lovely weekend!

But I think the issue there might be the way we're implementing that uninstall validator - specifically the requirement to have it tell you which fields are being used.

The uninstall validator interface runs off module name (not field name/type) so we can simplify a lot of that code to just look for components that have dependencies on that module _of any type_ not necessarily from a field type POV

This makes sense to me, if you know exactly what you're doing and you really want to uninstall that module, then you need to either find content using the component and delete it (the field delta, or the entire entity if it's test content or similar). Or you'd need to audit the constituent parts (components?) of the component and update it somehow to remove the dependency so it can be uninstalled, which is always going to be a non-trivial developer task.

But the more likely outcome here is 'whoops, better not uninstall that module then, forgot I was using it for this, thanks for telling me'.

IIRC it is indeed about being able to inform the Drupal user precisely what the problem is: not just “module X” is gone (which might be meaningless), but “plugin X is gone” (which more precisely describes the impact).

I'm not sure about this.

The uninstall validator is more about 'you can't uninstall module x yet because your content depends on it', and regardless of what the module is providing, that should prevent 'x is gone' - because it never gets to the point where it's gone.

At the moment it looks like XB is allowing plugins to go missing via Component::onDependencyRemoval (which we briefly discussed yesterday) when modules are uninstalled. I just opened 🐛 Component::onDependencyRemoval() should be an uninstall validator Active for that.

If modules are always prevented from being uninstalled when they provide a component dependency, then the only case a plugin can go missing is when a developer just deletes it. But the whole problem with that is there is no actual event to listen to to run any of this code, it's there one second, then completely missing the next. So to inform the user about that is more of just an error message when they're viewing content, and the plugin ID is there but the plugin itself is not any more, which I think is what 💬 ComponentNotFoundException: Unable to find component Active is for.

But in that case the missing plugin ID must be in the config somewhere, so separate content audit page or similar could do something like go through the config entities, look for plugin IDs that don't correspond to a plugin, and then give you a list of broken components - and then you can use the queries here to find where those components are used.

@larowlan in #11:

The current message from FieldTypeUninstallValidator dates back to >1 year ago (June 7, 2024, in 📌 Prevent modules from being uninstalled if they provide field types used in an Experience Builder field Fixed ), and predates the Component config entity type (introduced June 10, 2024, in 📌 "Developer-created components": mark which SDCs should be exposed in XB Fixed ). That simple fact alone suggests #11 is right to suggest that we can simplify things.

However, it does come with reduced quality/precision of information. The same will be true for all other plugin dependencies. All the ones currently in use by XB: entity_type:<plugin ID>, adapter:<plugin ID> and of course the one we're talking about here: field_type:<plugin ID>.

On the other hand, it would mean that we're deviating less from what core does.

@catch in #14:

(Responded on 🐛 Component::onDependencyRemoval() should be an uninstall validator Active and asked @larowlan to provide extra context there.)

At the moment it looks like XB is allowing plugins to go missing via Component::onDependencyRemoval

The intent definitely has never been to allow this. The intent has always been to detect, inform and gracefully degrade when this happens.

For example: a module update may cause an SDC or a block plugin to disappear! In neither case can config dependency validation logic nor extension uninstallation logic help. IOW: this is all foundations/metadata to allow for https://en.wikipedia.org/wiki/Robustness_principle throughout — the first/next thing that comes to mind is 💬 ComponentNotFoundException: Unable to find component Active … which is also what you point to! 👍

But in that case the missing plugin ID must be in the config somewhere, so separate content audit page or similar could do something like go through the config entities, look for plugin IDs that don't correspond to a plugin, and then give you a list of broken components - and then you can use the queries here to find where those components are used.

🤔 Not sure I follow. Yes, we use the PluginExists constraint, which will work for SDCs disappearing, block plugins disappearing etc. — in short: any plugins that Component config entities (or other config entities) depends on 👍

But … that won't work for StaticPropSources, because they're stored as JSON blobs (or nicely formatted JSON blobs in the case of config since 📌 Stop storing JSON blobs in XB config entities: improve DX Active !) that happen to also rely on a field type plugin. And StaticPropSources can exist for any component tree: both config-defined ones (currently in Pattern, PageRegion and ContentTemplate) and in content-defined (currently on the Page content entity type and on the article node type).

The solution you outline AFAICT works only for plugins depended upon by Component config entities? 🤔

I'm not quite following the last two paragraphs of #15. What plugins do the JSON blobs for StaticPropSources depend on that aren't dependencies of the Component config entities?

Possibly related to #16, if Component config entities have the config schema type of versioned_config_entity, what happens if an old version has a dependency but the active version doesn't have that dependency? Does the dependencies of the Component config entity include only the dependencies for the active version, and if so, is there anywhere that tracks the dependencies of old versions (for which there might still be stored json blobs)?

@effulgentsia in #17: I asked myself that same question when merging 📌 Version component prop definitions for SDC and Code components Active , and self-answered it in my last comment on that issue → .

A challenge is though that the dependency information does *not* distinguish between component versions. The need for that version-specificness in dependency information → came up also yesterday at #3501708-36: Prove that it *will* be possible to apply block settings update paths (assuming #3521221 in core) to stored XB component trees in config/content → .

I'm also not clear what wouldn't be covered by the component config dependencies per the question in #16, how would anything else get in there?

On versions: I would have thought the versioned component dependencies would be based on all the versions. Once a version is removed, dependencies would be recalculated and anything unnecessary dropped then. Anything more granular might tell you that there's a specific version that's adding the dependency, but for data integrity the whole config entity is the important thing to not delete.

This mismatch in expectations is why I pointed that out as an oversight in #3523841-69: Versioned Component config entities (SDC, JS: prop_field_definitions, block: default_setting, all: slots for fallback) + component instances refer to versions ⇒ less data to store per XB field row → . So I'll get that done, to help get this issue/MR on track, because it'll help us make decisions.

Went ahead and merged MR1126 (see my ℹ️ comments on it), because it records the current reality.

Now that the current reality is covered by explicit tests, let's discuss whether to stick to it or to change it 👍

My proposal

• keep the current reality working as-is
• … but harden it: Component::loadVersion() should erase all (non-enforced) dependencies in $this->dependencies, to ensure the correct dependencies are always visible; this would result in make the weird explicit calculateDependencies() calls in ::testComponentAutoUpdate() obsolete

My assessment of that approach:

• Pro: the dependency information continues to be very precise, and informs us exactly which content entity revisions depend on what — specifically, it makes the dependencies of the used component versions queryable
• Pro: … which in turn makes it easy to provide an "upgrade stale component instances OR delete stale revisions" UI for 📌 [later phase] When the field type for a PropShape changes, the Content Creator must be able to upgrade Postponed
• Con: ::getDependencies() works differently than other config entity types — but then again this is the first versionable one
• Con: the current approach (aka HEAD) kinda breaks config dependency management?

I think that last con can be fixed quite easily by decorating the config.manager service with a class that only overrides ::getConfigDependencyManager(), which would return not \Drupal\Core\Config\Entity\ConfigDependencyManager, but a \Drupal\experience_builder\Entity\VersionedConfigEntityInterface-aware alternative.

Alternatively, I could see a hybrid approach working, where we eliminate both cons but keep both pros, by either:

1. adding a ::getVersionSpecificDependencies() method, and ensure that XB calls that where appropriate instead of ::getDependencies()
2. making ::getDependencies() behave differently if and only if ::loadVersion() has been called — but this feels like magic

My concerns about storing dependencies per delta (aka ComponentTreeItem aka component instance)

Having dug into this, I see multiple concerns with #14 and MR1108.

P.S.: also, in the future, XB wants to support not just versioned Components but versioned ConfigTemplates and … versioned all sorts of config.

Pro: the dependency information continues to be very precise, and informs us exactly which content entity revisions depend on what — specifically, it makes the dependencies of the used component versions queryable

Except that if e.g. a plugin ID provider moved from one module to another per #7.1, this can be updated in the configuration entity once, but it can't/won't be updated in the dependency table, leading to things getting out of sync.

Say I have a contrib submodule that provides a plugin, experimental or something, I now want to merge that new plugin into my main module, so I can make the submodule obsolete, keeping the plugin ID the same. To update config, I would only need to load and resave config entities to get the plugin dependencies recalculated but no other changes.

In the above situation, if the dependency table is out of sync, it will prevent uninstall of the submodule that the contrib author is trying to remove - and it's something they should know nothing about because we already have a configuration entity system.

Pro: … which in turn makes it easy to provide an "upgrade stale component instances OR delete stale revisions" UI for

It's not clear to me why it makes it easier, the way I would see a bulk update UI/drush working is something like this;

1. Find the component config entities that have multiple versions.

2. Find the entity revisions that reference the old component version hashes in the component_version column.

3. Batch update those revisions to convert them to the current version.

For this you only need to know that a revision/delta is using an older component version hash, you don't need to know why it is. Also the versions themselves should be able to provide this information if it's needed - it's OK to be loading loads of component config entities to find this out on a specialist audit page/drush command.

Con: ::getDependencies() works differently than other config entity types — but then again this is the first versionable one

I think it should be as close to other config entities as possible, e.g. so that 🐛 Component::onDependencyRemoval() should be an uninstall validator Active can detect when a module that someone is attempting to uninstall is used in 10,000 older component config entities, even if it's not being used in the current version. Every time it deviates, it will result in special cases or brittleness.

The crucial thing to not forget is that while it's possible (and most common today) to populate a component's inputs (props/settings/…) with StaticPropSources powered by field types, that is not guaranteed to be the case. We know we'll want to support AdaptedPropSources, and it's conceivable to want to support something like a RemoteApiPropSource (e.g. to make the server fetch live weather information and pass it to the component while rendering). So, assuming that a content entity's XB field's component tree depends on a set of Component config entities because those happen to contain the dependencies of the StaticPropSources is insufficient, because it's not required that an instance of a component uses a StaticPropSource!

But there will still be a component config
entity, so whatever module is providing the RemoteApiPropSource can be added as a config dependency of that component, no?

The end-user functionality points all depend on the answer to my second point above I think - e.g. why isn't the component version field sufficient for this?

For this you only need to know that a revision/delta is using an older component version hash, you don't need to know why it is.

Good point! 😃 I didn't yet have that insight yet: .

But I was thinking more broadly: I was assuming that batch updates are either not pragmatic or impossible. Specifically, I was thinking about being able to directly query/fetch all content entiy revisions using component instances using component versions that happen to depend on some particular module or plugin. In the world you describe, that's still possible (*asterisk*), but requires first iterating over a Component config entity's versions, compute its dependencies, check if it's in them, and then use the matching versions in a query such as SELECT * FROM … WHERE version IN ($match1, $match2, …).

But there will still be a component config entity, so whatever module is providing the RemoteApiPropSource can be added as a config dependency of that component, no?

⚠️ No!

(Also, this is where the above *asterisk* comes into play.)

Because a Component config entity's prop_field_definitions (used only for SDC and JS components) describes only the default way for populating each prop using a field definition — there's nothing that dictates that the component instance MUST be populated by this field type.

That's why an instance of the same Component can use a StaticPropSource 99% of the time, except in some ContentTemplate where it'll use DynamicPropSource. Or even an AdaptedPropSource, which can combine StaticPropSources and DynamicPropSources. See \Drupal\Tests\experience_builder\Kernel\Plugin\Field\FieldType\ComponentTreeItemTest::testCalculateDependencies() for an example that contains the whole spectrum.

That is why I've been saying that the dependency information on Component config entities alone is insufficient. And I think \Drupal\Tests\experience_builder\Kernel\Plugin\Field\FieldType\ComponentTreeItemTest::testCalculateDependencies() pretty clearly answers #16.

That's why an instance of the same Component can use a StaticPropSource 99% of the time, except in some ContentTemplate

But that's configuration, not field data?

That was just an example. Nothing technically prevents us from using different sources for XB fields (in content entity revisions). We just haven't built the UX yet.

@lauriii has previously indicated he wants to keep that possibility.

That was just an example. Nothing technically prevents us from using different sources for XB fields (in content entity revisions). We just haven't built the UX yet.

But that UX will require explicit action on behalf of the user that can then be reflected as dependencies being stored somewhere.

It's possible that 'somewhere' could end up being a custom database table with a similar structure to the current one - but then that table would only need to exist to handle these special/one-off dependencies, not duplicate all the config ones, with the potential to go out of sync when config is updated etc.

The current MR shows that effectively what #28 and the conversations are pointing at - is likely the end state here

i.e. retain the table for plugin dependencies and not config dependencies - which is what I was trying to indicate in #6 and #11

I will update the MR to to that.

I also think we probably want to do 🐛 Component::onDependencyRemoval() should be an uninstall validator Active because it will give us test-cases that reflect reality - i.e. plugins going missing (I have a plan for how to do that) rather than modules being uninstalled.

I'll work on both today.

Updated this to only deal with plugins. All the ugly bits are gone.

Discussed this MR and specifically this bit at length with @catch:

i.e. retain the table for plugin dependencies and not config dependencies - which is what I was trying to indicate in #6 and #11

We agree that:

1. Such a table will be necessary XB adds support for creating/using DynamicPropSources and AdaptedPropSources in content entities. ⚠️ Right now, XB doesn't use either of those anywhere, except DynamicPropSources in the ContentTemplate config entity type. But for config entities, this is not an issue, because there config dependencies solve it for us :)
2. It will be able to use the exact schema we've been using so far.
3. … but for now we should remove this table + service, until it is actually needed. Because it's not guaranteed that that functionality will ever actually be built.

That way, we keep things pragmatically simple, but we know with a high degree of confidence that this approach will work, because … it's what we used to use.

IOW: we assume:

1. XB use in content entity revisions always references a Component revision
2. Component revisions have specific dependencies

Also discussed "Component config entity should have dependencies for ALL versions or not" with @catch. We agreed that it SHOULD contain all dependency info for all versions, because that's standard config dependency management.

We agreed that there should be a way for retrieving version-specific dependencies too, which XB can then use when e.g. performing (usage) audits and applying update paths. Something like this:

  public function calculateDependencies() {
    $this->dependencies = [];
    foreach ($this->getVersions() as $version) {
      // Load each version and compute its dependencies
      $this->addDependencies($this->getVersionSpecificDependencies($version));
    }
    return $this;
  }

  public function getVersionSpecificDependencies($version): array {
    $current_dependencies = [];
    $current_version = $this->getLoadedVersion();

    $this->loadVersion($version);
    parent::calculateDependencies();
    $deps = $this->dependencies;

    // Restore original state.
    $this->loadVersion($current_version);
    $this->dependencies = $current_dependencies;

    return $deps;
  }

Thanks, should we tackle #31 here or do we have a separate issue for that? Assuming here is fine.

This comment https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... confirms we're doing it here 👌

#32: I see that @catch and I didn't describe that very clearly — sorry 😅

• Definitely here: removing the separate xb_component_tree_dependencies table.

  ⇒ The "pre-<hr>-bits" in #31 describe why for the foreseeable future we simply won't need that.

• Also definitely here: making the stored representation of Component config entities reflect the full set of dependencies across all versions.

  ⇒ The "post-<hr>-bits" in #31 describe how @catch and I believe this could be achieved quite simply & elegantly (unless we overlooked things 😇)

  NOPE, you're right, because without that done this won't work:

      $dependencies = $this->configManager->findConfigEntityDependencies('module', [$module]);
  

  👍

Implemented #31 (With some modifications, pseudo code etc) and updated the tests.
Removed the separate table and services and updated the usage in FieldTypeUninstallValidator and associated tests.

This is green and ready for review again.

12 files, +123, −1031 🤠

And … I wanted to be very careful as I merged this crucial MR, and in doing so, I realized that a single place remained that relied on XB's "plugin dependency" concept: FieldTypeUninstallValidator's remaining half. But that too seemed … questionable now. See my observations on the MR.

I double-checked this with @catch, and he too thought that all of this is now obsolete.

The crucial thing is that thanks to

1. improved aka actually correct config dependencies as of very recently
2. combined with the key assumption described at the top of #31
3. combined with this MR changing the config dependencies of the (versioned!) Component config entity type now reflecting the UNION of all dependencies of all versions instead of only dependencies of the active version

Those 3 facts together … 👉 XB can now completely on config dependencies! 🥳 That's why the uninstall validator is obsolete.

I verified this through manual testing, by doing:

$ git diff
diff --git a/experience_builder.info.yml b/experience_builder.info.yml
index 76bf4e360..f2e3f9b7b 100644
--- a/experience_builder.info.yml
+++ b/experience_builder.info.yml
@@ -20,7 +20,7 @@ dependencies:
   # @see https://json-schema.org/understanding-json-schema/reference/string#dates-and-times
   # @see \Drupal\datetime\Plugin\Field\FieldType\DateTimeItem
   # @see \Drupal\experience_builder\JsonSchemaInterpreter\JsonSchemaStringFormat::computeStorablePropShape()
-  - drupal:datetime
+#  - drupal:datetime
   # To support the json-schema-definitions://experience_builder.module/image prop shape.
   # @see \Drupal\image\Plugin\Field\FieldType\ImageItem
   # @see \Drupal\experience_builder\JsonSchemaInterpreter\JsonSchemaType::computeStorablePropShape()

in XB, and

diff --git a/core/lib/Drupal/Core/Extension/ModuleInstaller.php b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
index 7d19ebb41bb..2e59fbb37a5 100644
--- a/core/lib/Drupal/Core/Extension/ModuleInstaller.php
+++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
@@ -651,6 +651,7 @@ protected function updateKernel($module_filenames) {
    * {@inheritdoc}
    */
   public function validateUninstall(array $module_list) {
+    return [];
     $reasons = [];
     foreach ($module_list as $module) {
       foreach ($this->uninstallValidators as $validator) {

in core.

Then, if I try to uninstall the datetime module (which provides the datetime field type) while no XB Component exists that depends on it:

but if I then install the sdc_test_all_props module (which exercises the full spectrum of prop shapes and hence shape-matched field types), we get this when uninstalling:

This, together with this MR's updated "version 1 → version 2 (with 1 still around) → version 2 (with 1 deleted)" test coverage in ComponentTest::testComponentAutoUpdate() that verifies config dependencies are updated correctly, means that we can delete XB's uninstall validator 🚀.

That, in turn, removes the need for "plugin dependencies" altogether, which will further sipmlify things!

Thanks to Lee's work on the MR, I was able to first reduce the scope of #3522199 at #3522199-3: Consider component trees in config translations when uninstalling the module providing a field type → , and then thanks to #38 and its commit, I was able to just now close that issue altogether 🎉

If we find that people still shoot themselves in the foot despite all this, we can restore something like this. But as @catch has pointed out repeatedly: it seems overkill and brings avoidable complexity — better to just rely on core's dependency management 👍

And … we actually discussed these same things a ~month ago (but a LOT has happened in that month, so I didn't recall 😇):

    // Config entities do not support plugin dependencies. (Plugin dependency
    // information is only necessary for component trees in content entities.
    // Because:
    // - Component config entities always exist in a single state
    // - content entity revisions contain component instances created based on
    //   the state at the time of creating the revision
    // This means that the Component config entity would prevent the
    // uninstallation of a module that provides a field type that is CURRENTLY
    // used for creating component instances, but that doesn't mean it
    // sufficiently protects HISTORICALLY created component instances!
    // @todo Consider removing this in https://www.drupal.org/i/3477428.
    if ($with_plugin_dependencies) {
      $dependency_types[] = 'plugin';
    }
    else {
      unset($dependencies['plugin']);
    }

… that's now gone, as indicated in the last sentence of #38 :)

Tagging because #40 addresses a concern @catch has been raising:

Except that if e.g. a plugin ID provider moved from one module to another per #7.1, this can be updated in the configuration entity once, but it can't/won't be updated in the dependency table, leading to things getting out of sync.

I love how much simpler the end result is!

When I started reviewing this MR in #38:

12 files, +123, −1031 🤠

Now: 23 files, +153, −1613 😊

Given that
Just added the missing bit: a way to actually inspect without having to look into the DB or analyzing your config by hand which versions of which components are used where:

… but @catch rightfully pointed out in chat that while prevention of (Component) config entities getting deleted accidentally is his main concern, XB should also prevent uninstallation (using an uninstall validator) when you have content entities that use Component config entities. That's why 🐛 Component::onDependencyRemoval() should be an uninstall validator Active is adding ComponentDependencyUninstallValidator.

Also just added the test coverage to prove this works correctly — and deferred the "usage of component revision by a config entity" to a follow-up: 📌 Update `ComponentAudit::getConfigEntityDependenciesUsingComponent()` to support component versions Active .

So: landing #3528723 first, I just first wanted to get this MR to an RTBC place because it's the one that actually affects the data model.

🐛 Component::onDependencyRemoval() should be an uninstall validator Active is in, and just proved (see #3528723-18: Component::onDependencyRemoval() should be an uninstall validator if the dependency being removed is a module or theme → ) it addresses @catch's concern → .

Better still … addressing the @todo that was added by 🐛 Component::onDependencyRemoval() should be an uninstall validator Active and pointed here … revealed that we can completely eliminate the "on dependency removal" complexity from ComponentSource plugins — and instead handle that complexity entirely in the Component config entity 🤓 🥳

Final tally: 33 files, +313, −1680. The additions are mostly increased test coverage and updates to the audit controller, resulting in the UI shown in #41.

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