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

content_for failing to capture the correct block input #639

Merged
merged 2 commits into from
Jun 27, 2022
Merged

content_for failing to capture the correct block input #639

merged 2 commits into from
Jun 27, 2022

Conversation

sdogruyol
Copy link
Member

@sdogruyol sdogruyol commented Jun 26, 2022 •
edited
Loading

Related to #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?

@sdogruyol sdogruyol changed the title content_for failing to capture the correct block content_for failing to capture the correct block input Jun 26, 2022
@straight-shoota
Copy link
Contributor

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 sdogruyol added the bug label Jun 27, 2022
@sdogruyol
Copy link
Member Author

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

@sdogruyol sdogruyol merged commit 317d086 into master Jun 27, 2022
@sdogruyol sdogruyol mentioned this pull request Jun 27, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sdogruyol @straight-shoota