Problem/Motivation

The JSON serialization process is unable to know when to serialize PHP's empty arrays as {} or as [] in JSON. This is a known gotcha when serializing to JSON. This can cause validation errors during SDC's prop schema validation. In particular, if a prop expects an object (associative array in PHP) it will receive an empty JSON array, and complain about the mismatched types.

This produces the following exception (abbreviated for legibility):

The website encountered an unexpected error. Please try again later.

Drupal\sdc\Exception\InvalidComponentException: [ellipses] Array value found, but an object is required in Drupal\sdc\Component\ComponentValidator->validateProps() (line 190 of core/modules/sdc/src/Component/ComponentValidator.php).
Drupal\sdc\Twig\TwigExtension->doValidateProps(Array, 'olivero:pager') (Line: 112)
Drupal\sdc\Twig\TwigExtension->validateProps(Array, 'olivero:pager') (Line: 42)
__TwigTemplate_1dbebacf17837daaff99a96584e759fe->doDisplay(Array, Array) (Line: 394)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 367)
Twig\Template->display(Array, Array) (Line: 168)
__TwigTemplate_1e024ae630f0bcbb43d72ffc1f96a3db___424038220->doDisplay(Array, Array) (Line: 394)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 367)
Twig\Template->display(Array) (Line: 40)
__TwigTemplate_1e024ae630f0bcbb43d72ffc1f96a3db->doDisplay(Array, Array) (Line: 394)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 367)
Twig\Template->display(Array) (Line: 379)
Twig\Template->render(Array, Array) (Line: 40)
Twig\TemplateWrapper->render(Array) (Line: 53)
twig_render_template('core/themes/olivero/templates/navigation/pager.html.twig', Array) (Line: 372)
Drupal\Core\Theme\ThemeManager->render('pager', Array) (Line: 436)

Steps to reproduce

  1. Install a component like the one in the tests for this MR core/modules/sdc/tests/modules/sdc_test/components/array-to-object
  2. Render the component in a page, like done in the test for this MR core/modules/sdc/tests/src/Kernel/ComponentRenderTest::checkArrayObjectTypeCast

Without this patch, the exception above is produced. As proved by the test-only patch. With the full fix, tests come back green.

Proposed resolution

Let's use fuzzy validation, as supported by the validation library of our choice. This will solve the array to object issue.

Additionally, this MR adds better error messages when the validator correctly complains about NULL values for other types. This is a very common error that has come up during our DX research cases. Adding inline suggestion fixes will improve developer experience significantly for these cases.

Issue fork drupal-3365480

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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ctrlADel created an issue. See original summary.

sharkbaitdc’s picture

Here's the error that I found when there's an empty object

carolpettirossi’s picture

@sharkbaitdc, can you please share the scenario where you found the error above just so I try to reproduce and see if the patch fixes it?

Thanks :)

ctrlADel’s picture

After talking with e0ipso it seems like documenting the way to correctly type empty values is the path forward. There are two main cases that we are likely to encounter

  • Array found expecting Object
  • Null found expecting Anything(string, number, array, etc)

In both cases the solution is to use the |default() filter to set the correct empty value.

Another thought was to try and capture this type of error in the validator and in the error message link to the fix on drupal.org.

ctrlADel’s picture

Status: Active » Needs review

Opened a MR that does two things
- Enables fuzzy type casting for arrays and objects during validation using Constraint::CHECK_MODE_TYPE_CAST from the validation library https://github.com/justinrainbow/json-schema
- Check the error messages for a NULL found error and append additional context that points people to the most likely problem/solution which is that the empty data value and the populated data value have different types. I wasn't able to find an easy way to check the schema against the props which is why I checked the error message text instead of a more thorough approach.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Can the issue summary be updated with steps to reproduce and the proposed solution please.

e0ipso’s picture

Title: SDC Array value found, but an object is required » [SDC] Improve error handoing during prop validation errors
Version: 11.x-dev » 10.1.x-dev
Component: theme system » render system
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update
Parent issue: » #3345922: Single Directory Components module roadmap: the path to beta and stable
FileSize
2.51 KB

Updated summary and added a test only patch.

e0ipso’s picture

e0ipso’s picture

Issue tags: +sdc-stable-blocker

Admitedly, this should be a stable blocker.

e0ipso credited sarahjean.

e0ipso’s picture

Suggesting credit to @sarahjean for the report in #3365455: Unhelpful error message with SDC when a optional variable is missing.

(Apologies if I should not be using the manual credit input text, but I didn't want the credit suggestion to be missed)

Status: Needs review » Needs work

The last submitted patch, 8: 3365480--8--test-only.patch, failed testing. View results

smustgrave’s picture

Status: Needs work » Reviewed & tested by the community

Hiding the tests-only patch so the review bot doesn't send back to NW.

Issue summary also looks good! Thanks for that.

Reviewing MR 4141 and following the steps of the issue summary can confirm the issue and that it has been fixed.

eojthebrave’s picture

Title: [SDC] Improve error handoing during prop validation errors » [SDC] Improve error handling during prop validation errors

Fix typo in issue title.

sharkbaitdc’s picture

I was converting pager into a sdc and one of the variables passed is the ellipse object which can be empty depending on the page you’re on.

  • lauriii committed 80174a6c on 11.x
    Issue #3365480 by ctrlADel, e0ipso, sharkbaitdc, smustgrave, sarahjean,...

  • lauriii committed 046cefe4 on 10.1.x
    Issue #3365480 by ctrlADel, e0ipso, sharkbaitdc, smustgrave, sarahjean,...
lauriii’s picture

Status: Reviewed & tested by the community » Fixed

Committed 80174a6 and pushed to 11.x. Thanks!

Cherry-picked to 10.1.x because it's a non-disruptive bug fix to an experimental module.

Status: Fixed » Closed (fixed)

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