harttle / liquidjs

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

is {%else if foo %} valid syntax? #667

Closed amit777 closed 5 months ago

amit777 commented 8 months ago

I noticed this liquid syntax is getting parsed correctly when it shouldn't (notice the space between else and if in else if invalid:

{% assign bar = "bar" -%}
{% if foo %}
FOO
{% else if invalid %}
THIS SHOULD NOT BE
{% else %}
ELSE
{% endif %}

It renders

THIS SHOULD NOT BE

ELSE
amit777 commented 8 months ago

I would have expected the parser to throw an error with this template about the {% else if invalid %}

joel-hamilton commented 8 months ago

I drafted a PR that throws an error in the else if case, but I just noticed that Shopify isn't throwing an error in the same case. They just parse the 'invalid' else if... as an else and ignore following conditions:

image

Should we be seeking to replicate the behaviour of the Ruby library here, or is throwing a parse error desired?

harttle commented 8 months ago

Though we don't replicate many of error scenarios in Shopify, consistency is always good. I think we can leave it as it is, as it's already consistent with Shopify.

joel-hamilton commented 8 months ago

Agreed that throwing an error isn't desired, I'll close the draft PR.

I think the real issue is that liquidjs is rendering multiple else blocks in the same conditional statement. I opened #670