Don't auto-call helpers

bminer / meteor-blade

Meteor Smart Package for Blade - HTML Template Compiler, inspired by Jade & Haml

10 stars 4 forks source link

Don't auto-call helpers #4

Open lalomartins opened 11 years ago

lalomartins commented 11 years ago

The hack with defineProperty to auto-call helpers if they're functions makes me sad. But there's a few better reasons than aesthetics why I don't like it:

It makes sense in Handlebars because its design philosophy is minimal templates with no logic; but code lines and interpolation are Blade is pure JavaScript, for better or worse, and therefore it makes more sense to make calls explicit.
And speaking of explicit, “explicit is better than implicit” ;-) if the helper is a function I'd rather see the “()” in my template so I know I'm calling something.
More importantly, Blade doesn't leave me any way to pass arguments to a helper… and some helpers are only useful with arguments!

lalomartins commented 11 years ago

Problem: () isn't allowed in attributes (eg img(src=myhelper())). Workaround: img(src="#{myhelper()}").

bminer commented 11 years ago

Hmmm.... I have to agree with you -- helpers should not be auto-called by the defineProperty hack. This is an easy change, but it's a breaking change. Also, some documentation might need to be written to clarify the differences between Handlebars and Blade (see https://github.com/bminer/node-blade/wiki/Using-Blade-with-Meteor).

I also agree that Blade doesn't allow you to pass arguments to a helper. This sucks.

To address your latest comment about no parentheses allowed in attributes... this is a limitation of the Blade parser that was added to greatly simplify parsing of attributes. The workaround you mention is the best way to handle this.

In your opinion, what's the best way to move forward with this? Change package.js to not auto-call helpers, then update the Blade + Meteor documentation?

Thanks for bringing this issue to my attention!

lalomartins commented 11 years ago

(it's meteor-blade that doesn't let you pass arguments, blade itself has no problem with that ;-))

Yes, that would be the solution I'd favour. I have already removed the code in question from my local copy… and it works fine.

One possible solution for the attribute thing only is to change the attrs function in the runtime to do something similar to what Handlebars does (_.result()). I don't know if it's necessary though, the interpolation workaround works rather well for more complex usage.