strict_variables throws exception on checking if variable exists

[ptoews]

I'm using a jekyll layout where I want to include content of pages, depending on whether it exists.
Apparently, it is supposed to be done like described in #89
In my case it would look as follows:

{% assign title = page.title %}
{% if page.name %}
    {% assign title = page.name %}
{% endif %}
<title> {{ title }} | {{ site.title }} </title>

But since I have strict_variables set to true, which is really useful for development, Liquid throws an exception on building (undefined variable name included).

In my opinion, strict_variables should not throw exceptions for cases where the undefined variable is checked for existance.

[ptoews]

I've found a workaround for this case, which works only because page is a hash:

{% assign title = page.title %}
{% for key in page %}
    {% if key == "name" %}
        {% assign title = page.name %}
    {% endif %}
{% endfor %}

[pushrax]

This is a really good point, thanks for opening the issue. Some options:

• your suggestion of using if as a key existence operator in strict_variables mode
• new filter ( | has: 'name') or operator (if defined page.name or if page has name) to check existence of a key
• make this work more like a Ruby hash: allow access to missing keys but raise if accessing a key on a missing value

[paleite]

Do we have an update on this?

[mdlincoln]

Just adding my 👍 as we'd love to enable strict_variables on our project (in our case, a Jekyll site) but we rely heavily on checking for variable existence in our templates, so we can't use this as currently implemented.

(thanks also for a great templating library!)

[maxandersen]

Here is another 👍 as I was hoping to use strict_variables too to avoid dumb mistakes - but it doesn't work as existence check is used in not only my templates but also many themes.

I would say to be backwards compatible it should be that page.missing_key is allowed, but page.missing_key.value should give error and then have a more explicit operator to get better checks in new code.

[pushrax]

cc @dylanahsmith @Thibaut IMO this is a bug, and fixing it would make adopting strict_variables possible in Shopify for new templates. Out of the options in #1034 (comment) what do you prefer, or do you have another suggestion?

[dylanahsmith]

I think we need to decide whether we want to expose the concept of undefined as being distinct from nil. Currently, strict_variables has that distinction and it can help with typos or variable renames that miss some usage. However, it also requires either making sure the variable is always initialized (e.g. explicitly set to nil) or that we add something new to explicit check for undefined or handle the undefined value (e.g. using a filter like the default filter).

Alternatively, we could decide we don't really want that concept of undefined and treat undefined as nil. That is basically how strict_variables works. This would also be more consistent with the already recommended way of checking for the existence of a variable. We could still make things more strict about where nil could be used in even in this case, like doing a variable lookup on nil (e.g. page.name when page is nil). We could also try to do static analysis in the future to determine where a variable lookup always returns nil as a way of making it more strict.

I think we should try to avoid mixing concepts for consistency, because it would lead to unexpected behaviour. For instance, if we treat the {% if some_var %} as a special case, then it would lead to the expectation that {{ some_var | default: other_var }} would work. If we special case the default filter as well, then it would seem like other filters might be able to work with an undefined value. So I think we would end up with either special cases that we would have to document or something more like javascript where undefined is an actual value which application code would have to be updated to handle in a backwards compatible way.

• make this work more like a Ruby hash: allow access to missing keys but raise if accessing a key on a missing value

I think this probably makes the most sense, since it will be conceptually consistent with how liquid works without strict_variables where there isn't a distinction between undefined and nil.

[pascalbetz]

Defined and present (truthy/falsey) are two different things, IMHO.

I would prefer to be able to check if:

• a key is defined
• a value from an existing key is present

while having strict_variables: true

To achieve this I had to jump through some hoops:

I added a custom filter

module Filters
  def key(object, element)
    object.key?(element)
  end
end
Liquid::Template.register_filter(Filters)

And then used it in my template like so

{%- assign hasKey = data | key: 'bandwidth' %}
{%- if hasKey and data.bandwidth %}
  Bandwith: {{ data.bandwidth }}
{%- endif %}

template.render!("data" => {}) #=> ""
template.render!("data" => {"bandwidth" => nil}) #=> ""
template.render("data" => {"bandwidth" => 1234 }) #=> "Bandwidth: 1234"

while direct access to undefined keys still raises.

[steveoh]

Has a syntax been chosen?

It seems like there are a lot of good ideas and one needs to be chosen and implemented.

[asbjornu]

