Overview
The ESLint and CLI validation doesn't cover example image prop values in component.yml files, where adding a relative path causes issues. The backend schema validation shouldn't let that happen, and the CLI push command should fail, but it currently doesn't: #3586958: Relative image paths are incorrectly allowed as example image prop values in JavaScript component config schema.
Even with that backend issue fixed, it's a much faster feedback if validate with ESLint.
Proposed resolution
Extend @drupal-canvas/eslint-config with a rule in the required config that validates example src values for canvas.module/image props.
The rule should report non-fully-qualified URLs for both single image props and array-of-image props, and the error message should include a short actionable recommendation, for example suggesting a placeholder URL such as https://placehold.co/600x400.
User interface changes
n/a
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | why-not-json-schema.gif | 94.95 KB | wim leers |
Issue fork canvas-3586959
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
Comment #2
balintbrewsComment #4
balintbrewsComment #6
balintbrewsPublished
@drupal-canvas/eslint-config@0.3.0and@drupal-canvas/cli@0.14.0.Comment #8
wim leersI'm basically staying away from "CLI Tool" entirely. But this commit caught my attention. Because it touches upon validation.
I see lots of custom validation logic being introduced in this MR. When this is precisely one of the strengths of SDC's choice to rely on JSON Schema: the validation can be deferred to JSON Schema 😅
IOW: Why not JSON Schema?
(This having landed even made me want to double-check I wasn't misremembering this working.)
Comment #9
balintbrewsBecause ultimately this should be the backend's responsibility, and we're only doing this as quick mitigation and to give faster feedback without having to execute an HTTP request. We don't intend to validate the entire schema here, and we already have a really good infrastructure to do quick checks via ESLint. I believe the backend should be the true authority of the entire schema. Thus the real fix needs to happen in #3586958: JavaScript component config schema validation allows invalid example image prop values.
Comment #10
balintbrewsComment #12
wim leersThis is what I expected 👍 😊 But that's exactly what my concern is: new/custom logic instead of just having the CLI tool use a JSON schema validation package?
Agreed, and the back-end is already doing this validation, isn't it?🙈Oy, I was wrong clearly, I didn't know about #3586958: Relative image paths are incorrectly allowed as example image prop values in JavaScript component config schema yet! And I failed to read the issue summary, too! 😬 Sorry!
EDIT: pushing that forward now: #3586958-6: Relative image paths are incorrectly allowed as example image prop values in JavaScript component config schema.