search

Shopify / theme-tools

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

JSON formatting issues with the Shopify Github Integration #373

Open eddysims opened 3 months ago

eddysims commented 3 months ago

Describe the bug

When using the shopify github integration, I am getting back file urls as an escaped string. Then when running prettier, it is removing the \ from the url.

Unformatted source

shopify:\/\/shop_images\/Logo_2bb829935e.webp

Expected output

shopify:\/\/shop_images\/Logo_2bb829935e.webp

Actual output

shopify://shop_images/Logo_2bb829935e.webp

Debugging information

Additional context

Here is my prettier config:

//.prettierrc.json
{
  "printWidth": 120,
  "singleQuote": true,
  "overrides": [
    {
      "files": "*.liquid",
      "options": {
        "singleQuote": false
      }
    }
  ],
  "plugins": ["@shopify/prettier-plugin-liquid", "prettier-plugin-tailwindcss"]
}
charlespwd commented 3 months ago

Known issue, getting fixed soon 🙏 It's a platform thing. The platform was putting them there to avoid XSS attacks in the case the JSON was dumped in an HTML document.

e.g.

{
  "thing": "</script><script>console.log('hello world');</script>"
}
<html>
  <script type="application/json">
    {
      "thing": "</script><script>console.log('hello world');</script>"
    }
  </script>
</html>

That would actually run the code in the JSON document here. The fix? Backslash all the / in the JSON doc so that the script can't run when dumped in the HTML doc. 

{
  "thing": "<\/script><script>console.log('hello world');<\/script>"
}
<html>
problem solved!
  <script type="application/json">
    {
      "thing": "<\/script><script>console.log('hello world');<\/script>"
    }
  </script>
</html>

Thing is, most of our "platform JSON" docs you can't dump directly into your HTML docs, so it's an unnecessary precaution. But we gotta do it carefully.

charlespwd commented 3 months ago

Closing as duplicate

eddysims commented 3 months ago

@charlespwd I see you closed this as duplicate. Do you have a reference to another place I can follow along with this?

charlespwd commented 3 months ago

It's an internal issue. Afraid I can't. This is a platform bug :/ the prettier plugin is working as intended 😅

It is on our roadmap to fix this though. You're definitely not the only one annoyed by that.

knjshimi commented 3 months ago

Hey @charlespwd , so it's completely fine to let prettier format the json file (unescaping forward dashes) when working with local themes, right?

eddysims commented 3 months ago

@charlespwd Thank you for the update

zrisher commented 3 months ago

Wish this wasn't closed. Had the same issue, took a while to find this because it wasn't listed as an open issue.

Even if Shopify has an internal issue to track this, it's helpful to keep it open for the general public to track until it is fixed.

charlespwd commented 3 months ago

Fair!