Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings

I'll get a config schema going, to clarify the scope and envisioned architecture.

Similar to what I did at #3452397-8: Allow specifying default props values when opting an SDC in for XB: attached is the goal :) If more clarification is needed, I could create a first iteration of the config schema too.

@larowlan Yay! 😄❤️

@lauriii

configuration/UI

Yes, it changes site-by-site. One site would want to use the Image field type, another would want to use the Media field type restricted to the "Photo", "Screenshot" and "Flickr" media types. For { "type": "string", "format": "uri" }, Experience Builder would likely default to the uri field type, and almost no site would need to override this. But Experience Builder would also default to UriWidget (which is essentially a <input type="url">) and … quite a few sites would probably likely to swap this out for a different widget that allows typing keywords that search either the Drupal site itself, or the site's webshop, or Wikipedia, or something else still, to provide a better UX, tailored to the needs of Content Creators on that site.

There's probably some other scenarios to account for but does it warrant a whole UI for configuring all of this?

That is fair, and building the UI here could indeed be considered premature. But … we already have it, in ComponentEditForm/

Field type change

No, existing stored PropSources would not be changed. Because that would result in existing content becoming broken or uneditable. I'll document that in #3461490: Document the current JSON-based data model, and describe in an ADR. This is nothing new though — the same exact challenge you point out here already exists today with the Component config entity's defaults.props.<prop name>.field_type.

Default value

This is literally covered in the file attached in #3. See Inherit *most* of the default input experience for `text` — only override the `default_value`.. IOW: yes, I've thought about that 👍

Thought about this some more, and I think I see now where @lauriii was comping from.

• Because this new PropShapeInput config entity type didn't exist yet, #3452397: Allow specifying default props values when opting an SDC in for XB had no choice but to create a UI, because people working on the UI would not be able to test additional components otherwise.
• (Fortunately, it was able to leverage some of the work that I did for TwoTerribleTextAreasWidget.)
• But if this PropShapeInput config entity type had existed already, then #3452397: Allow specifying default props values when opting an SDC in for XB would likely not have built that UI.
• … because #3454094: Milestone 0.1.0: Experience Builder Demo won't need that, and even #3455753: Milestone 0.2.0: Early preview won't need it.

So: descoping that part. Even if it is "as simple as copy/pasting parts from ComponentEditForm" — it's extra time and probably there will be gotchas. Let's avoid them.

why allow configuring this outside the component at all?

Because SDCs are designed to have no dependencies (other than CSS/JS, and perhaps reuse Twig templates if the SDCs are part of a theme).

To be more precise: SDCs have no way to declare that a particular field type or field widget must be installed.

That's exactly why the \Drupal\experience_builder\Entity\Component was introduced in the first place: to capture that information in a structured way that can be A) shared from one site to the other, B) deployed.

This issue aims to simplify that, by reducing the amount of choices that must be stored by each individual Component config entity, and instead relaying that to this new PropShapeInput config entity. A dozen PropShapeInput config entities may be sufficient for all SDCs used in XB, the 95p case is likely a few dozen — for hundreds of SDCs.

If your implied point is "but radio/select/textfield/textarea are provided by Drupal "core" core (i.e. not even a core module), so why do we need to care about those dependencies at all?!", then I'd disagree — because A) it's totally reasonable to want to use contrib-provided field types/widgets, B) there are plenty of field types outside "core" core, but by core modules: from images to date range, and much more.

This is closely related to the discussion, you, @catch and I have going on in #3444424: [META] Configuration management: define needed config entity types.

As described at #3454173-19: Media Library integration (includes introducing a new main content renderer/`_wrapper_format`): I think doing this might unblock that directly, which would remove #3456008: [later phase] Support matching enum SDC prop shapes against DynamicPropSources, not only generating StaticPropSources from the critical path.

Hence increasing priority.

Can we allow them to declare that?

Not without fundamentally changing an architectural choice AFAICT.

It seems that we are regressing the DX in a non-trivial way due to constraints that we have within the system.

I do not see how new PropShapeInput config entities harm the DX of SDC developers? 🤔 In fact, it's the literal opposite: it means that SDC authors continue to not have to think about neither how their SDC's props would be stored, nor what the input UX would be?

[…] this feels quite edge casey for components.

😳 Paraphrased quote from the very first meeting I was in about XB: "bring in the entire existing Drupal ecosystem, including for example when a site owner has done significant investment in making the UX of (custom) field widgets great — throwing that away is not acceptable".

Is that no longer true then?

@lauriii is not yet convinced, but he's not available to discuss it further and hasn't responded here, so to A) get this moving, B) ground the conversation with @lauriii more, getting started on this nonetheless.

