Shopify / liquid

Liquid markup language. Safe, customer facing template language for flexible web apps.
https://shopify.github.io/liquid/
MIT License
11.05k stars 1.38k forks source link

Don't parse nodes inside a comment tag #1755

Closed ggmichaelgo closed 10 months ago

ggmichaelgo commented 10 months ago

depends on https://github.com/Shopify/liquid-c/pull/209

What are you trying to solve?

Liquid Tags inside a comment tag are getting parsed, and any Liquid tags are required to be valid.

require 'liquid'

template = Liquid::Template.parse(<<~LIQUID)
  {% comment %}
    {% assign foo = "123" %}
  {% endcomment %}
  {{ foo}}
LIQUID

puts template.root.nodelist[0] # => Liquid::Comment
puts template.root.nodelist[0].nodelist[1].class # => Liquid::Assign

Because of this parsing behaviour, the Liquid codes inside the comment tag has to be valid. A known workaround of this issue is using a raw tag inside the comment tag:

{% comment %}{% raw %}
  {% if WIP ... %}
{% endraw %}{% endcomment %}

How are you solving this?

This PR updates the comment tag to only consume the tokens without creating child nodes.

require 'liquid'

template = Liquid::Template.parse(<<~LIQUID)
  {% comment %}
    {% assign foo = "123" %}
  {% endcomment %}
  {{ foo}}
LIQUID

puts template.root.nodelist[0] # => Liquid::Comment
puts template.root.nodelist[0].nodelist.empty? # => true

This change will improve the comment tag's Template parsing speed, and allow developers to have incomplete code inside the comment tag.

{% comment %}
  {% if 1 > 0 %}
    incomplete code...
{% endcomment %}

Legacy Support

Inside a comment tag, children comment tag and raw tags will have be valid and closed.

This comment tag inside a comment will have to be closed because {% endcomment %} tag delimiters needs to be balanced. (Closing the first comment tag in line 4 will result a syntax error)

{% comment %}
  {% comment %}
    child comment
  {% endcomment %}
{% endcomment %}

This raw tag inside a comment will have to be closed because {% endcomment %} inside a raw tag has been ignored:

{% comment %}
  {% raw %}
    {% endcomment %}
  {% endraw %}
{% endcomment %}
ggmichaelgo commented 10 months ago

I'm not sure if this is within the scope of this PR, but a broken tag inside of the comment still fails.

require "liquid"

template = Liquid::Template.parse(<<~LIQUID)
  {% comment %}
    {% assign foo = "123"
  {% endcomment %}
  {{ foo}}
LIQUID

@peterzhu2118 Ah... great catch! That should be a valid template, and I will update the PR to allow templates like that :)