Shopify / prettier-plugin-liquid

Prettier Liquid/HTML plugin
https://npm.im/@shopify/prettier-plugin-liquid
MIT License
93 stars 15 forks source link

Allow at most 2 unclosed nodes of the same type (open/open or close/close) in Liquid branches #186

Closed charlespwd closed 1 year ago

charlespwd commented 1 year ago

In this PR

Rationale

90 has been opened for a while. A lot of folks intentionally add unclosed nodes inside Liquid if statements. This PR allows this to happen while still throwing errors for the cases that should be errors.

That is, we're being slightly more lenient with what we allow to be parsed as a "full tree."

It should pretty print dangling open tags inside if statements
printWidth: 1
{% if condition %}
  <div
    attr="value"
    other="{{ product.id }}"
  >
{% else -%}
  <product-wrapper
    id="{{ product.id }}"
  >
{%- endif %}

It should pretty print dangling close tags inside if statements
printWidth: 1
{% if condition %}
  </div>
{% else -%}
  </product-wrapper>
{%- endif %}

It should pretty print dangling close tags inside unless statements
printWidth: 1
{% unless condition %}
  </div>
{% else -%}
  </product-wrapper>
{%- endunless %}

It should allow two nodes as well, without indenting
printWidth: 1
{% if condition %}
  <div
    attr="value"
    other="{{ product.id }}"
  >
  <p
    attr="value"
  >
{% endif %}

It should format as expected, Dawn example...
printWidth: 80
<div>
  {%- if block.settings.make_collapsible_row -%}
    <details id="Details-{{ block.id }}-{{ section.id }}" open>
    <summary>
  {%- endif %}
  ...
  {%- if block.settings.make_collapsible_row -%}
    </summary>
  {%- endif %}
  ...
  {%- if block.settings.make_collapsible_row -%}
    </details>
  {%- endif %}
</div>

Heuristic used

The logic used is as follows:

:+1: Now allowed:

{% if cond %}<a>{% endif %}
{% if cond %}<a><b>{% endif %}
{% if cond %}<a><b>{% else %}<c>{% endif %}
{% if cond %}<a><b>{% elsif cond %}<c><d>{% else %}<e>{% endif %}
{% if cond %}</a></b>{% elsif cond %}</c></d>{% else %}</e>{% endif %}
{% if cond %}</a></b>{% else %}</c>{% endif %}
{% if cond %}</a></b>{% endif %}
{% if cond %}</a>{% endif %}

:-1: Still recognized as error:

{% if cond %}<a>text{% endif %}
{% if cond %}<a>{{ 'output' }}{% endif %}
{% if cond %}<a>{{ 'output' }}</a><b>{% endif %}
{% if cond %}<b><a>{{ 'output' }}</a>{% endif %}

{% # 3 feels like a strech %}
{% if cond %}<a><b><c>{% endif %}

{% # etc. %}

Style choice

I decided to treat the unclosed nodes the same way we treat nodes that do not have a closing tag (e.g. <img>). That is, we won't mess with the indentation in reaction to them.

Consequences:

<div>
  {% if summary %}
    <detail attr=...>
    <summary>
  {% endif %}

  {{ summary }}

  {% if summary %}
    </summary>
  {% endif %}

  {{ content }}

  {% if summary %}
    </detail>
  {% endif %}
</div>

Which feels awkward as hell, but tbh it already was :sweat_smile:

Reasoning:

If someone does something funky like the above, but there's also an if branch with only one child, deciding which indentation of the siblings should have would get wonky real fast.

<div>
  {% if summary %}
    <detail attr=...>
      <summary>
  {% else %}
    <content-wrapper>
  {% endif %}

    # here?
    {{ summary }}

      # or here?
      {{ summary }}

    # what do we do with this?
    {% if summary %}
      </summary>
    {% endif %}

      # oh my...
      {{ content }}

  {% if summary %}
    </detail>
  {% else %}
    </content-wrapper>
  {% endif %}
</div>

Fixes #90