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
- Install a component like the one in the tests for this MR
core/modules/sdc/tests/modules/sdc_test/components/array-to-object
- 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.
Comment | File | Size | Author |
---|---|---|---|
#2 | error.patch | 12.59 KB | sharkbaitdc |
Issue fork drupal-3365480
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:
- 3365480-sdc-array-value compare
- 11.x changes, plain diff MR !4141
Comments
Comment #2
sharkbaitdc CreditAttribution: sharkbaitdc commentedHere's the error that I found when there's an empty object
Comment #3
carolpettirossi CreditAttribution: carolpettirossi at ImageX commented@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 :)
Comment #4
ctrlADelAfter 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
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.
Comment #6
ctrlADelOpened 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.
Comment #7
smustgrave CreditAttribution: smustgrave at Mobomo commentedCan the issue summary be updated with steps to reproduce and the proposed solution please.
Comment #8
e0ipsoUpdated summary and added a test only patch.
Comment #9
e0ipsoWrong parent, sorry.
Comment #10
e0ipsoAdmitedly, this should be a stable blocker.
Comment #12
e0ipsoSuggesting 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)
Comment #14
smustgrave CreditAttribution: smustgrave at Mobomo commentedHiding 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.
Comment #15
eojthebraveFix typo in issue title.
Comment #16
sharkbaitdc CreditAttribution: sharkbaitdc commentedI 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.
Comment #19
lauriiiCommitted 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.