🇧🇪Belgium @wim leers

Next week.

Fixed as part of #3512854, see #3512854-20: Determine core bug(s) and missing metadata that explain unmatched field type properties; make the reported % reflect reality → .1.

Ready now, because:

1. Not yet supported: a JSON schema (prop shape) for the following fields: test_optional__decimal, test_optional__language, test_optional__telephone, test_required__decimal, test_required__language, test_required__telephone
   

   and

   Not yet supported: a JSON schema (prop shape) for the following field properties: test_optional__decimal.value, test_optional__language.language, test_optional__language.value, test_optional__telephone.value, test_required__decimal.value, test_required__language.language, test_required__language.value, test_required__telephone.value
   

   … all of which is entirely reasonable: JSON Schema does not have native ways to express telephone numbers nor language references, and the "decimal" field type requires an adapter to be converted to a (JSON Schema) float. Instances of all other field types are either supported or explicitly ignored.

   BTW: this is now printed using E_USER_DEPRECATED, so that also fixes 📌 CI: surface the list of field types, field type props and field widgets not yet supported by XB Active . 👍

2. The explicitly ignored list is \Drupal\experience_builder\ShapeMatcher\JsonSchemaFieldInstanceMatcher::IGNORE_FIELD_TYPES and now also speeds up the matching. They are:
   

   public const IGNORE_FIELD_TYPES = [
       // The `list` field types allows each field instance to define its own set
       // of possible values. The probability of this exactly matching the explicit
       // inputs for a component is astronomical.
       // If we ever decide to allow this, then the `Choice` constraint must be
       // correctly specified on it. Otherwise, `::toDataTypeShapeRequirement()`
       // does not find any constraints and matches every such field instance
       // against every integer/float.
       'list_float' => ListFloatItem::class,
       'list_integer' => ListIntegerItem::class,
       'list_string' => ListStringItem::class,
       // The `map` field type has no widget, is broken, and is hidden in the UI.
       // @see https://www.drupal.org/node/2563843
       // @see \Drupal\Core\Field\Plugin\Field\FieldType\MapItem
       'map' => MapItem::class,
       // The `password` field type can never contain data that could be reasonably
       // displayed in a component instance.
       // @see \Drupal\Core\Field\Plugin\Field\FieldType\PasswordItem
       'password' => PasswordItem::class,
     ];
   

   … which is also entirely reasonable.

3. Finally, this now gets us to:
   

     /**
      * The current % of supported field types whose instances can be matched.
      */
     public const COMPLETION = 0.8846153846153846;
   
     /**
      * The current % of supported field type props.
      */
     public const COMPLETION_PROPS = 0.8918918918918919;
   

   … but it really is ~100% for most sites in practice: few sites use the telephone, language or decimal field types for crucial data.

IOW: as of this issue, it's clear that essentially all structured data can be exposed via DynamicPropSources and hence via ContentTemplates! 🥳

Per #3512854-19: Find bug(s) that explain unmatched field type properties → , those 2 new additions do not belong here; they belong under 🌱 [META] 7. Content type templates — aka "default layouts" — clarify the tree+props data model Active instead.

Actually, #18 is wrong, too! 😅

• It is about what field properties can be mapped into component instances.
• But it is not about the component instance form: field instances indeed appear in the content entity form. That means @larowlan was kinda right.
• Except … that it doesn't really matter (for 📌 Discover cases where is no 1:1 map between field, prop and widget values Active ), because on the component entity form, all field widgets must work anyway — even those whose data cannot be mapped into component instances.

So, it really doesn't belong on 🌱 [META] Expand support to *all* SDC prop shapes, and *all* Active either — if anything, it belongs on 🌱 [META] 7. Content type templates — aka "default layouts" — clarify the tree+props data model Active .

P.S.: eventually those unmatched field instances may end up being usable anyway, thanks to adapters (example: \Drupal\experience_builder\Plugin\Adapter\UnixTimestampToDateAdapter) being used in AdaptedPropSources. But the UX for that is not yet defined.

… which led me to re-discover 📌 Provide visibility into field types with different matching SDC prop shapes depending on field storage settings Active .

Removed 🐛 Find bug(s) that explain unmatched field type props Active from this meta and added it to 🌱 [META] Expand support to *all* SDC prop shapes, and *all* Active instead.

Reason: see #3512854-18: Find bug(s) that explain unmatched field type properties → .

I don't see how this actually relates to 📌 Discover cases where is no 1:1 map between field, prop and widget values Active — because that is solely about widgets on the content entity form.

This issue is about shape matching SDC props with field types, and hence show field widgets for those SDC props, which means it's about widgets on the component instance form.

As discussed at #3487284-18: [META] Add explicit end-to-end test coverage for using all core field widgets including multiple cardinalities in the "Page Data" (content entity) form → , that means it belongs under 🌱 [META] Expand support to *all* SDC prop shapes, and *all* Active instead.

off-topic
@effulgentsia in #10:

Also, renaming "props" to "properties" in the issue title. The "props" short-hand is a React convention, so I think should be reserved for React and component-based technologies that were inspired by React, including SDCs. Drupal field item properties, on the other hand, were inspired by object-oriented conventions, which typically use the unabbreviated word "properties".

Fair! I'll do this going forward. 👍

But … it's far too easy to confuse "props" and "properties". So, I do think we must continue to always disambiguate: the entire XB codebase and docs refers to "field properties" as "field props" (i.e. always with the prefix "field") vs "SDC props" (for props of SDCs specifically) vs "component props" (for props of any component that uses the term "props", which includes JavaScriptComponents aka "code components").
end of off-topic

I’d swear this was already working, but I guess not?! 🤪

Thanks to the solid test infra + @penyaskito’s excellent issue summary, I believe this is doable for a novice contributor 😊

📌 Stop storing JSON blobs in XB config entities: improve DX Active is in 🥳

FYI: with this done, I think it's time to re-assess 📌 ServerClientConversionTrait Active and friends, aka make a decision on whether to keep ClientServerConversionTrait because it is good enough, or whether to transition to something else/better.

Added 📌 Decouple generated source plugins from SDC plugins Active , 📌 [PP-1] Implement client-side validation of block settings Active , 📌 ComponentSourceInterface::submitConfigurationForm and validateConfigurationForm are never called Active .

This AFAIK ties into follow-ups opened for 📌 Add support for Blocks as Components Active that were never followed up on. A quick scan suggests:

• 📌 Define how block settings should be presented in the UI Active
• 📌 [PP-1] Implement client-side validation of block settings Active (see \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\BlockComponent::fixBooleansUsingConfigSchema())
• etc.

Per #3513589-6: [PP-1] Decouple generated source plugins from SDC plugins → , that is PP-1.

Can we first do 📌 [PP-1] Refactor GeneratedFieldExplicitInputUxComponentSourceBase and FieldForComponentSuggester to need only SDC's ComponentMetadata, not SDC plugin instances Postponed , so that everything is at least using not SDC plugin instances but just \Drupal\Core\Theme\Component\ComponentMetadata?

After that, I agree the client-side schema stuff is important to generalize! Related: 📌 Evolve the organically grown `::getClientSideInfo()` into something cohesive Active .

Related: #3521137-18: Stop storing JSON blobs in XB config entities: improve DX → , which is an outline of how to do something similar for the other field props on the XB field type.

As a byproduct, the TestDataUtilitiesTrait that @tedbow introduced in 📌 Convert test cases to use PHP arrays instead of JSON strings where possible Fixed to ease test development pain during initial development of XB, will largely be obsolete — that's what I asked for in #13, and which @phenaproxima almost completely finished! 🤩

In fact … since I'm feeling under the weather and my mind doesn't allow me to tackle anything complex right now, I just persevered and refactored it away: TestDataUtilitiesTrait is GONE! No more need to remember to wrap an array in a ::encodeXBData()! 🥳 All thanks to the one crucial insight by @phenaproxima! 👏

So: we'll not only make interacting with XB config less painful for site builders/XB users, but also for us developing XB, and hence improve development velocity slightly :)

P.S.: I'd still don't like that ->getValue(), TRUE has 7 matches in the codebase, but that's far better than the hundreds this MR removed. So, for the sake of iterative progress, didn't expand scope to that. Also: once 📌 Allow field types to control how properties are mapped to and from storage Needs work lands, XB won't have to work around that flaw in Drupal core anymore. If somebody wants to take that on: see attached PoC patch to get you going.

FYI: The DX niceties this brought were slightly reduced in 🐛 ComponentTreeHydrated prevents serialization Active out of necessity.

👉️🔄

Added 📌 Stop storing JSON blobs in XB config entities: improve DX Active .

Bumping to per ✨ Create a schema for "allowed_html" which provides a better config diff Needs work .

Is this still relevant, @tedbow?

I don't think this being assigned to @lauriii for #34 is highly relevant anymore.

@penyaskito has been leading the effort on the XB team to get as many SDC improvements into core as possible. I see he's got 🐛 Disable additionalProperties in slots, props in SDC json schema Active to RTBC too 🤩🥳

@penyaskito Could you take on #46 + #47 on the XB side too? 😇🙏

Legitimate failure:

Tried to find [href*="components/my-hero/my-hero.css"] in <head> 

Due to this test assuming non-aggregation:

    // Confirm that the iframe loads the SDC CSS.
    cy.getIframe(
      '[data-xb-preview="lg"][data-test-xb-content-initialized="true"][data-xb-swap-active="true"]',
    )
      .its('head')
      .should('not.be.undefined')
      .then((head) => {
        expect(
          head.querySelector(
            'link[rel="stylesheet"][href*="components/my-hero/my-hero.css"]',
          ),
          `Tried to find [href*="components/my-hero/my-hero.css"] in <head> ${head.innerHTML}`,
        ).to.exist;
      });

If I was right in #3, then #3485878 should've fixed this too 😇

@sandip There's a round of feedback on that MR, and there's also 🐛 Create loadAutoSave() helper function Active 😄

BE pieces + automatic-upgrade-path-upon-editing (the only backwards compatibility thing we've ever done, which feels odd, but it's so trivial that it is worth it) removal issue ( 📌 Remove backwards compatibility with "Text area" prop in code editor, superceded by "Formatted text" Postponed ) look good.

Trivial conflict resolution.

Thanks, @jatingupta40! Not passing tests yet, but a great start!

🥳

🥳

🥳

penyaskito → credited wim leers → .

This will eventually end up unblocking 📌 [PoC] Introduce a `ContentTypeTemplate` config entity Active , because for that (and for performance reasons) we'll need to do that 3rd bullet point.

RTBC'd with 2 caveats/requests: 1, 2.

If not addressed here, we need a follow-up for especially the latter.

@sandip If you're interested in doing more of this valuable work: 📌 Rename `FieldForComponentSuggester` to `StructuredDataSourceSuggester` Active 😊🤞

Thanks so much, @sandip, this feels way better! 😄

wim leers → created an issue. See original summary → .

Thanks for confirming.

The content template should not be deleted.

Indeed!

This should make XB to fall back to the default (example) values for those inputs.

Yes, that's what @lauriii proposed in #11 👍
In my mind, we would just delete the component instance. But … I think what you propose here (and I guess makes more sense indeed! 😄

So:

1. I have a ContentTemplate (for article nodes) with a hero component instance whose marquee prop is populated by
   

   {
     "marquee": {
       "sourceType":"dynamic",
       "expression":"ℹ︎␜entity:node:article␝field_funny_message␞␟value"
     }
   }
   

   i.e. it fetches the value field property of the field_funny_message field on the article node type.

   This was a DynamicPropSource.

2. The site builder wants to delete field_funny_message; and it's not reasonable for a ContentTemplate to block that, but the ContentTemplate should also not be deleted.
3. ContentTemplate::onDependencyRemoval(), checks iterates over all component instance inputs and finds the one that actually depends on this particular field (i.e. on the field.field.node.article.field_funny_message config). It replaces the DynamicPropSource with a StaticPropSource.

   Rough outline of algorithm:

   1. Check if this component requires explicit input ComponentSourceInterface::requiresExplicitInput(). If not: [] (the empty array)
   2. Sanity check: assert that Component::getSource() instanceof GeneratedFieldExplicitInputUxComponentSourceBase — only these sources can be populated by fields.
   3. (Untested PoC.) Populate all props with examples (which means: all required ones, plus the optional ones that do have an example).
      

      $component = Component::load(…);
      $source = $component->getSource();
      assert($source instanceof GeneratedFieldExplicitInputUxComponentSourceBase);
      if (!$component->requiresExplicitInput()) {
        return [];
      }
      
      // @see `type: experience_builder.generated_field_explicit_input_ux`
      $props_with_examples = array_keys(array_filter(
        $source->getSettings()['prop_field_definitions'],
        fn (array $def) => $def['default_value'] !== NULL
      ));
      return array_map(
        fn (string $prop_name) => $source->getDefaultStaticPropSource()->toArray()
        $props_with_examples
      );
      

      👆 This should return something like

      {
        "marquee": {
          "sourceType":"static:field_item:string",
          "value": "Hello, world!",
          "expression":"ℹ︎string␟value"
        }
      }
      

      (You will need to refactor GeneratedFieldExplicitInputUxComponentSourceBase to make certain methods available to ContentTemplate. For alternative implementation inspiration, see GeneratedFieldExplicitInputUxComponentSourceBase::buildConfigurationForm()).

   4. ComponentSourceInterface::validateComponentInput() on the end result
   5. Prior to saving, verify the end result by using \Drupal\experience_builder\Plugin\Validation\Constraint\ValidComponentTreeConstraintValidator::validate()

   IOW: kinda like \Drupal\Core\Entity\EntityDisplayBase::onDependencyRemoval() :)

This one seems simple enough :) I think tests are even overkill for this.

This is well on its way — and a whole range of tests fail thanks to recent test infrastructure additions, which makes sense: \Drupal\Tests\experience_builder\Kernel\Plugin\ExperienceBuilder\ComponentSource\JsComponentTest::testRenderJsComponent()'s current expectations expect no cache tags; this changes that! 🥳

add cache busting strings

Do you mean: a query string that includes the current time? Does that mean you're saying you're getting a cached response from the server side? If so: then this would actually be a server-side bug (cache tags, baby!).

Which … makes me actually jump to thinking this is just a duplicate of 🐛 Components sourced from JsComponent are missing source component cacheable metadata Active 😬 And I wouldn't have spotted that because I'm testing with settings.local.php containing

$settings['cache']['bins']['render'] = 'cache.backend.null';
$settings['cache']['bins']['dynamic_page_cache'] = 'cache.backend.null';
$settings['cache']['bins']['page_cache'] = 'cache.backend.null';

🙈

@mayur-sose Can you test this again with #3522217 applied? 🙏

#10: can you rephrase? I don't understand. 😅

Thanks for confirming, @mayur-sose! 😊🙏

P.S.: When a bug report did not need fixes, we should not mark it . 🤓

@akhil babu in #5:

Rather than adding '/' to paths that dont start with it, we should add validation to the entity form fields

💯

Signaling this requires FE-only changes.

📌 Support props that can use wysiwyg widgets Active is in now, which makes this much nicer to work on and more valuable to finish :)

📌 Support props that can use wysiwyg widgets Active is in!

📌 Support props that can use wysiwyg widgets Active is in!

🥳

Merge this on the client side with the existing list of auto-saves

Does that client-side issue already exist?

Can we get the title updated to reflect what the scope is?

NW per @larowlan's https://git.drupalcode.org/project/experience_builder/-/merge_requests/9...

#19: @deepakkm: Please see my response at 🐛 Entity saved with autosaved data is not respected Active , can you please clarify it?

Plus, as @larowlan argued: why is this even a bug? Isn't this a feature?

The title is very ambiguous. Tried to clarify it. Can you confirm it is correct?

However, when we search for 'Untitled Page,' we only get one result, which should not be the case.

Your wording implies there should be >1 result, but AFAICT there should be 0?

Confirmed with @lauriii that it's fine to consider #3518201 and #3518203 as "nice-to-haves".

Assigning to myself to ensure I follow through with issues for hardening against the "red retry" boxes in @lauriii's screenshot in #11.

Making #31's follow-ups findable.

See #3484395-32: CKEditor 5 not loading on formatted text Field Widgets in the component instance form → — a tiny change here turns out to have been a distracting bug!

Argh, can't merge because package-lock.json was just updated in 📌 Compile Tailwind CSS globally for code components Active . Can you do that, @bnjmnm? 🙏

Aiming for RTBC since it was so very close in #27 already!

Removing tag, that's now going to be handled in 📌 Document use cases where XB's field widgets don't 1:1 match how they would work in a twig theme Active .

daterange_default detour

@bnjmnm must've been testing with the (optional) daterange module installed and hence ran into a one-line logic bug introduced by 🐛 Component transforms need to be per sourceType, not per component prop Active . To get past that, he defined an empty client-side transform for the daterange_default widget, which then resulted in it being claimed to be working in \Drupal\Tests\experience_builder\Kernel\EcosystemSupport\FieldWidgetSupportTest, which then undermined the value/meaningfulness of that test … because it doesn't actually work (and I spent ~30 minutes debugging and trying to get it to work, which is why there's now a detailed analysis of where it goes wrong) 😅
To avoid that information to go to waste: ✨ [later phase] Support `daterange_default` field widget in component instance form Postponed .

Manual testing

Works great! 🤩

Based on work by @bnjmnm and I in 📌 Support props that can use wysiwyg widgets Active .

wim leers → created an issue.

Let's document this while it's still fresh in your head 😊

For #8.

Will investigate tomorrow what issues this needs. I expected 📌 Improve server-side error handling Active to have handled the /xb/api/v0/config/component case at the very least 😬

BTW, this might end up fixing both 💬 ComponentNotFoundException: Unable to find component Active and 📌 Handle update and delete of SingleDirectoryComponent `Component`s, plus missing config dependencies Active too!

🐛 Cannot delete JS components due to component depending on them Active might solve most of this.

🐛 Cannot delete JS components due to component depending on them Active might solve most of this.

"components disappearing" ==

• block → the block plugin disappearing from a module
• sdc → the SDC disappearing from a module
• js → should not be possible thanks to config dependencies

So I placed the component on a page, deleted the component from the code base, cleared caches, and reloaded XB.

Hm, that seems fine then. That's equivalent to the SDC disappearing.

How is this different from 🐛 Canvas and hover preview out of sync after drag-drop and publish Active ? That issue summary literally also refers to "hover":

Expected Result: The new background color (bg-blue-500) should be visible both in the Canvas and on hover.

@tedbow: Please:

1. also enable CSS + JS aggregation for \Drupal\Tests\experience_builder\Functional\XbPageVariantTest to ensure we have pure BE test coverage (because I doubt the end-to-end tests test the live site, they AFAIK only test the XB editing UX)
2. do a final round of manual testing with a code component that has an auto-save entry, and verify that this also works as the anonymous user.

Looking good!

Will review this tomorrow 👍 Meanwhile, it'd be great to have @f.mazeikis' question answered — funny enough I was forced to ask the same at #3517941-12: [META] Robust component instance error handling → , independently of this! 😅

But: how did you make that component crash during rendering? It seems like you deleted an SDC, code component or a block plugin, instead of triggering one of the 6 failure modes described at the top of this issue?

Oh, perhaps you tested this by doing what @f.mazeikis also guessed over at #3519168-37: Handle components provided by ComponentSources disappearing ungracefully — enables deleting JS components that are in use → , i.e. by deleting a Component config entity by force?

Reopened 🐛 Auto-save changes from code editor get lost if you navigate out too quickly Active per #12 👍

Per #3516390-12: Compile Tailwind CSS globally for code components → , that's still left to be done.

It looks like the library and the props forms fail to render because of the component is erroring. That is at least equally important with not crashing the whole preview. Is there an issue for that already?

No!

But: how did you make that component crash during rendering? It seems like you deleted an SDC, code component or a block plugin, instead of triggering one of the 6 failure modes described at the top of this issue?

If so: what you did is actually not related to this issue, because this issue is focused squarely on rendering of component instances never failing. AFAICT 🐛 Cannot delete JS components due to component depending on them Active should fix both symptoms. If it doesn't, I'll file issues under 🌱 [META] Production-ready client-side data model + internal HTTP API Active .

until we have designs to not have to spend time on implement a temporary solution.

Well … the point of the issue summary's sequencing (which you approved) is to do ✨ [PP-4] Present all component instances that fail to render using toasts Active using Radix UI toasts to spend zero time on a temporary solution → , but to allow the design team to be better informed when designing this.

It would be great if the component was wrapped with the HTML comment so that it would be possible to see what is the name of the component in XB.

That's a trivial change to \Drupal\experience_builder\Element\RenderSafeComponentContainer::handleComponentException(), which is why it was built that way 👍 However, why not just add a data-component-id attribute in addition to the current data-component-uuid?

  public static function handleComponentException(\Throwable $e, string $componentContext, bool $isPreview, string $componentUuid): array {
    \Drupal::logger('experience_builder')->error(\sprintf('%s occurred during rendering of component %s in %s: %s', $e::class, $componentUuid, $componentContext, $e->getMessage()));
    if ($isPreview) {
      return [
        '#type' => 'container',
        '#attributes' => [
          'data-component-uuid' => $componentUuid,
        ],
        '#markup' => new TranslatableMarkup('Component failed to render, check logs for more detail.'),
      ];
    }
    return [
      '#type' => 'container',
      '#attributes' => ['data-component-uuid' => $componentUuid],
      '#markup' => new TranslatableMarkup('Oops, something went wrong! Site admins have been notified.'),
    ];
  }