Shopify / Shopify/liquid

Custom Liquid::Block tag not working properly inside a for loop tag

Open
#698 5 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

Guys, I'm working with a code that gave me an strange behaviour. For example, I have a liquid's post view that renders many comments and its edit's forms, those comments are inside a snippet with a custom tag form.

In this custom tag, the code initialize the @form_type that is used to define the form's path and other things.

Now, the strange thing! This tag works properly outside the for loop, but in the for the initialize function is called just once and the render function is called correctly (x loop times). The problem with it is that in the second time all instance variables defined in the custom tag are nil. With this behaviour the form_by_type function does not return anything and the final html form will be without action, class, id.....

Let me show you some example code:

<!-- post.liquid  -->
{% for comment in post.comments %}
   {% include 'post/comment', c: comment %}
{% endfor%}
<!-- snippets/post/comment.liquid  -->

<div id="comment-{{c.id}}">
  <div class="comment-text">
    {{ collaborative_comment.text }}
  </div>

  <div class="edit-comment hide">
    {% form 'edit_comment', remote: true %}
      <textarea rows="10" cols="80" name="comment[text]">{{c.text}}</textarea>
      <input type="submit" value="Update">
    {% endform %}
  </div>
</div>
# tags/form_tag.rb

class FormTag < Liquid::Block
  include RenderView

  def initialize(tag_name, markup, options)
    super

    @form_html_options = {}
    @form_type         = markup.split(',')[0].strip

    markup.scan(Liquid::TagAttributes) do |key, value|
      @form_html_options[key.to_sym] = value
    end
  end

  def render(context)
    @context    = context
    @form_type  = @context[@form_type]
    @form       = initialize_form
    @resource   = @context[@form[:resource_key]]
    @form_token = @context['authenticity_token']

    @context.stack do
      @context['form'] = {
        'notice' => @context['notice'],
        'errors' => @resource ? @resource['errors'] : @context['alert']
      }

      @html_block = render_all(@nodelist, @context)
    end

    render_view('form')
  end

  private

  def initialize_form
    resource_key, url, method = form_by_type

    {
      resource_key: resource_key,
      url: url,
      method: method
    }
  end

  def form_by_type
    case @form_type
    when 'login'
      [ 'user', '/users/sign_in', :post ]
    when 'edit_comment'
      comment_path = "/comments/#{@context['c']['id']}"
      ['edit_comment', comment_path, :put]
    end
  end

  def form_method
    return :post if form_special_methods?
    @form[:method]
  end

  def form_special_methods?
    @form[:method] == :patch || @form[:method] == :put || @form[:method] == :delete
  end
end

Liquid::Template.register_tag('form', FormTag)
<!-- app/views/tags/form_tag/form.html.erb -->
<form accept-charset="UTF-8"
      id="<%= @form_html_options[:id] %>"
      class="<%= @form_html_options[:class] %>"
      method="<%= form_method %>"
      action="<%= @form[:url] %>"
      data-remote="<%= @form_html_options[:remote] || false %>">
  <%= hidden_field_tag :authenticity_token, @form_token %>

  <% if form_special_methods? %>
    <%= hidden_field_tag("_method", @form[:method]) %>
  <% end %>

  <%= @html_block.html_safe %>
</form>

I can do this work putting the instance variables initialization inside the render function, but I want to know if is right do this and if it have some bad implications. Or if this code have something wrong that are causing this behaviour.

Thanks...

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 with tags/form_tag.rb and the Liquid::Block lifecycle, then compare how initialize and render are invoked for a custom tag inside a for loop. Use the supplied post.liquid and snippets/post/comment.liquid examples to reproduce the repeated render behavior; done means establishing the supported instance lifecycle and documenting or correcting the observed state handling.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.