Shopify / theme-tools

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

Warn against block.id usage #595

Closed miazbikowski closed 4 days ago

miazbikowski commented 1 week ago

What are you adding in this PR?

Part of https://github.com/Shopify/theme-tools/issues/463

We want to warn developers against the use of block.id in their liquid code in cases where the ID's instability could cause problems. This PR addresses usage in if, elseif, unless and case blocks. (I've got some comments in the code to demonstrate the cases)

Note: did not end up doing assign as a variable can have valid use cases (like those in my code comments)

Follow ups

Dev docs PR

What did you learn?

console.log will break your code and not tell you why. This is because of the language server.

Before you deploy

navdeep5 commented 4 days ago

🎩 ed and everything looks amazing! Quick follow-up question, why would this not have a warning: image

miazbikowski commented 4 days ago

🎩 ed and everything looks amazing! Quick follow-up question, why would this not have a warning

@navdeep5 and works just like or and there is a warning

image
charlespwd commented 4 days ago

@navdeep5 x in col is not valid liquid. It should be col contains x. It's not working for that because the markup doesn't parse/goes to the fallback rule of having markup as a string.