Overview
Route experience_builder.component_props_form uses a form \Drupal\experience_builder\Form\ComponentPropsForm
This form mixes form API and React components via the semi-decoupled theme engine.
This theme engine:
- Hijacks the default twig theme engine
- Has hardcoded special treatment of theme suggestions that end in _xbxb
- Uses json in twig to map Form API properties to JSX data-types
Proposed resolution
At its heart, the return from this form is a serialized representation of the form.
Here's a snippet of the return from the semi-decoupled theme engine:
<template hyperscriptify>
<drupal-form--xbxb
attributes="{"class":["component-props-form"],"data-drupal-selector":"component-props-form","action":"form_action_p_pvdeGsVG5zNF_XLGPTvYSKCf43t8qZYSwcfZl2uzM","method":"post","id":"component-props-form","accept-charset":"UTF-8"}">
<drupal-html-fragment slot="children">
<div class="field--type-string field--name-heading field--widget-string-textfield js-form-wrapper form-wrapper"
data-drupal-selector="edit-xb-component-props-static-static-card1ab-heading-wrapper"
id="edit-xb-component-props-static-static-card1ab-heading-wrapper">
<drupal-form-element--xbxb attributes="{}" label-display=""before"" title-display=""before""
type=""textfield""
name=""xb_component_props[static-static-card1ab][heading][0][value]"">
<drupal-html-fragment slot="children">
<drupal-input--xbxb
attributes="{"class":["js-text-full","text-full","form-text","form-element","form-element--type-text","form-element--api-textfield"],"data-drupal-selector":"edit-xb-component-props-static-static-card1ab-heading-0-value","type":"text","id":"edit-xb-component-props-static-static-card1ab-heading-0-value","name":"xb_component_props[static-static-card1ab][heading][0][value]","value":"hello, world!","size":60,"maxlength":255,"placeholder":""}"
children="""" render-children=""""></drupal-input--xbxb>
</drupal-html-fragment>
<drupal-html-fragment slot="label"><label for="edit-xb-component-props-static-static-card1ab-heading-0-value"
class="form-item__label">heading</label></drupal-html-fragment>
<drupal-html-fragment slot="renderChildren">
<drupal-input--xbxb
attributes="{"class":["js-text-full","text-full","form-text","form-element","form-element--type-text","form-element--api-textfield"],"data-drupal-selector":"edit-xb-component-props-static-static-card1ab-heading-0-value","type":"text","id":"edit-xb-component-props-static-static-card1ab-heading-0-value","name":"xb_component_props[static-static-card1ab][heading][0][value]","value":"hello, world!","size":60,"maxlength":255,"placeholder":""}"
children="""" render-children=""""></drupal-input--xbxb>
</drupal-html-fragment>
</drupal-form-element--xbxb>
</div>
But we already have a way to serialize things, and that's the serialization module.
I propose we should:
- Finish #3467498: Component props edit form route should use a custom wrapper format so that we have a unique wrapper format we can target to trigger the serialization
- Add custom serializers to support converting form API's data structure to a new serialization format. We could name this format something like
hyperscript_template or xb_template (the name doesn't really matter).
- These serializers can make use of the renderer for aspects they do not know about (things we currently don't handle with _xbxb theme suggestions) but can correctly map to the JSON contents of the twig templates currently used by the semi decoupled theme engine.
User interface changes
Comments
Comment #2
wim leersSo rather than me theorizing, I'd rather have somebody with more distance to the XB code base, but deep insight into the https://www.drupal.org/project/jsx codebase, to respond here. That person would be @effulgentsia.
P.S.: the term "hijacking" has a very negative connotation. I don't think that is necessary.
Comment #3
larowlanI realise it's a strong word, I'm trying to convey that the current approach is not suitable for production sites
I've toned it down
Comment #4
larowlanComment #5
effulgentsia commentedI agree that
experience_builder_system_info_alter()is too heavy handed in terms of overriding all themes to use the semi_coupled engine, and that this will need refactoring prior to XB being suitable for production sites.Do we have any precedent for using the serialization module to serialize render arrays? I think of the serialization module as there to serialize models, but render arrays are views (not in the Views module sense, but in the MVC sense). And serializing render arrays to HTML is exactly what the render and theme systems do.
Conceptually, the
*--xbxb.html.twigfiles function exactly like theme hook suggestions. They override the base template and render different HTML than what the base template renders. In this case, that "different HTML" is an HTML custom element.I agree that it's weird that the
*--xbxb.html.twigfiles contain JSON instead of Twig. I think there's ways that that can be refactored. For example, the JSON could be moved to a different filename, like*--xbxb.spec.json, and the semi_coupled engine could be refactored to a Twig function, so that the*--xbxb.html.twigfiles could become regular Twig files that just do something like:That would then also remove the need for
experience_builder_system_info_alter().Not sure if the above is the best DX though, and there might be better ideas for how to bridge XB's theme hook suggestions to rendering the desired custom element, but I don't see how or why the serialization module belongs in the mix for this. Maybe I'm wrong though and am just underestimating the scope or utility of the serialization module?
Comment #6
larowlanSymfony setializer defines two interfaces, NormalizerInterface and SerializerInterface. The first one is for turning objects into arrays, the second one is for turning arrays into strings. Eg there's an XmlSerializer, a JsonSerializer. We'd just have a XB template setializer for turning render arrays into a string in our bespoke manner
Comment #7
effulgentsia commentedBut no other module that defines theme hook suggestions does that. E.g.,
field_ui.moduleaddsform_element__new_storage_typeto itshook_theme(); it doesn't define a new serializer. How much of the theme system would we need to duplicate into an XB template serializer?Comment #8
larowlanI think with some refactoring of theme manager (eg adding methods as extension points) , we could subclass it and not need to duplicate much
But it would require a spike to identify those extension points
I also think storing the link between a react component and a form element plugin should happen via form element plugin definitions. The setializer can read that to work out what should be formatted for hyperscriptify and what should not
This means in the future we can allow modules to opt in their custom elements. The hard coded mapping in the JS could be replaced with a mapping of lazy imports we could share via Drupal settings
This requires changes to vite config for the UI and the use of import maps, but we would need to do that anyway to support UI extension
I can work on a spike on this when I'm back
Comment #9
longwaveJust been digging into the semi-coupled theme engine a bit, and I think that if theme engines were tagged services, instead of using hook-style function names, then you could hopefully just decorate the Twig engine in order to extend it?
Comment #10
wim leersComment #11
effulgentsia commentedBy the way, if I understand the desired UX correctly, we're wanting to render the forms in the right-hand sidebar using a set theme, neither the site's front-end theme, nor the admin theme. If this is correct, then even if we did no refactoring to the semicoupled theme engine, we could just use it as it was initially designed for: as the theme engine of the theme that we use for rendering the forms in the right-hand sidebar, so we wouldn't need to hack-alter the engine of themes we don't control.
That said, I'm not opposed to refactorings that decouple it from being a theme engine, so that it could be more cleanly integrated with any theme.
Comment #12
wim leersIt's not entirely correct.
@lauriii has indeed indicated that XB needs its own look and feel.
*.tsxfiles.Comment #13
effulgentsia commentedI don't get that. If we always negotiate the Stark theme, why can't we instead create an
xb_starktheme that's just like Stark (whether a fork or subtheme) but that setssemicoupledas its theme engine, and always negotiate that theme? That would allows us to removeexperience_builder_system_info_alter()which is the part that's "taking over the twig theme engine for the whole site" that this issue's title is referring to.Comment #14
wim leers🤔
Ohhhhhhhh! 🤓😁
TIL that's possible! 🤯 Quoting
core/modules/system/tests/themes/test_theme_nyan_cat_engine/test_theme_nyan_cat_engine.info.yml:… built by a certain "lauri" 9 years ago at #2574717: Remove PHPTemplate, and add test coverage for multiple theme engine support … during DrupalCon Barcelona … 2015! 😄
I think this should be the next thing for @deepakkm to tackle now that #3477164: Lift all methods out of `SdcController` into separate invokable services-as-controllers is 99% done.
Comment #15
wim leersCaptured #13 into a concrete proposed solution. @effulgentsia, can you review/confirm, and then assign back to @deepakkm if you do agree? 🙏
Comment #16
wim leersComment #17
effulgentsia commentedI agree with #14, but that wouldn't fully address @larowlan's original goals in this issue. He may still want to pursue the "custom serialization format" part, which I disagree with, but who knows, maybe he'll come up with an MR that convinces me. Therefore, I think we should open a child issue to do just the part that's in #13 and what's minimally required for that, and leave this issue as potentially still having @larowlan's more ambitious goals.
Comment #18
wim leersFair! Done: #3478287: Introduce the `xb_stark` theme that uses the Semi-Coupled theme engine, removes the need for taking over the Twig theme engine for the whole site.
Let's wait for #3478287 to land, and then ask @larowlan to re-assess 😊
Restored original issue summary.
Comment #19
wim leersComment #20
wim leersComment #21
wim leers#3478287: Introduce the `xb_stark` theme that uses the Semi-Coupled theme engine, removes the need for taking over the Twig theme engine for the whole site is in.
Unpostponing and assigning to @larowlan per @effulgentsia in #17.
Comment #22
effulgentsia commentedWith #3478287: Introduce the `xb_stark` theme that uses the Semi-Coupled theme engine, removes the need for taking over the Twig theme engine for the whole site in, we're no longer "taking over the Twig theme engine for the whole site", so retitling the issue to what I understand @larowlan's proposal to be. However, per #5, I still don't really see why we would want to circumvent the theme system when rendering to HTML, and I think trying to do so would lead to a lot of difficulties when the form has a mix of some elements that have XB suggestion overrides and some that don't interspersed through the tree (e.g., either can be a child of the other).
Comment #23
wim leersAgreed with @effulgentsia. Perhaps this is the more appropriate issue status 😇
Comment #24
wim leersBump, @larowlan: can we close this? 😇
Comment #25
larowlanThere is a core idea that has some overlap here #3506337: Make Drupal core multi-frontend
I think we can close this but revive it in core by adding support for splitting the renderer up so that HTML is just one serialization format we can generate from render arrays.