Shopify / Shopify/liquid

Setting the context of a liquid variable can cause a namespace collision

Open
#1,930 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Bug Report
Dominant language
Ruby
Stars
11.9k
Forks
1.5k
Avg merge
17h 55m
Merged PRs (30d)
3

Description

I have a drop like this:

class FolderDrop < Liquid::Drop
  def initialize(folder)
    @folder = folder
  end

  delegate :name, to: :@folder

  def children
    @folder.component_instance.children.current.front_end.to_a
  end
end

I have a ComponentInstance drop that exposes a few methods like normal.

In the past, returning an array of ActiveRecord models that respond to to_liquid worked well. Liquid would call to_liquid on each in turn and we'd get the resulting liquid array with liquid objects.

This PR introduced the concept of calling context= on the item before calling to_liquid. This is confusing anyway since how do we know that this common method name isn't already being used? Shouldn't the attribute at least be namespaced (liquid_context)?

https://github.com/Shopify/liquid/pull/1781

In my case I have a column called context and coincidentally set it as a attr_readonly in Active Record. This caused a hard to debug LiquidError: internal so I ended up hacking my copy of liquid to raise the real error which was ReadonlyAttributeError on the column.

So I guess I have a few questions:

  • Why doesn't Liquid allow the real error to be surfaced in a development environment (acknowledging that it's not just used in Rails)?
  • Am I wrong in returning an array of values without calling to_liquid on them myself first?
  • Is the solution to this to namespace the context attribute in liquid?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing Shopify/liquid#1781 and tracing how arrays of objects are handled through context= and to_liquid. Reproduce the namespace collision with a Liquid::Drop and an object exposing context, then determine the expected error handling and API behavior. Done means the questions have a documented decision and corresponding regression coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
backend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.