Skip to content
This repository has been archived by the owner on Oct 19, 2023. It is now read-only.

hbsfy with --traverse fails with inline partials #54

Closed
kentmw opened this issue Apr 12, 2016 · 12 comments
Closed

hbsfy with --traverse fails with inline partials #54

kentmw opened this issue Apr 12, 2016 · 12 comments

Comments

@kentmw
Copy link
Contributor

kentmw commented Apr 12, 2016

With Handlebars 4.0.0, they introduced inline partials: http://handlebarsjs.com/partials.html#inline-partials

Our project added the --traverse flag so that we could bundle our partials in our browserify bundles (very helpful!).

These don't seem to mix as the inline partials are defined inline inside the same template.

Take this example from the handlebars documentation:

{{#*inline "myPartial"}}
  My Content
{{/inline}}
{{#each children}}
  {{> myPartial}}
{{/each}}

In this example myPartial will assumed to be node_module dependency. My thoughts is it would be nice to be able to give an indication to the hbsfy parser that this is an inline partial and not modify it.

Something like:

{{#*inline "myPartial"}}
  My Content
{{/inline}}
{{#each children}}
  {{> inline:myPartial}}
{{/each}}

I believe this would just require:

if (p.indexOf('inline:') !== 0) {
   ...
}

on line https://github.com/epeli/node-hbsfy/blob/master/index.js#L139

I'll make a pull request if we think this the way to solve this.

@mandragorn
Copy link

👍

kentmw added a commit to kentmw/node-hbsfy that referenced this issue Apr 12, 2016
@kentmw
Copy link
Contributor Author

kentmw commented Apr 12, 2016

Jk, made the pull request anyway. If anyone wants to check it out, I'd appreciate it.

@kentmw
Copy link
Contributor Author

kentmw commented Apr 12, 2016

I'm working out the fact that you still have to revert the name of the partial back to its non-prefixed state after skipping the commonjs aspect.

@kentmw
Copy link
Contributor Author

kentmw commented Apr 12, 2016

Well, without attempting to do just-in-time updates to the file or js that comes from your templates on this line: https://github.com/epeli/node-hbsfy/blob/master/index.js#L132, it seems like the way to do it is just have the same prefix for the inline partial name.

{{#*inline "inline:myPartial"}}
  My Content
{{/inline}}
{{#each children}}
  {{> inline:myPartial}}
{{/each}}

@kirbysayshi
Copy link
Collaborator

@kentmw thank you so much for the initiative with the PR! Also for reporting this, I had no idea handlebars added this.

While your inline: suggestion would definitely be future proof, I'm concerned about introducing new syntax to handlebars that only exists in this plugin.

What do you think about instead searching the template for *inline statements when a PartialStatement is found, and if an *inline is indeed found, abort the hbsfy traverse logic? That would make the inline logic more of an override, while not introducing new syntax and preserving what users would like expect.

@kentmw
Copy link
Contributor Author

kentmw commented Apr 13, 2016

@kirbysayshi updated my PR to be transparent with no new syntax. Also added tests.

@kentmw
Copy link
Contributor Author

kentmw commented Apr 14, 2016

@kirbysayshi I think it's ready for your review.

@kentmw
Copy link
Contributor Author

kentmw commented Apr 15, 2016

@kirbysayshi sorry to be persistent, but I was hoping you could take a look at the PR this week. Our project is blocked until this is resolved. Hopefully it just needs a merge and re-release

@kirbysayshi
Copy link
Collaborator

If you're using npm, you can always refer to your github fork in your package.json so to not be blocked by PRs to public packages

@kentmw
Copy link
Contributor Author

kentmw commented Apr 18, 2016

@kirbysayshi thanks for the tip. Pretty sure the company I'm working with wouldn't be too happy moving their project to depend on my github fork, but I'll give it a go. I appreciate the thought.

kirbysayshi added a commit that referenced this issue Apr 18, 2016
#54 - adding capability to use inline partials with --traverse flag
@kirbysayshi
Copy link
Collaborator

Should be fixed in v2.7.0, published.

@kentmw
Copy link
Contributor Author

kentmw commented Apr 18, 2016

Thank you!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants