Disallow SDCs with `type: string, enum: …` with an empty string listed to convey optionality: the prop should then just be optional

Related issue with examples
By default XB adds a 'None' value to the SELECT OPTIONS, which is great like this no need to add a empty string in enum.
But it doesn't seem possible to set this None value as the default value.

What I tried:
- Not to add examples at all
- examples: []
- examples: ['']

All of them return the same error
Twig\Error\RuntimeError: An exception has been thrown during the rendering of a template ("[linno_theme:select/select] Does not have a value in the enumeration ["ratio-32x9","ratio-21x9"]. The provided value is: ""."). in Twig\Template->yield() (line 1 of themes/custom/linno_theme/components/select/select.twig).

I'll look into this.

Hi all, I've created a new component called "CTA Button" to replicate the issue and confirm that it still persists. Below is the component.yml file:

name: CTA Button
description: This is component for adding CTA.
status: stable
props:
  type: object
  properties:
    cta_url:
      type: string
      title: Link URL
      description: Add the url to be used.
      format: uri
      examples:
        - 'https://www.drupal.org'
    cta_title:
      type: string
      title: Link Title
      description: Add the title to be shown on hovering
      examples:
        - 'Learn more'
    cta_icon:
      type: string
      title: CTA icon
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - ''
        - PDF
        - Video
        - External link
        - Arrow icon

I agree with @wim leers and @heyyo's point that

By default XB adds a 'None' value to the SELECT OPTIONS, which is great like this no need to add a empty string in enum.

I also believe the empty string is unnecessary for the component props, as the "None" option provided by Drupal already covers this functionality.

To address the issue as per the comment #3, I removed the empty string value from enum. However, I’d prefer that we accept the "None" option provided by Drupal, rather than adding an empty string in enum.

By removing the empty string, this should also resolve issues with other components. I've attached before and after screenshots of the fix for reference.

Please review the MR changes and let me know if any further adjustments are needed or if there's a better solution to address the issue.

Thanks!

I was very against this, but definitely last IS update in #10 changed my mind. Thanks Wim!
There are examples in core (at least in core tests) using this, so we might need a core issue for fixing those.

The reason is exactly the point that changed my mind:

> This is an abuse misuse of JSON Schema/SDCs. The optionality of a type: string, enum: … should be indicated by NOT listing it as a required prop.

Core will be used as implementation reference for most devs. This is not only about XB support (even this issue should be focused on that and I'm offtopic), but about defining best practices.

Moving back to "needs work" and assigning myself to look into the failing tests.

I've updated the approach here to match the proposed solution, added a new test SDC and updated tests as needed.

After updating the approach, a number of tests are now failing due to them using the SDC sdc_test:my-cta as this is now not compatible with XB. (as it contains an enum with an empty.) It appears it is used in some other tests which now error, due to the tests trying to use it. I'm not clear on the path forward for this yet.

@wim leers, thank you for the detailed feedback and insights — much appreciated!

You took a different approach than what's in the issue summary, @thoward216, and you had me first thinking it was genius, simple and elegant, but … I think I spotted some negative consequences: do you agree?

Yes, I do agree with your assessment. You're absolutely right — I took a different approach by omitting "" from the enum instead of explicitly disallowing it. At first glance, it seemed like a clean and unobtrusive way to sidestep the HTML limitation, but I can now see the downside — particularly in cases where "" is explicitly included in a required field's enum. This results in an unusable form element, which defeats the purpose of validating the SDC in the first place.

After re-reviewing the changes in my merge request, I realized that the current fix does not fully resolve the issue, especially for props like cta_icon1 and cta_icon2.

While the intention was to handle enums with empty strings more gracefully, the approach I took (silently omitting "") doesn't correctly address all problematic cases. For instance:

• cta_icon1 includes "" in the middle of the enum array.
• cta_icon2 includes "" at the end.

Both of these cases still result in invalid for below component props

cta_icon:
      type: string
      title: CTA icon
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - ''
        - PDF
        - Video
        - External link
        - Arrow icon
    cta_icon1:
      type: string
      title: CTA icon 1
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - PDF
        - Video
        - ''
        - External link
        - Arrow icon
    cta_icon2:
      type: string
      title: CTA icon 2
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - PDF
        - Video
        - External link
        - Arrow icon
        - ''

Thanks again for the clear explanation — it’s really helped clarify the situation. I'll adjust the implementation to explicitly disallow "" in enums and ensure proper test coverage for these cases.

Thanks @thoward216 for the quick implementation of the requested changes. I’ve been reviewing the updated merge request with the feedback incorporated, but I’m encountering an issue while testing it with the same component props mentioned in comment #23.

When I use the component props as follows (from the earlier example):

name: CTA Button
description: This is component for adding CTA.
status: stable
props:
  type: object
  properties:
    cta_url:
      type: string
      title: Link URL
      description: Add the url to be used.
      format: uri
      examples:
        - 'https://www.drupal.org'
    cta_title:
      type: string
      title: Link Title
      description: Add the title to be shown on hovering
      examples:
        - 'Learn more'
    cta_icon:
      type: string
      title: CTA icon
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - ''
        - PDF
        - Video
        - External link
        - Arrow icon
    cta_icon1:
      type: string
      title: CTA icon 1
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - PDF
        - Video
        - ''
        - External link
        - Arrow icon
    cta_icon2:
      type: string
      title: CTA icon 2
      description: Choose the icon to be display for cta
      examples: ['PDF', 'Video', 'Arrow', 'External Icon']
      default: Video
      enum:
        - PDF
        - Video
        - External link
        - Arrow icon
        - ''

I did not encounter the specific error message ('Prop "%s" has an empty enum value.', $prop_name) in the ComponentMetadataRequirementsChecker class, after the feedback was incorporated and the changes were made in the MR. However, I am now encountering a different issue in the Experience Builder UI. The error message displayed is: "An unexpected error has occurred while rendering the component's form." I’ve attached a screenshot for reference.
Could you please suggest any specific steps or checks I should follow when creating or testing the component? If there’s anything I might have missed or overlooked in the process, I’d really appreciate it.

Thanks!

@wimleers - thanks for the context and suggested path forward in #22 will take a look into this.

@tirupati_singh - I've pulled in the latest commit locally and tested your component in #24 and it appears to work as expected, it does not show in the component library in experience builder and appears within the "disabled" section under "/admin/appearance/component/status" with the reasons - see screenshot. - Was the component already placed within experience builder and saved?

@wimleers thanks, yes I'm unblocked on this.

Tests are now all passing with the updated approach. Moving to needs review.