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.

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

When use content_for with yield_content, output duplicate page content. #636

Closed
zw963 opened this issue Jun 17, 2022 · 9 comments
Closed
Labels

Comments

@zw963
Copy link

zw963 commented Jun 17, 2022

Description

We assume have following view code.

image

i render it like this:

get "/" do
  current_user = UserQuery.new.first

  render "src/views/home.ecr", "src/views/layouts/layout.ecr"
end

Will result in:

image

This issue only happen when used content_for in view with yield_content in layout.

if remove one of them, render result correct.

Expected behavior: [What you expect to happen]

I should never render duplicate content twice.

Actual behavior: [What actually happens]

Reproduces how often: [What percentage of the time does it reproduce?]

always.

Versions

Crystal 1.5.0-dev [0123976d8] (2022-06-09)

LLVM: 13.0.1
Default target: x86_64-pc-linux-gnu

Arch Linux.

i can reproduce on 1.4.1 too.

@sdogruyol
Copy link
Member

sdogruyol commented Jun 18, 2022

@zw963 thanks for the report, what's your Kemal version? I remember fixing this behavior, this is the previous issue #630

@zw963
Copy link
Author

zw963 commented Jun 18, 2022

Current, i use version: 1.1.2, but, i can reproduce this on the master, i can reproduce it on d53d253 too

 ╰─ $ shards list
Shards installed:
  * kemal (1.1.2 at d53d253)
  * radix (0.4.1)
  * exception_page (0.2.2)
  * backtracer (1.2.1)
  * avram (0.23.0)
  * lucky_task (0.1.1)
  * pg (0.26.0)
  * db (0.11.0)
  * habitat (0.4.7)
  * wordsmith (0.3.0)
  * dexter (0.3.4)
  * shell-table (0.9.3)
  * pulsar (0.2.3)
  * lucky_cache (0.1.1)
  * splay_tree_map (0.2.2)
  * habitat (0.4.7)
  * cadmium_transliterator (0.1.0 at 46c4c14)

@zw963
Copy link
Author

zw963 commented Jun 18, 2022

I can confirm, my installed kemal version include those changes in #630

@sdogruyol sdogruyol added the bug label Jun 18, 2022
@zw963
Copy link
Author

zw963 commented Jun 25, 2022

Hi, if need more context for this issue? if you want, please tell me, i will add it here.

@sdogruyol
Copy link
Member

@zw963 I tried to reproduce the issue and could not reproduce. Here's the repo that I used to reproduce https://github.com/sdogruyol/kemal-layout-bug, could you please clone and run this on your local and see if it works on your side too?

@zw963
Copy link
Author

zw963 commented Jun 26, 2022

@zw963 I tried to reproduce the issue and could not reproduce. Here's the repo that I used to reproduce https://github.com/sdogruyol/kemal-layout-bug, could you please clone and run this on your local and see if it works on your side too?

Yes, i can, please check following screenshot.

image

BTW: i consider there exists some syntax error on view, i fixed, please check following diff.

 ╰─ $ git diff
diff --git a/src/views/home.ecr b/src/views/home.ecr
index fb85025..df95b8a 100644
--- a/src/views/home.ecr
+++ b/src/views/home.ecr
@@ -1,6 +1,6 @@
 <h1> Title 1</h1>
 <h2> Title 2</h2>
 
-<% content_for "title" %>
+<% content_for "title" do %>
  <title>Inner Title</title>
-<% %>
\ No newline at end of file
+<% end %>

Above diff i create a PR for it. sdogruyol/kemal-layout-bug#1

@sdogruyol
Copy link
Member

Thank you @zw963, I can reproduce it now 👍 I'll look into what's going on

@sdogruyol
Copy link
Member

@zw963 just merged #639 into master, could you please try with that. It should work now 👍

@zw963
Copy link
Author

zw963 commented Jun 27, 2022

@sdogruyol , it works! thank you.

@zw963 zw963 closed this as completed Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants