harttle / liquidjs

A simple, expressive, safe and Shopify compatible template engine in pure JavaScript.
https://liquidjs.com
MIT License
1.5k stars 237 forks source link

Sequential `else` tags all render #670

Closed joel-hamilton closed 6 months ago

joel-hamilton commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

@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.

github-actions[bot] commented 6 months ago

:tada: This issue has been resolved in version 10.10.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket: