Problem/Motivation

In UI Patterns 2, we didn't care so much about required props because it was managed by the Form API:

  1. if a prop is required, we add the #required render property
  2. ComponentForm can be submitted without filling a value
  3. So it is not possible to render a component without required props filled

In Display Builder, we can render component without subimiting ComponentForm:

  • on preview panel
  • on builder panel
  • also on final dsipaly rendering if the component was only dropped without any change in ContextualForm

So we have this fatal error:

Twig\Error\RuntimeError: An exception has been thrown during the rendering of a template The property xxx is required."

Proposed resolution

Add default values to required props.

There is 2 places where we can do this:

  • ComponentLibraryPanel: this is the best place in my himble opinion, because the logic is executed only once by buildign session and can be easily cached
  • Instance::attachToRoot and Instance::attachToSlot, so executed at each attachment

The logic, if the prop is required

  1. Set the default value if exist
  2. Set the first examples value if exist

We can go further, but it would be come complicated.

Other tasks

Do we need to add the rule in sdc_devel: "each required prop must have a default value" ?

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:

Comments

pdureau created an issue. See original summary.

pdureau’s picture

The MR draft is still in an early, naive, state:

  • no check of examples property
  • performance issues, i guess, because component plugins are instantiated many times
  • we presume the default source of each prop type is a widget with value configuration, but it would be better to check
  • phpunit fail
  • ...

But it is enough for you to test the idea with proejcts heavily relying on required props in components, like the Oliveor theme of Drupal CMS.

pdureau’s picture

Assigned: Unassigned » pdureau

Try again

pdureau’s picture

Assigned: pdureau » Unassigned
Status: Needs work » Needs review
pdureau’s picture

Assigned: Unassigned » pdureau
Status: Needs review » Needs work
pdureau’s picture

Title: Fatal: Default value for component props » Fatal: Default value for props, on load
Assigned: pdureau » mogtofu33
Status: Needs work » Needs review

Tested by Dang Tran (before the logic has been moved to the helper), it was OK

The helper asked this morning has been done but:

  • I don't know how to dependency injection with this kind of "raw" classes (not a service, not a plugin...)
  • No static method because of the injected depedencies
  • No unit or kernel test

Follow-up #3549761: Fatal: Default value for props, on update

mogtofu33’s picture

Status: Needs review » Needs work

Will fix the helper and add unit test.

mogtofu33’s picture

Assigned: mogtofu33 » Unassigned
Status: Needs work » Fixed

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

  • mogtofu33 committed 98fe1e4b on 1.0.x authored by pdureau
    [#3549348] fix: Fatal: Default value for props, on load
    
    By: pdureau
    By...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.