content_for failing to capture the correct block input

sdogruyol commented 2 years ago

Related to https://github.com/kemalcr/kemal/issues/636, I discovered that content_for is not capturing and yielding the correct block input inside the ecr view. It weirdly captures the whole content of the file

Take this for example:

src/views/home.ecr

Hello <%= name %>

<% content_for "meta" do %>
<title>Kemal Spec</title>
<% end %>

src/views/layout.ecr

<html>
    <head>
        <%= yield_content "meta" %>
    </head>
    <body>
        <%= content %>
    </body>
</html>

src/kemal-bug.cr

require "kemal"

get "/" do
  render "src/views/home.ecr", "src/views/layout.ecr"
end

Kemal.run

When you go to / you'd expect the correct output with <meta> tag inside the <head> tag however this is the following output.

<html>
    <head>
        Hello world

               <title>Kemal Spec</title>
    </head>
    <body>
        Hello world
    </body>
</html>

the Hello world outside of the block in src/views/home.ecr is also captured by the content_for block and then yielded again which leads to duplicate content. For more info here's where it's yielded in the content_for macro https://github.com/kemalcr/kemal/blob/master/src/kemal/helpers/macros.cr#L37

I think the wrong capture of the block might be a bug in Crystal itself in ECR, WDYT @straight-shoota?

straight-shoota commented 2 years ago

ECR generates Crystal code like this for the content template:

content_io << "main"
content_for "title" do
  content_io << "block"
end

The content_for macro expands to creating a Proc with the body of the macro block. When that block is called from yield_content, it returns the value of the last expression (content_io << "block") which is the IO content_io. At this point, it already contains all the previously appended content ("main" in this example) and thus it is part of the return value.

In order to achieve the intended behaviour here, the proc call needs to be wrapped by setting content_io to a fresh instance. Something like this should work:

if __content_filename__ == __caller_filename__
  %old_content_io, content_io = content_io, IO::Memory.new
  %proc.call
  %result = content_io.to_s
  content_io = %old_content_io
  %result
end

sdogruyol commented 2 years ago

You rock @straight-shoota 🤘 That worked like a charm 👍

kemalcr / kemal

content_for failing to capture the correct block input #639