`assign` tag mutates cached template context

[ashmaroli]

The source-code documentation suggests caching Liquid templates and using them to render multiple data: https://github.com/Shopify/liquid/blob/0847bf560fbb99d20410408c27d750b7b412d9fa/lib/liquid/template.rb#L9-L10

Fair enough. But if a cached template contains an assign tag, rendering it mutates the template's context for subsequent renders.

This may be illustrated with the following Ruby script and its output:

Script

require 'liquid'

CONTENTS = <<~TEXT

       Neo: "I am just {{ title | default: 'a regular guy' }}.."
  {%- if agent == '   Smith' %}{% assign title = 'A DISEASE' %}{% endif %}
  {{ agent }}: "Neo, you're {{ title | default: 'The One' }}!!"
TEXT

TEMPLATE = Liquid::Template.parse(CONTENTS)

puts TEMPLATE.render("agent" => "Morpheus")
puts TEMPLATE.render("agent" => " Trinity")
puts TEMPLATE.render("agent" => "  Oracle")
puts TEMPLATE.render("agent" => " Trinity")
puts TEMPLATE.render("agent" => "   Smith")
puts TEMPLATE.render("agent" => "  Oracle")
puts TEMPLATE.render("agent" => " Trinity")
puts TEMPLATE.render("agent" => "Morpheus")

Output

     Neo: "I am just a regular guy.."
Morpheus: "Neo, you're The One!!"

     Neo: "I am just a regular guy.."
 Trinity: "Neo, you're The One!!"

     Neo: "I am just a regular guy.."
  Oracle: "Neo, you're The One!!"

     Neo: "I am just a regular guy.."
 Trinity: "Neo, you're The One!!"

     Neo: "I am just a regular guy.."
   Smith: "Neo, you're A DISEASE!!"

     Neo: "I am just A DISEASE.."
  Oracle: "Neo, you're A DISEASE!!"

     Neo: "I am just A DISEASE.."
 Trinity: "Neo, you're A DISEASE!!"

     Neo: "I am just A DISEASE.."
Morpheus: "Neo, you're A DISEASE!!"

Workaround

The workaround is to explicitly reset the assigned variable in an else tag

{% if foo %}
  {% assign bar = 'lipsum' %}
{% else %}
  {% assign bar = '' %} (or {% assign bar = false %})
{% endif %}

But I feel that the workaround is just a hack and that there shouldn't be a persisting mutation in the first place.

[pushrax]

This seems to be due to https://github.com/Shopify/liquid/blob/c69a9a77c6a02962a35b377111606576ddb00693/lib/liquid/template.rb#L154-L164

combined with https://github.com/Shopify/liquid/blob/c69a9a77c6a02962a35b377111606576ddb00693/lib/liquid/template.rb#L196-L202

Should probably copy those hashes into the context rather than using them directly.

[pushrax]

You can also work around this manually in Ruby by passing a Liquid::Context to render or with MY_TEMPLATE.assigns.clear between renders (for all those hashes).

[ashmaroli]

Thank you for responding @pushrax. I currently have a workaround proposed for the downstream project similar to your second suggestion:

@template ||= Liquid::Template.parse(contents, opts)

@template.instance_assigns.clear
@template.render!(*args)

Was waiting for an official comment or workaround from you guys.. If this is not further actionable even for the next major version of Liquid, you may close this ticket.

[dylanahsmith]

Should probably copy those hashes into the context rather than using them directly.

I agree. Although, that could mean needing to deeply duplicate the values in those hashes if we want to avoid problems from the value being mutated during rendering.

If conceptually the template is just the result of parsing and rendering shouldn't mutate its state, then it also seems wrong to expose render errors on it through Template#errors. Perhaps we should deprecate that in favour of passing in a context and getting errors from it. Similarly, we should extract the mutable state from ResourceLimits into a separate object to use in the context, so inspecting the final scores would need to be done on the context after rendering.