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

CommentFileSizeAuthor
#8 why-not-json-schema.gif94.95 KBwim leers

Issue fork canvas-3586959

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

balintbrews created an issue. See original summary.

balintbrews’s picture

balintbrews’s picture

Assigned: balintbrews » Unassigned
Status: Active » Needs review

  • balintbrews committed d20871a7 on 1.x
    feat(CLI Tool): #3586959 Validate image prop example URLs in Code...
balintbrews’s picture

Status: Needs review » Fixed

Published @drupal-canvas/eslint-config@0.3.0 and @drupal-canvas/cli@0.14.0.

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.

wim leers’s picture

Status: Fixed » Needs work
StatusFileSize
new94.95 KB

I'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.)

balintbrews’s picture

Why not JSON Schema?

Because 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.

balintbrews’s picture

Status: Needs work » 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.

wim leers’s picture

we're only doing this as quick mitigation and to give faster feedback without having to execute an HTTP request.

This 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?

Because ultimately this should be the backend's responsibility

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.