search

SlexAxton / require-handlebars-plugin

A plugin for handlebars in require.js (both in dev and build)
804 stars 202 forks source link

Handlebars3.0.3 upgrade - picking up where chrisrink left off #237

Closed ryechus closed 8 years ago

ryechus commented 8 years ago

I started where @chrisrink left off and was able to fix the problems he was having pretty simply.

SlexAxton commented 8 years ago

Looks pretty good. Nice tests too!

ryechus commented 8 years ago

I removed the extra space. Let me know if you find anything else.

SlexAxton commented 8 years ago

Nope. Looks solid.

chrisrink commented 8 years ago

Thanks @ryechus for finishing this off. A few small notes. I actually got this working and have been testing it in my production environment for the last month with the follow:

 function isHelper(statement){
        return !!((statement.path && statement.path.original) && (statement.type === 'SubExpression' || (statement.params && statement.params.length) || statement.hash));
      }

So @ryechus solution is pretty much the same thing with the exception of the additional statement.path.original check. I don't remember the use case exactly of when that was used. Nested helper in a partial maybe? Anyways, if that pops up I believe that would be the solution.

In the long term, I made a patch request for Handlebars to better figure this out in their public function

Handlebar.AST.helpers.helperExpression

This looks like it will be fixed in Handlebars 4.0 but it might be a good idea to use the one tested by the Handlebars guys.

{code} helperExpression: function(node) { return (node.type === 'SubExpression') || ((node.type === 'MustacheStatement' || node.type === 'BlockStatement') && !!((node.params && node.params.length) || node.hash)); {code} https://github.com/wycats/handlebars.js/pull/1055

Thanks again