Overview

This is a follow-up issue that appeared on #3568264: Update code component props schema to support 'array' type in this conversation: https://git.drupalcode.org/project/canvas/-/merge_requests/645#note_706433

In JsonSchemaType, I removed the exception thrown when maxItems is less than 2 to improve the DX, but leave the exception thrown has better DX than just returning NULL.

This 3 messages: https://git.drupalcode.org/project/canvas/-/merge_requests/645#note_709202, describe pretty well what is the error, what the developer gets.

Proposed resolution

Improve the DX for maxItems, or maybe even more things for the schema.

Issue fork canvas-3576578

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

isholgueras created an issue. See original summary.

velmir_taky made their first commit to this issue’s fork.

velmir_taky opened merge request !663

velmir_taky’s picture

velmir_taky commented
Status: Active » Needs review

Improved the InvalidArgumentException message in JsonSchemaType::computeStorablePropShape() — now includes the actual maxItems value and suggests using a non-array type instead.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » wim leers
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Improve the way developers get errors for maxItems » Inform SDC/code component developers of nonsensical `type: array` prop restrictions: `maxItems <2`
Assigned: wim leers » tedbow
Status: Needs review » Reviewed & tested by the community
Related issues: +#3467870: Support `{type: array, …}` prop shapes

It's not "improve", it's "implement at all".

#3467870: Support `{type: array, …}` prop shapes just added that exception as a way to land foundations early and force exactly this to happen later, when complete support for it would be implemented.

tedbow made their first commit to this issue’s fork.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Reviewed & tested by the community » Needs work

working on test failure

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » Unassigned
Status: Needs work » Reviewed & tested by the community

\Drupal\Tests\canvas\Kernel\PropShapeToFieldInstanceTest::test should now pass and phpstan passes

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: Unassigned » tedbow
Status: Reviewed & tested by the community » Needs work

tests failure

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +technical debt

😂👏 “judging Gracie”

Per #6: this is paying off technical debt we accepted to go faster in the summer of last year.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Component: Code » Component sources
Assigned: tedbow » Unassigned
Status: Needs work » Reviewed & tested by the community

The test failure was only due to @tedbow improving the message (thanks! 🙏): the test expectation needed updating to match. He did that, good to go now.

Suggested commit message:

chore(Component sources): #3576578 Inform SDC/code component developers of nonsensical `type: array` prop restrictions: `maxItems <2`

By: velmir_taky
By: wim leers
By: tedbow
By: isholgueras
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Parent issue: » #3520484: [META] Production-ready ComponentSource plugins
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3576805: DX: reorganize test components: `canvas_test_sdc` should have its (intentionally invalid) SDCs moved to `canvas_test_sdc_invalid`

@tedbow's https://git.drupalcode.org/project/canvas/-/merge_requests/663#note_710706 remark is not sufficiently negative — let's fix that distraction: #3576805: DX: reorganize test components: `canvas_test_sdc` should have its (intentionally invalid) SDCs moved to `canvas_test_sdc_invalid`.

  • tedbow committed 6d77b0b1 on 1.x authored by velmir_taky 
    chore(Component sources): #3576578 Inform SDC/code component developers...
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Reviewed & tested by the community » Fixed

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

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

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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