Overview
Proposed in #3461499-17: Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings, +1'd by @lauriii in #19 of that issue.
🛑 Blocked on #3461499: Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings.
For a while now, @lauriii had expressed concerns about the whole "opting components in" UX (even though this was what we decided early on). Only now we're sufficiently confident that we can do without this, based on:
- what #3461499: Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings proved to be possible
- #3462705: [META] Missing features in SDC needed for XB becoming gradually more clear, and especially @lauriii specifying that it's okay for XB to only allow SDCs that define >=1 example for every required prop, and accepting that this has consequences for per-site default values
Combining the ability to compute field type + widget (point 1 above) and default value (point 2 above) means that the crucial information that the Component config entity used to gather, no longer needs to be gathered.
Proposed resolution
- Automatically generate
Componentconfig entities for all SDCs that meet the requirements, upon:- Module installation
- Theme installation
- Clearing all caches
- Bonus/optionally: on every request if
Twig development mode
is enabled at/admin/config/development/settings
- For now those criteria are simple:
- every required prop must have >=1 example.
- every prop (optional or required) must have a
title→ surfaced by #3467162: Follow-up for #3461422: display SDC prop's human-readable name (`title`), not its machine name
(Future criteria: SDC's
statusmust/must not be a certain value, enum values must have labels, etc.) - Use the exact same
Componentconfig entity that exists today, so including populating the following:
defaults: props: <prop name>: field_type: … field_widget: … default_value: … expression: …(this enables #3464042: Add ComponentAuditabilityTest)
- When: unfortunately,
\Drupal\Core\Theme\ComponentPluginManagerdoes not specify a cache tag. So, we have no way of knowing when it discovers new SDCs 😬 - Work-around: decorate that service, pass everything through, but decorate
getDefinitions(). Call the decorated method first, and compare the discovered definitions with those that haveComponentconfig entities already. Any that meet the requirements but do not yet have aComponentconfig entity, get aComponentconfig entity.
(Q: Why::getDefinitions()and notgetAllComponents()? A: because then we avoid instantiating all SDC plugins. But if the latter is simpler for now, then just go for that. Actually, maybe that's acceptable becausesdc_library_info_build()calls it too, and a module getting installed would rebuild libraries anyway.) - Delete all
experience_builder.component.*.ymlfiles from the codebase — these should be auto-generated now! (This might require some shenanigans/fiddling in tests, especially in kernel tests.)
User interface changes
None — other than /admin/structure/component maybe having more things listed :)
Issue fork experience_builder-3463999
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
wim leersComment #4
wim leersComment #5
wim leersComment #6
wim leersComment #7
wim leersAsked @lauriii to confirm he agrees with the proposed resolution, especially with an eye towards #3464042: Add ComponentAuditabilityTest.
Comment #8
lauriiiI responded on #3464042: Add ComponentAuditabilityTest. I'm concerned that storing this information in the
Componentconfig impacts DX negatively. We need to consider how are components authored, and how are they edited during their lifecycle, and how that overall experience looks like. Today, if components or templates are being changed, the changes are reflected immediately to the site. We should strive that same experience. We need to remove as many barriers from the XB from authors; the goal is for it to feel just as if you're editing a regular template or component.Comment #9
wim leersThis is not true:
/admin/config/development/settingsdiscoverycache bin'scomponent_pluginscache item! And since there's no cache tags associated with that, the only way to achieve that is to go to/admin/config/development/performanceand click (more efficiently through the CLI:drush cc discovery).Actually … the way it's cached is so aggressive that a major core bug for it was just fixed yesterday: #3460598: Single directory component CSS asset library not picked up in admin theme immediately after module install without cache clear. 😅
IOW: to actually achieve what you describe, we'd need an , which re-discovers all SDC metadata on every request. We could and should do that! Opened issue for that: #3464388: SDC *.component.yml metadata is cached aggressively, gets in the way of component development.
And … when that mode is on … we'd also just automatically update the
Componentconfig entity! 🤓Result:
Comment #10
wim leersThis blocks #3464042, and at #3464042-4: Add ComponentAuditabilityTest you can find a detailed write-up that justifies both this issue as well as that one, based on XB's product requirements.
Comment #11
wim leers#3461499: Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings is in.
@f.mazeikis, see #3461499-24: Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings for some additional context, but here's the most relevant bit:
Comment #12
wim leersComment #13
wim leersAFAICT this issue would likely have independently surfaced #3465981: Follow-up for #3461499: transform at-rest field storage settings using FieldItemInterface::settingsFromConfigData(), especially if #3446722: Introduce an example set of representative SDC components; transition from "component list" to "component tree" lands before this.
#3465981 is only a soft blocker to this though, because there's plenty of other pieces to be done in this issue.
Comment #15
wim leers#3467162: Follow-up for #3461422: display SDC prop's human-readable name (`title`), not its machine name surfaced one additional requirement.
Comment #16
wim leersComment #17
wim leersDiscussed in detail with @f.mazeikis. He discovered oversights in
(Candidate)StorablePropShape+ the config schema while working on this MR. The solution is evident: afieldInstanceSettingssibling tofieldStorageSettings. He'll extract that as a blocking issue, with a fairly trivial MR that can land very quickly, to unblock this issue to do things "properly". 👍Comment #18
f.mazeikis commentedPostponed until blocker #3467712: Follow-up for #3461499: Add fieldInstanceSettings to `(Candidate)StorablePropShape` + `StaticPropSource` is resolved.
Comment #19
wim leersThat's in now! 🚀
Comment #20
larowlanRather than this being automated, what about an import wizard from the list builder as an action link? When we support blocks that could also be included
Comment #21
wim leers#20:
@lauriii has stated it's a hard requirement that it is possible to do it fully automatically.
As a next step, we would only do it fully automatically when in "development mode" (because on production you wouldn't want new SDCs appearing in XB. See #3464042: Add ComponentAuditabilityTest, and the discussions with @catch on that subject over at #3444424: [META] Configuration management: define needed config entity types.
Comment #23
wim leersReviewed 🏓
The failing
Cypress E2Etests are showing this in the output:… and sure enough,
components/containers/shoe_tab_group/shoe_tab_group.component.ymldoes look rather bizarre:I went back to #3446722: Introduce an example set of representative SDC components; transition from "component list" to "component tree" and found that @ctrlADel's original implementation of that (commit:
ef34f8106dfe2103df38055154efc9bef9ba9b1d) was never changed by @tedbow nor me. So I think this was just an honest mistake.Fixed in https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...
Comment #24
wim leersA bunch of the components that @ctrlADel added in #3446722: Introduce an example set of representative SDC components; transition from "component list" to "component tree" do have
examplesbut then specify no examples. That's odd/weird, but XB should graciously handle that: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...Comment #25
wim leersI pushed a handful of tiny commits to help get this MR to green — because the failures were happening in areas far removed from the changes that are actually in the scope of this MR (and far outside areas @f.mazeikis has previously touched).
I also solved one very tricky thing WRT default config: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1....
This MR still needs:
… but those should be smooth sailing now 😊🤞 Looking forward to seeing this one land! 😄
Comment #26
wim leers#3467972: Unable to save node article form — remove obsolete TwoTerribleTextAreasWidget + fix duplicate `XB:image` SDC unblocked this issue — thanks @f.mazeikis for reviewing that!
Comment #27
wim leersTests look great!
I bet fixing https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... will also fix the failing Cypress E2E test.
This feels very close now 😊🥳
Comment #28
wim leersThis issue represents a huge leap forward! 🚀
Most importantly in the short term: this will make it an order of magnitude easier to start testing https://www.drupal.org/project/demo_design_system 👍
Also:
Comment #30
wim leersComment #31
wim leersComment #32
wim leersThis caused a regression: #3470550-10: Component config entities are incomplete: missing entries for optional props, causing errors in ComponentPropsForm.