theme-tools icon indicating copy to clipboard operation
theme-tools copied to clipboard

LiquidHTMLSyntaxError false positive with dynamic attributes

Open alibosworth opened this issue 10 months ago • 3 comments

Describe the bug It seems like liquid with dynamic attribute name is considered an error

Source


<img {{ src_attr }}="{{ image | image_url: width: fallback_width }}" />


Screenshot 2024-03-28 at 5 33 02 PM

If i change the attribute name to a fixed string, it doesn't error (img src="...")

Expected behaviour Be able to dynamically set html attribute names in liquid

Actual behaviour Theme-check flags it as an error state

Debugging information -Current Shopify CLI version: 3.58.0

alibosworth avatar Mar 29 '24 00:03 alibosworth

I wonder if it trips because img is a void element (you don't need the self closing thing). IIRC those were supposed to be allowed.

https://github.com/Shopify/theme-tools/blob/main/packages/prettier-plugin-liquid/src/test/html-attributes/index.liquid#L97-L99

We indeed don't have a unit test for "attribute name only" though. That wouldn't be a bad thing to add!

Thanks for reporting!

charlespwd avatar Apr 03 '24 12:04 charlespwd

Confirmed: image

charlespwd avatar Apr 03 '24 12:04 charlespwd

Ah found it: image

The liquidNode rule takes precedence over the attribute ones. And both start with the same rule since an attribute name is (text | liquidDrop)*. So what happens here is that the liquidNode rule is "completed" and doesn't try to parse the = as though it was an attribute. Since the rule is completed, then it starts to try parsing another attribute or a closing tag character. Can't, therefore throws.

It might be simply a matter of kicking liquidNode down the list. But I'll have to think about it a bit more.

charlespwd avatar Apr 03 '24 12:04 charlespwd

👋🏻 Hi @alibosworth, the fix for this has been merged and should be available in the next patch version. Thanks!

lukeh-shopify avatar Sep 06 '24 17:09 lukeh-shopify