Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Liquid::UndefinedVariable is raised for defined attribute with nil value, when strict_variables: true #749

Closed
nijikon opened this issue Apr 25, 2016 · 4 comments
Closed

Liquid::UndefinedVariable is raised for defined attribute with nil value, when strict_variables: true #749

nijikon opened this issue Apr 25, 2016 · 4 comments

Comments

@nijikon
Copy link

nijikon commented Apr 25, 2016 

2.3.0 :001 > Liquid::Template.parse("{{foo}}").render!({ 'foo' => nil}, strict_variables: true)
Liquid::UndefinedVariable: Liquid error: undefined variable foo

Is this desired behaviour?

We want to be strict for variables being defined, nil value is allowed because we handle that with an if.
nijikon added a commit to polishgeeks/liquid that referenced this issue Apr 25, 2016
@nijikon
handle nil value for key when strict_variables is on, Fixes Shopify#749
bcfaba5
@nijikon nijikon mentioned this issue Apr 25, 2016
Open
@dylanahsmith
Copy link
Contributor

That seems like a bug to me.

@kke kke mentioned this issue Feb 27, 2017
Merged
SpComb added a commit to kontena/kontena that referenced this issue Mar 1, 2017
@SpComb
CLI: Fix stacks YAML reader handling of undefined variables (#1884)
ead9f17 
The current CLI stacks YAML reader uses `Liquid::Template.render(..., strict_variables: true)`. This means that that any references to undefined variables will omit the tag from the output and push an error message into `template.errors`. The CLI ignores those errors, which cascades into other difficult to debug issues when parts are missing from the resulting YAML:

* #1846
* #1890

A Liquid bug also means that optional `required: false` stack variables also behave as undefined variables: Shopify/liquid#749

This PR fixes the CLI stack YAML reader to:

* Workaround Liquid bugs to treat optional  `required: false` stack variables as falsey values for Liquid conditional blocks

* Fail `kontena stack ...` fast with a  `Liquid error: undefined variable ... ` message if the YAML contains invalid variable references.

    These errors would be nicer if they also contained the source YAML `file:line`, but that is missing from the `Liquid::UndefinedVariable` error...
@pascalbetz
Copy link
Contributor

We currently have the same problem. Variable is defined but nil and this causes mentioned error.
I think the problem lies with Liquid::Context#find_variable (https://github.com/Shopify/liquid/blob/master/lib/liquid/context.rb#L163).

The check for unless value (https://github.com/Shopify/liquid/blob/master/lib/liquid/context.rb#L174) does not take nil into account which is a valid value when raise_on_not_found is set.

If there is any chance that this gets accepted i'll craft a PR. Let me know.

@fw42
Copy link
Contributor

fw42 commented Aug 18, 2017

Happy to look at a PR

@pascalbetz pascalbetz mentioned this issue Aug 18, 2017
Merged
@alexspeller
Copy link

Looks like this issue was fixed by a979b3e

@pushrax pushrax closed this as completed Feb 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@alexspeller @pascalbetz @nijikon @dylanahsmith @pushrax @fw42