Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sequential else tags all render #670

Closed
joel-hamilton opened this issue Feb 16, 2024 · 3 comments · Fixed by #671
Closed

Sequential else tags all render #670

joel-hamilton opened this issue Feb 16, 2024 · 3 comments · Fixed by #671

Comments

@joel-hamilton
Copy link
Contributor

joel-hamilton commented Feb 16, 2024

When there are multiple sequential else tags, the content inside of eachelse branch is rendered.

Eg:

<p>If</p>
{% if false %}
  don't show
  {% else %}
  show
  {% else %}
  don't show
{% endif %}

<p>Unless</p>
{% unless true %}
  don't show
  {% else %}
  show
  {% else %}
  don't show
{% endunless %}

<p>Case</p>
{% case true %}
  {% when false %}
    don't show
  {% else %}
    show
  {% else %}
    don't show
{% endcase %}

liquidjs renders:

<p>If</p>

  show
  
  don't show


<p>Unless</p>

  show
  
  don't show


<p>Case</p>

    show
  
    don't show


Shopify has inconsistent rendering behaviour here, it renders only the first else branch inside if and unless conditionals, but it renders all else blocks inside of case conditionals.

image

I believe the correct behaviour is to render only the first else block in all conditionals (if, unless and case). I'd be happy to put up a fix for this.

@jg-rp
Copy link
Contributor

jg-rp commented Feb 17, 2024

I believe the correct behaviour is to render only the first else block in all conditionals (if, unless and case).

I agree, possibly with an option to throw an error on superfluous else blocks.

Shopify/Liquid's {% case %} / {% when %} behaviour gets a little worse if {% when %} tags appear after {% else %}.

{% case "x" -%}
  {% when "y" -%}
    a
  {%- else -%}
    b
  {%- else -%}
    c
  {%- when "x" -%}
    d
  {%- when "x" -%}
    e
{%- endcase %}

Shopify/Liquid output

bcde

The else blocks are only rendered if all preceding when expressions evaluate to false, but all truthy when blocks are rendered, no matter the order.

{% case "x" -%}
  {% when "x" -%}
    a
  {%- else -%}
    b
  {%- else -%}
    c
  {%- when "x" -%}
    d
  {%- when "x" -%}
    e
{%- endcase %}

Shopify/Liquid output

ade

@joel-hamilton
Copy link
Contributor Author

@jg-rp good catch, liquidjs currently has some strange behaviour around this as well, mainly that truthy whens will be rendered even after an else, and the else won't be rendered. Fixed in #671, that PR has before/after examples.

Copy link

🎉 This issue has been resolved in version 10.10.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants