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

Partial is rendered twice #146

Closed
euclio opened this issue Apr 12, 2017 · 10 comments
Closed

Partial is rendered twice #146

euclio opened this issue Apr 12, 2017 · 10 comments
Assignees
@sunng87
Labels
bug

Comments

@euclio
Copy link

euclio commented Apr 12, 2017

Not sure if this is an issue with the library or my syntax. I'm updating a project that I wrote from the legacy partial syntax to the new syntax.
I have three (simplified) templates like this:

<!-- base.hbs -->
<html>
<body>
{{> layout}}
</body>
</html>
<!-- blog.hbs -->
{{#> base}}

{{#*inline "layout"}}
<div>
{{#> content}}
Parent content
{{> otherHelper}}
{{/content}}
</div>
{{/inline}}

{{/base}}
<!-- blog_post.hbs -->
{{#> blog}}

{{#*inline "content"}}
Child content
{{/inline}}

{{/blog}}

The problem is that the otherHelper helper gets rendered (and output) twice, and I cannot figure out why.
@sunng87
Copy link
Owner

sunng87 commented Apr 13, 2017

The syntax for

{{#> content}}
Parent content
{{> otherHelper}}
{{/content}}

is: if content partial not found, render the inner template. It seems my implementation rendered {{> otherHelper}} even when content is defined. This should be a bug. I will try to fix it this weekend.

@sunng87 sunng87 added the bug label Apr 13, 2017
@sunng87 sunng87 self-assigned this Apr 13, 2017
sunng87 added a commit that referenced this issue Apr 14, 2017
@sunng87
(fix) fix partial included on evaluation [#146]
81ee5ad 
Signed-off-by: Ning Sun <[email protected]>
sunng87 added a commit that referenced this issue Apr 14, 2017
@sunng87
(fix) fix partial included on evaluation [#146]
765fd2a 
Signed-off-by: Ning Sun <[email protected]>
@sunng87
Copy link
Owner

sunng87 commented Apr 14, 2017

@euclio could you please verify if current master branch works for you?

@euclio
Copy link
Author

euclio commented Apr 14, 2017 via email

@euclio
Copy link
Author

euclio commented Apr 18, 2017

Hm, having trouble testing this because I think that I'm getting a mismatch between handlebars-iron and handlebars-rust.

error[E0281]: type mismatch: the type `fn(&handlebars::Helper<'_>, &handlebars::Registry, &mut handlebars::RenderContext<'_>) -> std::result::Result<(), handlebars::RenderError> {helpers::join}` implements the trait `for<'r, 'r, 'r, 'r, 'r> std::ops::Fn<(&'r handlebars::Helper<'r>, &'r handlebars::Registry, &'r mut handlebars::RenderContext<'r>)>`, but the trait `for<'b, 'r, 'c, 'd, 'r> std::ops::Fn<(&'b handlebars::render::Helper<'r>, &'c handlebars::registry::Registry, &'d mut handlebars::render::RenderContext<'r>)>` is required (expected struct `handlebars::render::Helper`, found struct `handlebars::Helper`)
   --> src/routes.rs:169:51
    |
169 |     hbse.handlebars_mut().register_helper("join", Box::new(helpers::join));
    |                                                   ^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: required because of the requirements on the impl of `handlebars::helpers::HelperDef` for `fn(&handlebars::Helper<'_>, &handlebars::Registry, &mut handlebars::RenderContext<'_>) -> std::result::Result<(), handlebars::RenderError> {helpers::join}`
    = note: required for the cast to the object type `handlebars::helpers::HelperDef`

@sunng87
Copy link
Owner

sunng87 commented Apr 19, 2017

Ops, this happens when you have different version of handlebars library. Could you please also checkout handlebars-iron and change it's depended handlebars to the master branch?

@euclio
Copy link
Author

euclio commented Apr 19, 2017 •
edited
Loading

Ah. Just confirmed that it works on master, thanks! As a side note, why does handlebars-iron not expose handlebars as a pub extern crate if they're so tightly coupled?

@sunng87
Copy link
Owner

sunng87 commented Apr 19, 2017

It seems I should. I'm not quite familiar with re-export at the moment.

I'll close this for now.

@sunng87 sunng87 closed this as completed Apr 19, 2017
@sunng87
Copy link
Owner

sunng87 commented Apr 19, 2017

@euclio by the way do you have any links about re-export? I googled it only found a reddit thread says it's an undocumented technique. There was #67 about exporting JSON related crate. Maybe I can make decision for both.

@euclio
Copy link
Author

euclio commented Apr 19, 2017

I think there's an old issue where pub extern crate didn't work as expected, and you had to simulate the behavior by pub useing the item imported by extern crate. However, these days it seems to work as expected. Not aware of any links beyond the first edition of the rust book.

While I think that re-exporting handlebars is the right thing to do, because ideally handlebars-iron will be the only other dependency using it. I'm not sure that it makes as much sense for serde, since you'll need all your dependencies to use the same version of serde anyways, so it makes sense for a user to use their own serde dependency. See rust-lang/cargo#2064 for some more discussion on this.

I can submit a PR for the re-export if you'd like, but it's a small change.

@sunng87
Copy link
Owner

sunng87 commented Apr 19, 2017

Thank you @euclio , the patch is welcomed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sunng87 sunng87

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@sunng87 @euclio