Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: theme check ValidJSON incorrectly reports error on number setting placeholder #4933

Closed
2 tasks done
ivansimplistic opened this issue Nov 27, 2024 · 7 comments · Fixed by Shopify/theme-liquid-docs#855
Labels
Area: @shopify/theme @shopify/theme package issues Type: Bug Something isn't working

Comments

@ivansimplistic
Copy link

Please confirm that you have:

  • Searched existing issues to see if your issue is a duplicate. (If you’ve found a duplicate issue, feel free to add additional information in a comment on it.)
  • Reproduced the issue in the latest CLI version.

In which of these areas are you experiencing a problem?

Theme

Expected behavior

There shouldn't be an issue on this schema fragment:

{ "type": "number", "id": "free_shipping_threshold", "label": "Threshold", "default": 50, "placeholder": "ex: 50", "info": "Leave the field empty (value 0) to disable the Free shipping progress bar." },

Actual behavior

When running shopify theme check I get an error of type ValidJSON , message Incorrect type. Expected "number" for the placeholder field of a setting of type "number".

The vscode extension also reports the same issue.

If I change the placeholder from "ex: 50" to just 50 the error goes away, however I can't upload that to shopify because it returns an error indicating that placeholder must be a string.

Verbose output

shopify theme check output:

  config/settings_schema.json                                                 
                                                                              
                                                                              
  [error]: ValidJSON                                                          
  Incorrect type. Expected "number".                                          
                                                                              
  1923  "placeholder": "ex: 50",                                              
  


When changing the placeholder value to a number, the `shopify theme dev` command can't upload that file because shopify returns this:
`
Failed to upload file config/settings_schema.json:
-Section 18: setting with id="free_shipping_threshold" placeholder must be a string
`

Reproduction steps

Operating System

Windows 11

Shopify CLI version (check your project's package.json if you're not sure)

3.70

Shell

No response

Node version (run node -v if you're not sure)

v20.17

What language and version are you using in your application?

No response

@ivansimplistic ivansimplistic added the Type: Bug Something isn't working label Nov 27, 2024
@gonzaloriestra gonzaloriestra added the Area: @shopify/theme @shopify/theme package issues label Nov 28, 2024
@charlespwd
Copy link
Contributor

Per docs, the placeholder is a placeholder value for the input. I'm not sure a string should be accepted. I think this is working as intended?

@ivansimplistic
Copy link
Author

Actually, only the "default" value must be expressed as number, the docs don't indicate the type of the "placeholder" value in the schema, and shopify only allows type string, so theme check shouldn't return this error.

@charlespwd
Copy link
Contributor

I'll verify internally because the docs say:

A placeholder value for the input.
These values only appear for settings defined in settings_schema.json. They don't appear for settings defined in a section's schema.

Which seem to say it should be a value, but the language is confusing. If the backend throws an error and you are in a deadlock this isn't good.

We should probably update the docs as well if that's the case.

@ivansimplistic
Copy link
Author

Thanks,
Yes, we are in a deadlock, the only workaround is to disable that check for the files that contain placeholder on number inputs.

@charlespwd
Copy link
Contributor

Should be fixed now. The docs/json schemas are pulled independently from the CLI/vscode extension version so you don't need to wait for a new release.

@ivansimplistic
Copy link
Author

it works now.
thank you!

@charlespwd
Copy link
Contributor

Thanks for reporting :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: @shopify/theme @shopify/theme package issues Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants