search

assemble / assemble-handlebars

Assemble engine plugin for Handlebars templates
https://github.com/assemble/assemble
MIT License
8 stars 19 forks source link

remove partial pre-compiling (buggy) #11

Closed AndersDJohnson closed 10 years ago

AndersDJohnson commented 10 years ago

Unless you have a working use case for passing a string template... it isn't working for me.

doowb commented 10 years ago

We pass in a string in assemble here.

What's not working? In any case, I think handlebars is compiling it in the register function so this code probably isn't needed.

jonschlinkert commented 10 years ago

I think there might be another bug in assemble that is conflicting here. I'm working on tracking it down now.

doowb commented 10 years ago

Actually, they don't compile it first. It might be a good idea to remove it anyway and I was thinking about this when doing the layout helpers. I thought there might be a conflict with those and assemble, but hadn't had the time to test it yet. I think this can still be a good PR since it leaves the partials as strings throughout the entire process.

AndersDJohnson commented 10 years ago

@doowb Getting this error from Handlebars core: You must pass a string or Handlebars AST to Handlebars.compile.

If they don't pre-compile, this would be a performance hit. I can look into it further if we're hesitant to merge this.

jonschlinkert commented 10 years ago

I'm good with merging this, but let's make sure we test it against real code first and make sure there are no breaking changes. also, I think a bug was created between 0.4.9 and 0.4.10 that might be related, perhaps not, but it's worth a look.

AndersDJohnson commented 10 years ago

As @doowb (https://github.com/assemble/assemble/pull/354#issuecomment-26230385) and I have discovered, this appears to be necessary to support loading layouts as partials in the options.partials config.

jonschlinkert commented 10 years ago

@adjohnson916 no prob, I'm following

jonschlinkert commented 10 years ago

btw, @adjohnson916 "loading layouts as partials" specifically refers to this pr https://github.com/assemble/handlebars-helpers/pull/75, correct? If so I think we just need to get the features mentioned in https://github.com/assemble/handlebars-helpers/issues/16 implemented then we're good to merge it, so we're not messing with it too much after people are using it.

AndersDJohnson commented 10 years ago

@jonschlinkert Correct.

doowb commented 10 years ago

@adjohnson916 Will you remove the var tmpl; on the first line of this function and commit it? Then I'll merge this in and bump this repo.

AndersDJohnson commented 10 years ago

@doowb Done.