Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
Account created on 26 December 2006, over 18 years ago
  • Senior Principal Software Engineer β€” Drupal Acceleration Team at AcquiaΒ  …
#

Merge Requests

More

Recent comments

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Just caught up on #3519352 β€” high-level responses at #3519352-38: Content templates, part 3b: store exposed slot subtrees on individual entities β†’ .

Makes me want to #22+++++++++++++ even more than I could've anticipated πŸ˜„ β€” as in: "do this first, but keep #3519352 in mind".

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

@larowlan in #21:

one about an edge case of a slot named '0'

Another reason to do πŸ“Œ Constraint slot names allowed by XB in its component tree Active ! πŸ˜„

@catch in #28: many of those concerns have been discussed in πŸ“Œ [PoC] Introduce a `ContentTypeTemplate` config entity Active and in … calls 😭. But they definitely don't have full/final answers yet. The idea was to make things more concrete by incrementally implementing, and identify unknown unknowns 🀞

I'm glad to see @lauriii already said as much in #29 😊

WRT deleting component subtrees (for this issue: of unused slots' subtrees): ✨ [later phase] Provide API for finding and UI for surfacing dangling/dead component subtrees β€” aka garbage clean-up Postponed β€” but also of defunct components, which is necessary because of πŸ› Cannot delete JS components due to component depending on them Active .

@phenaproxima in #32:

the frontend has absolutely no concept of "this tree will fit into a slot".

  1. That's intentional: docs/adr/0005-Keep-the-front-end-simple.md
  2. That's for later: ✨ Component restrictions Active
  3. Yes, the client side will need a new "node type" for its client-side data model β€” see 3.4 UI Data Model: communicating a `component tree` to the front end in docs/data-model.md β€” currently only 3 node types exist: component, slot and region. I do agree a new one will be needed for this. That's what 🌱 [META] Production-ready client-side data model + internal HTTP API Active exists for.

P.S.: I'm very tempted to make "bonsai trees" an official term in XB, so can you elaborate on why you used that term and your rationale? πŸ˜„

@catch in #33:

RE: information disclosure risks Yep, I raised exactly that concern. πŸ’―

The reason we have no choice but to allow this is that we know from the XB team members who've worked on https://github.com/acquia/cohesion (aka "Acquia Site Studio") for years that site builders will accidentally delete a slot, confirm through all the warnings, and then later realize that actually they do want to undo it after all.

RE: Wow β€” glad you caught that! Had not crossed my mind yet!

@larowlan in #37: see the validation+config schema added in ✨ Content templates, part 2: store a component tree in the template, and allow individual entities to fill in specific slots in that tree Active β€” exposed slots are intentionally only allowed for the full/canonical view mode. Because otherwise you get different component trees per view mode, which would be an authoring/usability nightmare. I first identified this in #3511366-3: [META] Introduce a `ContentTypeTemplate` config entity + related infrastructure β†’ , @lauriii responded at #3511366-7: [META] Introduce a `ContentTypeTemplate` config entity + related infrastructure β†’ .2.

Apologies, @phenaproxima, for having sent you down this path 😬 Turns out that this "incremental step" was fraught with more challenges than I anticipated. I expected it to need refactoring later on for sure, but I tried to unblock you on next steps for content templates. Turns out that was not quite realistic.

Per discussion with @effulgentsia, we think this is best tabled until after πŸ“Œ [PP-1] Consider not storing the ComponentTreeStructure data type as a JSON blob Postponed . @larowlan is actively working on spikes to help us make the best choice possible.

Agreed that an ADR is needed and appropriate, but during this read-through/catch-up, I did not end up with the impression there's only 2 possible choices as @lauriii indicates in #35 β€” I think there's many possible choices.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Added two missing things for the client-side data model:

  1. Missing "node type" in docs/data-model.md's 3.4 UI Data Model: communicating a `component tree` to the front end: a node type for exposed ContentTemplate slots: #3519352-32: Content templates, part 3b: store exposed slot subtrees on individual entities β†’
  2. Missing "node type" in docs/data-model.md's 3.4 UI Data Model: communicating a `component tree` to the front end: a node type for overrides/personalization variants
πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Is this still relevant?

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

πŸ“Œ Compile Tailwind CSS globally for code components Active recently landed.

Is this still relevant?

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

I've not seen/heard reports of people running into this for a long time. Tentatively deprioritizing.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

πŸ› The placement of the component preview is sometimes inconsistent Active landed yesterday and is kinda related.

I'd swear we fixed this a very long time ago. Is this still relevant? I bet @hooroomoo will know this off the top of their head! πŸ€žπŸ˜„

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Tagging per @lauriii in #4.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

This one would be best reviewed by somebody intimately familiar with both React, XB's use of React, Drupal behaviors and the AJAX system. Besides @bnjmnm that's only one person: @larowlan πŸ˜…

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

πŸ₯³ β€” thanks!

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

@tedbow, thoughts? :D

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Alright, unpostponing, but reflecting the lower urgency for this by reducing priority to .

Thanks, @lauriii!

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

The main thing I'm wondering is, once that lands, would we still need to define the format in the front end? If that's the case there should be explicit explanations of when that's necessary vs […]

πŸ’― β€” yes, totally, because this is really a non-trivial work-around to something that a combination of upstream dependencies (Drupal core, and even Composer IIRC!) is forcing upon us! πŸ€ͺ

Alas, @larowlan indicated in #23 that that updated upstream dependency won't change all that much for us, except for the provider πŸ€·β€β™€οΈ

Signaling that @bnjmnm should feel free to merge this when he considers it ready: RTBC! πŸ˜„

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Nice catch, thanks!

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Fair point. @effulgentsia, thoughts?

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

The List with search test case of ApiContentControllersList has been failing.

Also: I found a pretty massive bug which indicates the current test coverage is not adequate.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

This MR is not yet updating \Drupal\experience_builder\Extension\XbWrapperNode and friends, but I'd be happy to land the current MR first β€” it just needs the test expectations to be updated now πŸ€“

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

An SDC with the following in its *.component.yml:

props:
  type: object
  required:
    - heading
  properties:
    heading:
      type: string
      title: Heading
      examples:
        - blabla

… yet the explicit input it receives is not

heading: "something"

but

heading: "something"
foo: :"bar"

IOW: some key-value pairs that are not expected by the SDC!

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

I can think of one mechanism: for ComponentTreeHydrated to detect the presence of #lazy_builder or not, and if it exists, for XB to decorate the component source's lazy builder with its own, so that it's XB's "decorating lazy builder" that adds the prefix & suffix.

P.S.: I doubt my #4 was as precise/accurate as my #12 πŸ˜…

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

@larowlan in #7:

both

Hah! I should've known πŸ˜…

@longwave in #8:

I agree that slimming down the inputs to remove a lot of duplicated data is worthwhile, although this feels like it adds a huge amount of complexity

+1 to both those statements.

would this be better off as some kind of new core API?

πŸ€” I'm not sure I follow. Are you thinking of ✨ Add way to "intern" large field item values to reduce database size by 10x to 100x for sites with many entity revisions and/or languages Needs review ? If so: I don't see how that applies here, because this is not about "interning", but about compacting? If not that: can you elaborate? πŸ™

@catch in #9: I don't think there will be a big number of "versions" of a particular component. So storing all versions in a single Component config entity seems reasonable. Especially because we aim to allow instances using the older version to be able to transition/update to the newer versions (until none of the old version are left, and the old version can be deleted).

That being said … making the version part of the Component config entity ID would make what I summarily described β†’ in 🌱 [META] Production-ready data storage Active trivial:

Expand on what πŸ“Œ Calculate field and component dependencies on save and store them in an easy to retrieve format Active did: omit Component config entity dependencies from deps_config, and introduce a separate component_versions field prop, to store <component config entity ID>:<version> <component config entity ID>:<version> … β†’ allows finding which revisions are using not the latest version of a component

So that is definitely worth considering, too.

I think @larowlan has a good (implied) point: if it's okay for us to rely on "external data" for DynamicPropSources (aka the FieldConfig entity, the host entity an instance lives in, etc.), then why is it not for StaticPropSources?

So: let's get a working PoC and then re-assess πŸ‘

@larowlan Given that this will touch the Component config entity implementation, its config schema, and (Static|Dynamic)PropSource, I think it probably makes sense for me to take this one on. Then you can tackle πŸ“Œ [PP-1] Consider not storing the ComponentTreeStructure data type as a JSON blob Postponed at the same time? πŸ€“

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

I think you're thinking of ✨ Add way to "intern" large field item values to reduce database size by 10x to 100x for sites with many entity revisions and/or languages Needs review ?

Not opposed to #3469082, nor to #7.2. But I think especially #7.2 is something that could happen later, and most certainly should not be XB-specific.

The only reason for me to prefer @larowlan tackling it here instead of landing #3469082: time (well, and potential scope creep). #3469082 will not land before 11.2, which means it won't be in time for XB's 1.0-goal-by-DrupalCon-Vienna-in-October.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

The fallback component will not, I assume, have any props -- but it will render child slots, presumably to keep as much of the layout intact as possible.

was also still wrong in the issue summary β€” see #69 and earlier for why that's not the case.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Cross-posted with @lauriii's #66.

You're right, @lauriii, that behavior was changed πŸ‘ But the docs still said that was the behavior until a few seconds ago. And @penyaskito also got confused by the code β€” so docs in the relevant code have been clarified too πŸ‘

Yep, +1 for a follow-up on that. And +1 for postponing that until we have made decisions around what the data model is going to be.

Done: ✨ [later phase] Provide API for finding and UI for surfacing dangling/dead component subtrees β€” aka garbage clean-up Postponed .

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

This MR elevated the importance of πŸ“Œ Constraint slot names allowed by XB in its component tree Active .

@penyaskito seems to have struggled to understand the same bits of the MR as me β€” good reassurance β€” so I got his +1s for some of the comment/docs additions πŸ˜ŠπŸ™

The subtler/trickier bits of the Fallback plugin made me discover a bug in the block component source: πŸ“Œ `BlockComponent::validateComponentInput()` allows garbage to pile up Active .

Epic work here, everyone β€” especially @larowlan & @f.mazeikis!

I think the essence of this issue is best described by

  • docs/components.md#3.4 Fallback for architecture and end-user impact
  • the ComponentInterface::setSource() docblock for details wrt when "changing the source" is appropriate
πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

This seems like a stable blocker? πŸ˜‡

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Thanks!

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

@larowlan in #52: very nice work! πŸ‘πŸ€© Glad to see #48.1 was easy, #48.2 has a stronger reason than I realized to be the way it is (thanks for the follow-up #3524052) and #48.3 was solved subsequently πŸ‘

@lauriii in #54: The fallback rendering actually was rendering the subtrees on both the live site and in the preview. That's what the Fallback source was literally doing.

You can run into this without even doing anything too crazy because if you add a required field to an existing content type with content, it makes all of the content invalid. We have no safe guards against that today.

This is not a fair comparison IMHO. If an SDC/code component adds a new required prop, the component will refuse/fail to render if it is not populated. Content entities in Drupal that don't have a required field just won't render that field, and subsequent edits will require a value to be specified.
Note: supporting new required props on SDCs/code components is actually the easiest problem to solve. Explanation at #3509115-6: Plan to allow editing props and slots for exposed code components β†’ .

document how one could delete all instances of a component when you really want to get rid of all of the content.

So, you're proposing a new issue for identifying and surfacing dangling/dead component subtrees? If so: +1, and no work should happen on it until after πŸ“Œ [PP-1] Consider not storing the ComponentTreeStructure data type as a JSON blob Postponed + πŸ“Œ [PP-1] Spike: Explore storing a hash lookup of component inputs Postponed are done, because they would significantly simplify the logic for finding+listing those dead subtrees, and for bulk-deleting them.

Tagging for follow-up.

@larowlan in #61 Woah, πŸ“Œ [PP-1] Autosave data should store converted server-side representation of model Postponed would be a massive change indeed.

Re-reviewing in detail. Hopeful to merge πŸ€“πŸ€ž

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

@effulgentsia: please prioritize this ASAP, because the decisions here need to be properly documented, given that proposals are starting to surface to change it: [#3524298.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

This would be a massive change in the auto-save architecture.

Unfortunately, the ADR for that still hasn't landed: πŸ“Œ Document the reasons for not using Workspaces for saving in 0.2.0 Active . 😬

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Important for usability of Code Component creators.

Critical for XB's data integrity when we add support for changing props/slots.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Note: a new required prop would actually be trivial to support. Because XB requires required props to have an example value!

Especially since πŸ› Component trees with dynamic prop sources that introspect field values don't add the relevant fields to their dependencies Active just added ::getDefaultExplicitInput(), this would be easy to support.

Conceptually speaking, the needed changes would be:

  1. ComponentTreeHydrated would have to change from
    $source->renderComponent($stored_explicit_input)
    

    to

    $source->renderComponent($stored_explicit_input + $source->getDefaultExplicitInput())
    </li>
    <li>EITHER <code>ValidComponentTreeConstraintValidator

    would need to be updated to relax from

    $component_source->validateComponentInput(
              inputValues: $stored_explicit_input,
              component_instance_uuid: $component_instance_uuid,
              entity: $host_entity,
            ),
    

    to also do + $source->getDefaultExplicitInput()

    OR

    \Drupal\experience_builder\Controller\ClientServerConversionTrait::convertClientToServer() should do + $source->getDefaultExplicitInput(), which would ensure that any edit to any component tree would automatically add new required props' default values.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Related: ✨ Usage info for code components with unpublished changes Postponed . Because … it'd be 100% safe to change props and slots whenever as long as there are no instances yet. And thanks to πŸ“Œ Calculate field and component dependencies on save and store them in an easy to retrieve format Active , that's now actually possible.

IOW: the conversation here needs to be forked along two paths:

  1. 0 instances exist: anything is allowed
  2. >=1 instances exist: care is needed
πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Can you reproduce this, @mayur-sose? πŸ™

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

~3 weeks of silence, so I have to assume #2 was accurate.

Feel free to reopen if not!

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

@lauriii: has this been decided by now? 🀞

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Per @tedbow's testing in #3, plus the fact that 2 months later this still hasn't been reopened, considering this done.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

The screenshot shows this particular PHP notice/deprecation came from the field_group module. I'm 95% confident that πŸ“Œ Improve server-side error handling Active indeed fixed it.

If not, feel free to reopen, @heyyo! 😊

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Thanks, @meghasharma!

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Never got the STR, but πŸ“Œ Improve server-side error handling Active landed in the meantime πŸ‘

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

AFAICT this has been (largely?) solved elsewhere?

@balintbrews, can you give us an update? πŸ˜‡πŸ™

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Marked πŸ› Error Active as a duplicate. This is very low-hanging fruit.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Cross-posted with @longwave in #43 β€” obviously we agree 😁

Updating title, because thanks to ::testRemoveFieldTypeProviderModule(), @phenaproxima actually solved more while at it πŸ₯³

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Was failing tests. Wanted to see this through, so debugged, and found that reverting a single line was sufficient to make it pass. Added a clarifying comment.

Glad to have @longwave's +1 for the new GeneratedFieldExplicitInputUxComponentSourceBase::getDefaultExplicitInput().

Agreed with @longwave's suggestion to improve it + adopt it more widely in a follow-up. Tagging for that. (That will be squarely in the issue queue component, rather than issue's . πŸ‘)

@phenaproxima's additions of ComponentInputs::getPropSourcesWithDependency() and ComponentTreeItem:: updatePropSourcesOnDependencyRemoval() look fantastic (plus, the test coverage of course). The latter new method will undoubtedly come in handy when updating the Pattern and PageRegion config entities. Created ✨ Implement `::onDependencyRemoval()` for `Pattern` and `PageRegion` config entities: when a module providing a field type is uninstalled, replace it with the default StaticPropSource Active for that.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Thanks, @brandonlira!

Specifically, we need to know which database (SQLite, MariaDB …) and which version.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Realization while running, will verify in code tomorrow: this might be caused by the global asset library (auto-saved or not) not being an asset library dependency _yet_ of every code component’s asset library.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

I intend to get to the bottom of that one tomorrow, but it is reassuring to read you do not anticipate problems with this implementation order 😊

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

@mglaman and @penyaskito pointed something out that I missed: #3518185 forgot to update the cache tag logic of the JS component config entity.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Even if #13 says πŸ› Components sourced from JsComponent are missing source component cacheable metadata Active didn't fix it (and perhaps that's due to a bug I spotted in the current MR, perhaps not), #3522217 should land first because otherwise it wouldn't be clear how to debug/reason about this issue.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Thanks, @bnjmnm!

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Removed

debt: we know we'll need to support implicit inputs (e.g. block plugins can declare required + optional "contexts" that allow block plugins behave differently not only to component instance author's explicit wishes ("explicit inputs"), but also based on the component developer's logic ("implicit inputs")

because that doesn't affect the data storage.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

we would have one table per component version (set of fields)

Try to do it in a storage layer that supports one table per component (version), not one per component version per entity type (as is the case with fields in core)

🀯 That could easily be hundreds of DB tables: a site can easily have a 100 components (the issue summary assumes 50), and for each of those multiple versions. (Note that "version" here is a massively overloaded term β€” there can be very different reasons. See #3523841-6: Version component prop definitions for SDC and Code components β†’ .)

⚠️ Concern: this would make πŸ“Œ [later phase] When the field type for a PropShape changes, the Content Creator must be able to upgrade Postponed much harder. What if a site with a million existing revisions decides to implement hook_storage_prop_shape_alter() to change the field type for a prop of an SDC that is present in all of them (to improve the authoring experience, or to switch from plain images to Media Library or $REASON).
This architecture would require rows to be removed from one table and moved into another!

Although I think it could be argued that that would be much clearer. It'd also allow dropping tables for older "component versions" that don't have any remaining rows anymore, and would also allow removing the corresponding entries in the Component config entity that πŸ“Œ Version component prop definitions for SDC and Code components Active would've added.

πŸ€” Not sure yet, but for sure interesting πŸ˜„

Explore what views integration would look like

I don't see yet how that'd be meaningful. Views lists things of the same type in a single list/grid/table/…. But here those same things (instances of the same component version) are spread across many entities and bear no relation to one another. Unless you're thinking about listing all the different component instances of a single entity? Or something else still? But listing the first or fifth or Nth instance of some component still is not meaningful?

I struggle to follow your thinking here πŸ˜‡

Explore what this would like for e.g. Block settings that whilst modeled using config schema (and therefore typed data) are arbitrary in shape and would traditionally be stored in a serialized column

I'm really curious about this part 🧐

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

So for example if a block changes its settings, we have to loop over every revision and search for components and then update the whole blob.

Indeed. And for that, we have πŸ“Œ [SPIKE] Prove that it's possible to apply block settings update paths to stored XB component trees Active .

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Thanks to πŸ“Œ Calculate field and component dependencies on save and store them in an easy to retrieve format Active + πŸ“Œ [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table Active , it's now trivial to find all the content entity revisions in which a certain block plugin is used.

But what's still missing is fundamental core support for generically updating the inputs expected by plugins that may be used in multiple contexts, and not just e.g. Block config entities. The core issue for that is πŸ› Block plugins need to be able to update their settings in multiple different storage implementations Active .

Thanks, @catch, for opening that, and stating it as starkly as you did: β€” I don't either. 😬

Self-assigned in #10, forgot to link here to the write-up I posted at #3520484-22: [META] Production-ready ComponentSource plugins β†’ as promised. Unassigning.

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

This potential direction is why I prioritized πŸ“Œ [later phase] Support matching `{type: array, …}` prop shapes Postponed and got it finished. Because I have a hard time seeing how this work with multi-value fields. Especially because of #3052670: Support multi-valued "field union"s β†’ .

So, to avoid us adopting this and potentially losing multi-value support, I made sure πŸ“Œ [later phase] Support matching `{type: array, …}` prop shapes Postponed was working, and proves that multi-value scalars (type: array, items: { type: integer } β€” see the sparkline test SDC) and multi-value object shapes (see the image-gallery test SDC) can work in the current architecture.

(I'm not fundamentally opposed to this β€” just concerned we'd forget about that, and now we can't! πŸ‘)

Related: I tried to push #3467890 forward and assigned it to you at #3467890-13: [later phase] Support `{type: object, …}` prop shapes with single level that require *multiple* field types: use `field_union`? β€” OUT OF SCOPE: nested components/component reuse β†’ for feedback, @larowlan πŸ˜„

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Basically only a naming remark left β€” other than that, this is is IMHO good to go!

Please assign this to @longwave for final review after you've addressed the naming nit.

@longwave, I think the new ::getDefaultExplicitInput() should be used in multiple places in GeneratedFieldExplicitInputUxComponentSourceBase β€” curious if you agree. That'd allow us to be more confident about it IMHO.

Production build 0.71.5 2024