Shopify / theme-check

The Ultimate Shopify Theme Linter
Other
337 stars 95 forks source link

Prevent undefined object errors (UndefinedObject) not accounting for default value #527

Closed qedric closed 2 years ago

qedric commented 2 years ago

<div class="{{ class | default: '' }}">

This is causing a false positive for the undefined object error. If there's a default pipe, surely this should be ignored?

charlespwd commented 2 years ago

An oversight. I'm assuming this comes from a snippet file and the class param can be omitted?

charlespwd commented 2 years ago

In the meantime, you can do one of the following:

qedric commented 2 years ago

Exactly, a render tag without the class Param, Q

Le lun. 6 déc. 2021 à 16:04, Charles-Philippe Clermont < @.***> a écrit :

An oversight. I'm assuming this comes from a snippet file and the class param can be omitted?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Shopify/theme-check/issues/527#issuecomment-986859678, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMSM6I3YSLDJ2HSRSRINUWLUPTGJFANCNFSM5JOUSYRQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

qedric commented 2 years ago

Will do, thanks :)

Le lun. 6 déc. 2021 à 16:06, Charles-Philippe Clermont < @.***> a écrit :

In the meantime, you can do one of the following:

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Shopify/theme-check/issues/527#issuecomment-986861219, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMSM6I42VZJBO7RJJWA2A7DUPTGO3ANCNFSM5JOUSYRQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

charlespwd commented 2 years ago

Still going to fix it, but just learned from a unit test that this wouldn't flag an error:

{% assign class = class | default: '' %}
<p class="{{ class }}">...</p>

It's one way to declare your variables. I still think the magic comment would be better though. But this has the benefit of already being supported :)