@pascalbetz, that custom filter looks exactly like what I'm after – until this issue is resolved, at least. I'm just wondering how you "install" such a custom filter into a Jekyll site. Do I need to create a whole repository, gem, bundle and all to host those 6 lines of code to say plugins: "jekyll-key" in my Jekyll site's _config.yml or can a Jekyll plugin reside "locally" within the Jekyll site, somehow?

[pascalbetz]

Sorry, no experience with Jekyll.

[asbjornu]

This article on Jekyll plugins taught me that plugins can just be dumped in a _plugins folder, so your filter seems to be working in my Jekyll instance now, @pascalbetz. Thanks!

[chuckhoupt]

I've found that the Liquid contains operator works well for existence testing variables. Shouldn't this be the canonical way to test for fields?

Example:

{% if page contains "name" %}
   Name: {{ page.name }}
{% endif %}

[pascalbetz]

Looks like contains should work (according to the code) but the documentation does not mention this.

[pushrax]

contains will work for container types like hashes, but I don't think it will work for drops, which don't respond to include?.

liquid/lib/liquid/condition.rb

Lines 20 to 27 in e83b1e4

	'contains' => lambda do |_cond, left, right|
	if left && right && left.respond_to?(:include?)
	right = right.to_s if left.is_a?(String)
	left.include?(right)
	else
	false
	end
	end,

Though modifying the implementation of contains to call key? if it exists could work. Or we could alias Drop#include? to Drop#key?. All are slightly breaking changes but likely safe.

[chuckhoupt]

Re: docs, Using contains with hashes is mentioned on the Wiki, but not on the gh_pages branch page.

https://github.com/Shopify/liquid/wiki/Liquid-for-Designers#if--else

[Hampei]

I would also like to be able to check if a top-level variable is defined, not just keys on hashes or methods on drops.

{% if defined page %} feels like the best option to make clear that it's a special case where strict_variable checking is not being used and it makes it clear that it's separate from being defined but nil.

[sschuldenzucker]

A brief reminder that this issue is Insanely Stupid (TM) and makes strict_variables unexpectedly useless.
It's like building a house with no door. Or a swimming pool in the desert.
Don't mean to offend anyone, I'm sure you guys are busy and all. It's just clearly, aesthetically, stupid.

[h0jeZvgoxFepBQ2C]

This should be really changed, it doesn't make any sense that you cannot check if a variable exists like this.
Using contains works, but is a pretty awkward solution.

[sschuldenzucker]

Lol, I just read my own comment & I was clearly very annoyed by this back then. Hope you'll accept my apologies. I've implemented a simple variant of this in liquidjs now (see issue linked above) & it's working ok for me. Would be nice if it was a little bit smarter (you can't do OR or stuff atm & passing variables to functions has some weird semantics), but it's bearable and I have static checks.

[h0jeZvgoxFepBQ2C]

Lol, just found my own comment again on this. Please implement a check which doesn't raise exceptions :D (We are using drops btw!)

It would be really cool if we could do some checks like in ruby

pages&.topic1&.subtopic234

[paleo]

I'm not convinced that we need to add an operator. The correct behavior could be: if the variable exists, then strict_variables allows to use it. Values undefined and null should be allowed.

Example, with the following payload:

{
  blue: undefined
}

In the LiquidJS template with strict_variables: true:

{% if blue %} <-- no error
{% endif %}
{% if red %} <-- error here
{% endif %}

[drewlustro]

imo:

• check for existence via if should be possible regardless of whether the variable is defined.
• an exception should be raised when attempting to render the contents of said variable

Thus the following code would be exception-free. last_name is not defined, but it is only used within a guarded if block:

content = "Hello {% if last_name %}{{last_name}}{% else %}placeholder{% endif %}"

template = Liquid::Template.parse(content, error_mode: :strict)
template.render!({}, {strict_variables: true})

Hello placeholder

I'm surprised this behavior is not supported.

[alancleary]

As @pushrax mentioned, the {% if hash contains "key" %} solution doesn't work on drops, including the global site variable in Jekyll. So if you want to check if a variable was defined in your Jekyll site's _config.yml file you would need to check the site drop's keys:

{% if site.keys contains "salutation" %}
{{ site.salutation }}
{% endif %}

[Hampei]

In the end we added our own tag for the top-level variables issue:

module Templating
  class IfAvailableTag < Liquid::Block
    def initialize(tag_name, var_name, tokens)
      super
      @var_name = var_name
    end

    def render(context)
      if context.find_variable(@var_name.strip, raise_on_not_found: false)
        super
      else
        ""
      end
    end
  end
end

Liquid::Template.register_tag("ifavailable", Templating::IfAvailableTag)