Shopify / theme-tools

Everything developer experience for Shopify themes
https://shopify.dev/docs/themes
MIT License
48 stars 14 forks source link

theme-check(ValidJSON) mistakenly highlights plural with html as error #357

Open frej-hobbii opened 2 months ago

frej-hobbii commented 2 months ago

Describe the bug When adding a locale that has html content inside, the theme check highlights it as an error, even though it works as expected in liquid.

When combining _html postfix to render the locale string as HTML, and a plural, the linting gets confused.

locale

  "ticker": {
    "ticker_physical_stores_html": {
      "one": "<strong>{{ count }} huge</strong> store",
      "other": "<strong>{{ count }} huge</strong> stores"
    }
  },

liquid

<div>{{ 'ticker.ticker_physical_stores_html' | t: count: 3 }}</div>
Screenshot 2024-05-14 at 10 30 22 Screenshot 2024-05-14 at 10 30 40

The CLI theme check works as expected, resulting in no errors

Screenshot 2024-05-14 at 10 45 21

Expected behaviour The VS code extension's theme check should work the same as the CLI

Actual behaviour False negative is thrown

Debugging information

Additional context Add any other context about the problem here.

mgmanzella commented 2 months ago

👋 hi @frej-hobbii , thanks for reporting this issue 🙏 you're seeing this behavior because of this keyname: ticker_physical_stores_html

in our translation schemas a keyname with _html postfix expects string types, not JSON. changing the name of the key will solve your issue: Screenshot 2024-05-15 at 3 35 15 PM

steffenagger commented 1 month ago

@mgmanzella that's not really a solution. While this does make the theme-check happy, removing the _html suffix will result in the html inside the translation-strings being escaped. Can you elaborate on how to achieve pluralized translations containing html, while making the theme-check happy? Or perhaps reopen the issue.

pluralized translations prevent translations from being escaped

mgmanzella commented 3 weeks ago

👋 thanks for the clarification, you're right and the fix for this would be a change in theme liquid docs to add this use case in the schema that validates this json