Problem/Motivation

I am replacing list_group with a component in my theme, adding some props. This throws an error about missing props schema on the overridden component.

As mentioned in the SDC FAQs:

My component doesn't have props, how do I write its schema?

Component schema is mandatory for validation, and for integration with display building modules like SDC Display or UI Patterns. The way to declare empty props is not by leaving them out but by actually saying they are empty.

This is a similar issue to #3437559: Components need to have empty props to be replaceable.

Steps to reproduce

Replace list_group, add props.

Proposed resolution

Add an empty props schema to the component.yml

props:
  type: object
  properties: {}

Remaining tasks

Add empty props to all components to make it easier for child theme replacement.

Issue fork ui_suite_bootstrap-3630724

Command icon 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:

About issue forks

Comments

scottsawyer created an issue. See original summary.

scottsawyer’s picture

scottsawyer
Primary language English
Location Atlanta
commented

I found these components did not have props schema.

  • badge
  • blockquote
  • card_group
  • list_group
  • progress_stacked
  • toast_container

scottsawyer opened merge request !335

scottsawyer’s picture

scottsawyer
Primary language English
Location Atlanta
commented
Status: Active » Needs review

The PHPStan error is for a test, not sure what is going on there, but it feels like a separate issue.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: Unassigned » pdureau
Category: Bug report » Task

Hi,

Thanks for creating the issue and MR.

With UI Patterns, there is no need to add empty props in components.

@pdureau: Is there already a core issue about removing the need to declare props when not using UI Patterns?

pdureau’s picture

pdureau commented
Assigned: pdureau » Unassigned

@pdureau: Is there already a core issue about removing the need to declare props when not using UI Patterns?

It is not about UI Patterns. There is a peculiar rule in Core SDC API:

  • if a SDC is defined in a module, props can't be missing, so an empty /props/properties is expected
  • if a SDC is defined in a theme, props are optional, like what we do in ui_suite_bootstrap

As far as I know, there is no Core issue about this yet, but the expected change would be to make props optional also in modules instead of the other way around.

So, I would advise to not change anything in ui_suite_bootstrap

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Needs review » Postponed (maintainer needs more info)

Ok.

@scottsawyer you mentioned that the problem happens with a child theme, right?

@pdureau mentioned about modules.

@scottsawyer are you using other modules altering SDC like Canvas too?

scottsawyer’s picture

scottsawyer
Primary language English
Location Atlanta
commented

This is in a child theme. A colleague was updating the site and ran into this error:

Component "my_theme:list_group" is attempting to replace "ui_suite_bootstrap:list_group",
however component replacement requires both components to have schema definitions.

This is from his AI:

Core 11.4.9 now requires props on both sides. Upstream's list_group has none, so I added a new local patch that gives it an empty one: 

 slots:
   items: ...
+# Empty props schema: Drupal 11.4.9+ requires a props schema on both components
+props:
+  type: object
+  properties: {}

Now that I am re-reading this, I think this is specifically because I added props to the replacing component, but the replaced component doesn't have props, hence the schema mismatch.

So, I think what I am asking for is actually a feature request; adding empty props to all components so that if they are replaced, the replacing component can add props.

I will update the IS.

scottsawyer’s picture

scottsawyer
Primary language English
Location Atlanta
commented
Issue summary: View changes
Related issues: +#3437559: Components need to have empty props to be replaceable
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Postponed (maintainer needs more info) » Needs work

Thanks for the info about the usage context.

Then in this case I think it is a Core feature request or bug report. To allow to replace a component without props with a one with props without throwing an error.

scottsawyer’s picture

scottsawyer
Primary language English
Location Atlanta
commented
Related issues: +#3633275: Allow replacing components to add props even when replaced components do not define props.

@grimreaper I opened a core feature request as I don't think this would be considered a bug in that it works as designed. I don't really have a lot of hope that it is actually acted on in the near future, but there it is.

However, as was determined in the other related issue, the simple answer seems to be just adding the empty props object to components that have not already done so. I am not sure what the possible negative consequences might be, but it seems to me that this is a small and reasonable change that at least has precedent for why this would be a welcome feature (not just my personal case).

Is there something lacking in my implementation? Does this add some maintenance burden that I am not aware of?

Thank you for your consideration.