(Worst case: we throw this away, but it'll still inform #3462709: Introduce `hook_storable_prop_shape_alter()`, use it to prefer the Media Library widget for "image" PropShape if Media Library is installed 👍)

I started with a test that gathers all unique SDC prop schemas. Then I started writing logic to automatically suggest the sane core-provided field type choice.

In doing so, I started treading on the territory of #3456008 — and I believe that we now should not do that issue anymore, at least not in its original scope: #3456008: [later phase] Support matching enum SDC prop shapes against DynamicPropSources, not only generating StaticPropSources.

I am now thinking that not using a config entity type might actually be the only viable choice, because the config entity being proposed here only is viable if there is a good way to transform a given SDC prop schema into a PropShapeInput config entity ID — because that's how we could achieve sane dependencies.

The only reason that worked in the original proposal, is because HEAD does not yet support the {type: string, enum: …} use case, which is AFAICT widespread among SDCs. Once that is considered, the only way to generate a config entity ID is by hashing the enum.

The issue title + summary need a drastic update. See the new test class, which has 3 test methods that build on top of each other. I'll continue this MR tomorrow and will update this issue as well as #3450586: [META] Back-end Kanban issue tracker.

The changes in this issue now allow editing {type: string, enum: …}! 🥳 (confirming what I wrote in #15 about #3456008).

The \Drupal\experience_builder\Form\ComponentEditForm UX became simpler:

Before

After

Discussion with @lauriii

Yesterday morning, prior to writing #15, @lauriii and I met and discussed this problem space in detail.

The key outcomes of that discussion:

1. XB will require that an SDC provides an example value for each required prop for that SDC to be surfaced in XB. IOW: the examples array must not be empty, and XB will use the first one. ⇒ Removes the need for 25% of what the Component config entity provides.
2. @lauriii was heavily leaning towards "no UI" and "logic with an alter hook".
3. Tricky: complex shapes, like json-schema-definitions://experience_builder.module/image — those would need a plugin system to provide the appropriate field type + field storage.

This issue: current MR state and current consequences

This issue proves it's possible to generate the default field_type, expression and field_widget value for each prop. ⇒ Removes the need for 75% of what the Component config entity provides.

This issue did implement it all as logic (i.e. with nothing stored as a config entity — see my comment #15 for why that turned out to be trickier than I thought when I created this issue), with a TODO for an alter hook.

As demonstrated in #16, this means the scope for #3456008: [later phase] Support matching enum SDC prop shapes against DynamicPropSources, not only generating StaticPropSources changes to only having to worry about matching existing field instances that can be adapted to match the SDC prop's enum values.

Questions for @lauriii prior to landing this MR

1. Is it acceptable for the logic that determines which field type + widget to use to change and hence newly created instances of this component to potentially use a different field type than the pre-existing component instances?
2. SDC supports default *.component.yml-defined values. But that won't work for e.g. a default image. Is it okay for XB to layer on support for that by auto-discovering a propname.(gif|png|jpg|…) file in the SDC's directory? 🤔 (Will add this to #3462705: [META] Missing features in SDC needed for XB, to potentially add that to upstream SDC.)
3. Is it acceptable that the default value sourced from a component's examples cannot be overridden on a per-site basis?

I think the answer to the first question is "yes". (Note that every stored StaticPropSource is self-contained: the user will continue to be able to edit it, even if it's a completely different field type.)

If the answer to the second and third questions is "yes", then we can in principle remove the Component config entity. But that would introduce a huge risk: it'd make exporting/sharing configuration (in the future: #3455629: [PP-1] [META] 7. Content Templates — aka "default layouts" — affects the tree+props data model, but now: config/optional/field.field.node.article.field_xb_demo.yml, which has config dependencies on the Component config entities it uses) brittle.

Therefore I think it'd be preferable to keep Component config entities under the hood — without showing any of that in the UI. I think that would also keep the door open for supporting per-site default values in the future, plus it will likely help with #3454519: [META] Support component types other than SDC, block, and code components.

If you agree, then I'll create three follow-up issues:

1. one that generates Component config entities for all discovered SDCs 1) upon installing XB, 2) upon installing a new module or theme
2. one to remove \Drupal\experience_builder\Form\ComponentEditForm after that auto-generation is happening
3. one for computing the appropriate field type + storage for $ref-defined prop shapes

Oh, and once this lands, #3454173: Media Library integration (includes introducing a new main content renderer/`_wrapper_format`) becomes trivially unblocked: it'll only require:

1. new configuration to allow declaring a preference for image vs media (which oddly would not be possible through a new experience_builder.settings.yml config, only through a new config entity, because simple config cannot have module nor config dependencies)
2. updating \Drupal\experience_builder\SdcPropJsonSchemaType::findFieldTypeStorage() to respect that configuration

Those 2 things would be in an MR for that issue which I could start and then the front-end team could continue.

Alternatively, I could repurpose #3462709: Introduce `hook_storable_prop_shape_alter()`, use it to prefer the Media Library widget for "image" PropShape if Media Library is installed for that, so that #3454173 can stay focused on the bigger picture, of making that media library widget actually work (well).

Discussed #17 with @lauriii. His replies:

1. Yes, but needs follow-up to devise a UX to allow existing StaticPropSources using a previous field type+widget to update to the new one.
2. Yes
3. Yes

He agreed with the 3 follow-ups proposed at the bottom of #17.

We also discussed #18, and he prefers the "alternatively" — so repurposed that 👍

P.S.: Note that this also blocks #3463583: Use the `all-props` component for testing form backend + frontend integration.

This blocks #3455975 too.

See #3455975-5: HTTP API: update /xb-component/{component_id} to list possible prop sources for current entity context.

Follow-ups:

• #19.1: #3463996: [META] [PP-1] When the field type, storage/instance settings, widget, expression or requiredness for an SDC/code component prop changes, the Content Creator must be able to upgrade (and I realized it had a blocker itself: #3464003: [later phase] [needs design] Introduce "adapters" UX), and then the 3 at the bottom of #17:
• #3463999: Auto-create/update Component config entities for all discovered SDCs that meet XB's minimum criteria
• #3464025: Remove Component config entity's add/edit form
• #3464031: [later phase] Introduce `FieldTypeStorageForJsonSchemaDefinition` plugin type

  Bonus follow-ups: #3464036: Component config entity should validate that the SDC actually (still) exists + #3464042: Add ComponentAuditabilityTest.

  I'll continue working on this MR on Monday. Now: time for blog posts :)

The MR is green and updated to account for the slightly changed #3463999: Auto-create/update Component config entities for all discovered SDCs that meet XB's minimum criteria future 👍

Doing one final pass next.

Prioritizing this to, unblock many subsequent steps:

• front-end: #3463583: Use the `all-props` component for testing form backend + frontend integration (blocking @bnjmnm in particular)
• front-end: #3462709: Introduce `hook_storable_prop_shape_alter()`, use it to prefer the Media Library widget for "image" PropShape if Media Library is installed (blocking no one in particular, but this is critical for #3454094: Milestone 0.1.0: Experience Builder Demo!)
• back-end: #3463999: Auto-create/update Component config entities for all discovered SDCs that meet XB's minimum criteria 👉 @f.mazeikis should be back on XB tomorrow, and this will be an excellent fit

Per #23 and a thorough self-review: going ahead and merging. I thoroughly tested this, and can confirm that this unblocks #3463583: Use the `all-props` component for testing form backend + frontend integration, because with this MR I can get:

Note: it won't look like this once merged, due to a regression that was introduced in the last ~16 hours, but it should be an easy fix: #3465140: Regression: prop labels present on field widgets, but invisible.

In the MR, there's 4 @todos pointing to #3463999: Auto-create/update Component config entities for all discovered SDCs that meet XB's minimum criteria, which is where the automatic fallback logic that this added can be removed. "Automatic fallback logic" is a nice alternative phrasing for "magic 🧙", and we should avoid it. Removing that magic is an order of magnitude simpler once these component config entities are fully auto-generated. So, deferring that part to that issue 👍

Finally: #3462709: Introduce `hook_storable_prop_shape_alter()`, use it to prefer the Media Library widget for "image" PropShape if Media Library is installed exists (and after this MR is in, it'll be a crystal-clear change in a single place, rather than hacks sprinkled all over) to avoid having to wait for #3464031: [later phase] Introduce `FieldTypeStorageForJsonSchemaDefinition` plugin type — it's similar to #3463583: Use the `all-props` component for testing form backend + frontend integration in that it'll allow the people working on XB's front-end to see not just a few in XB's component props form in the right sidebar, but pretty much the full range of field widgets. That means they can start working on making them match the design (#3459259: UX design tracker), as well as adding tests to ensure that field widgets that use Drupal.behaviors actually work.

P.S.: this addresses a bunch of @todos, contains many more docs than the infrastructure it is expanding, and does update the diagrams. So it definitely leaves the code base in a better state than it was in :)

With that: 🚢

#24 contains the rationale for bypassing the usual review process.

This landing has actually resulted in 3 important areas getting unblocked:

1. #3463583-17: Use the `all-props` component for testing form backend + frontend integration → @bnjmnm fully unblocked, no more back-end work necessary there, he can merge when he pleases ✅ — which means the front-end contributors can now work on expanding what #3462441: Contextual form values need to be integrated with Redux: start with single-property field types introduced to more field widgets.
2. #3462709-8: Introduce `hook_storable_prop_shape_alter()`, use it to prefer the Media Library widget for "image" PropShape if Media Library is installed → I've gotten the Media Library Widget to render — but it won't work well until the necessary CSS + JS assets are loaded too.
3. I caught @f.mazeikis up on the last ~month of changes, and he's gotten started on #3463999: Auto-create/update Component config entities for all discovered SDCs that meet XB's minimum criteria 👍

Oh, and this also unblocked longer-term back-end issues in addition to #3463999:

• #3455975: HTTP API: update /xb-component/{component_id} to list possible prop sources for current entity context
• #3464031: [later phase] Introduce `FieldTypeStorageForJsonSchemaDefinition` plugin type
• (one less blocker) #3464025: Remove Component config entity's add/edit form

FYI, follow-up: #3465981: Follow-up for #3461499: transform at-rest field storage settings using FieldItemInterface::settingsFromConfigData().