[later phase] Default props values may introduce additional config dependencies

Here's a patch that updates the existing test coverage to use the text field type instead of the string field type and it does not cause a test failure. That proves that Component::calculateDependencies() is missing some additional logic.

As the issue summary says: Look at how Drupal's existing Field UI handles this.

FYI: I marked this for a later phase because this is IMHO low-risk. It's a known TODO, with a known solution, in Drupal core.

Sibling follow-up issue 📌 [PP-1] Default props values should support files/images Postponed is much riskier, so is not marked for a later phase.

✨ Allow specifying default props values when opting an SDC in for XB Fixed is in.

Wim Leers → credited catch → .

Sibling issue: 📌 `StringSemanticsConstraint::MARKUP`: agree how SDC prop JSON schema can convey it should be markup, and Needs review .

If #3467959-35: SDC and code component props should be able to receive HTML, editable in CKEditor 5 → is considered acceptable, we won't have to deal with the FilterFormat scenario described in this issue.

But … the root problem remains. See the example of XB's image SDC:

uuid: 1574ac0a-b1b6-4b32-aaef-a15c18d95aa0
langcode: en
status: true
dependencies:
  module:
    - media_library
label: Image
id: sdc.experience_builder.image
provider: experience_builder
source: sdc
category: Other
settings:
  plugin_id: 'experience_builder:image'
  prop_field_definitions:
    image:
      field_type: entity_reference
      field_storage_settings:
        target_type: media
      field_instance_settings:
        handler: 'default:media'
        handler_settings:
          target_bundles:
            image: image
      field_widget: media_library_widget
      default_value: {  }
      expression: 'ℹ︎entity_reference␟{src↝entity␜␜entity:media:image␝field_media_image␞␟entity␜␜entity:file␝uri␞␟url,alt↝entity␜␜entity:media:image␝field_media_image␞␟alt,width↝entity␜␜entity:media:image␝field_media_image␞␟width,height↝entity␜␜entity:media:image␝field_media_image␞␟height}'

… while this contains

dependencies:
  module:
    - media_library

That is inadequate, it should have:

dependencies:
  config:
    - media.type.image
  module:
    - media_library

(due to its field_instance_settings)

IOW: \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\GeneratedFieldExplicitInputUxComponentSourceBase::calculateDependencies() must be updated.

📌 Calculate field and component dependencies on save and store them in an easy to retrieve format Active is making this a lot simpler.

📌 Calculate field and component dependencies on save and store them in an easy to retrieve format Active is in, I'll need to double-check what remains here.

This is still necessary.

The root cause is StaticPropSource::calculateDependencies():

  /**
   * {@inheritdoc}
   */
  public function calculateDependencies(FieldableEntityInterface|FieldItemListInterface|null $host_entity = NULL): array {
    assert($host_entity === NULL || $host_entity instanceof FieldableEntityInterface);
    // The only dependencies are those of the used expression. If a host entity
    // is given, then `content` dependencies may appear as well; otherwise the
    // calculated dependencies will be limited to the entity types, bundle (if
    // any) and fields (if any) that this expression depends on.
    // @see \Drupal\Tests\experience_builder\Kernel\PropExpressionDependenciesTest
    return $this->expression->calculateDependencies($this->fieldItemList);
  }

… this is only looking at the field_types involved in the expression.

We need this to be expanded, to do something like \Drupal\Core\Field\FieldConfigBase::calculateDependencies(), which does:

    // Let the field type plugin specify its own dependencies.
    // @see \Drupal\Core\Field\FieldItemInterface::calculateDependencies()
    $this->addDependencies($definition['class']::calculateDependencies($this));

… which would call \Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem::calculateDependencies() and hence in turn solve this.

Still needs expanded test coverage.

AFAICT this will need a new ComponentSourceInterface::calculateSettingsDependencies() method; akin to \Drupal\Core\Field\FieldItemInterface::calculateStorageDependencies().

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

While this could theoretically be retroactively fixed, this is the last "missing dependency" issue to my knowledge! So with this in done, XB's usage information would actually be completely accurate, and could prevent painful surprises/brokenness (by preventing uninstallation).

Also: 📌 Version component prop definitions for SDC and Code components Active will quite cause not 1 but multiple versions to exist of the prop_field_definitions. So: it'd be good to have this fixed before that, to ensure nothing is overlooked.

So: tagging .

Do we need to update `\Drupal\experience_builder\PropSource\DynamicPropSource::calculateDependencies` or create another issue for that? Maybe this will become obvious as I review this more. But thought I would put it here not to forgot or if someone else knows

#26: no, that is not necessary. I see that PropSourceTest::testDynamicPropSource() doesn't have an explicit example, but PropExpressionDependenciesTest::testCalculateDependencies() does already test that e.g. a DynamicPropSource using:

• ℹ︎␜entity:node:article␝field_image␞␟value results in a calculated dependency on field.field.node.article.field_image
• ℹ︎␜entity:node:article␝field_image␞␟{src↝entity␜␜entity:file␝uri␞␟url,width↠width} also results in a calculated dependency on field.field.node.article.field_image

• Big thanks to @tedbow for carefully reviewing @nacho's refactor towards a data provider! 🙏
  
• Follow-up created for (pre-existing) bug surfaced in this MR's test coverage: 🐛 SdcPropKeysConstraintValidator::validate() should complain about extraneous keys too, not just missing keys Active .
  
• My proposed resolution in #19 was pointing to the right thing, but was overkill, which @isholgueras identified 👍 No interface expansion, just an addition to StaticPropSource 🥳

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