formatjs / handlebars-intl

Handlebars helpers for internationalization.
http://formatjs.io/handlebars/
Other
265 stars 28 forks source link

Add function support to helpers #67

Open jimf opened 9 years ago

jimf commented 9 years ago

Update formatDate, formatTime, formatRelative, formatNumber, and formatMessage to allow for their primary argument to be specified as a function that returns the required argument type.

yahoocla commented 9 years ago

Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! :smile:

jimf commented 9 years ago

CLA signed. Should I close this PR and open another?

caridy commented 9 years ago

I imagine that the function is just a another helper, why not just use a handlebars' subexpression?

{{formatNumber (numFn)}}

in which case you have full control over the values to be passed to that helper, etc.

jimf commented 9 years ago

It's not, at least not formally. It's for supporting model presenters, identical to what's laid out in pragmatic-backbone.

This pattern is supported by all of the built-in Handlebars helpers. Example: https://github.com/wycats/handlebars.js/blob/93faffa549166c492267cc96d3e6848923760d90/lib/handlebars/base.js#L175

caridy commented 9 years ago

I will like @ericf to chime in. I really don't like that pattern, just like I don't like these helpers to support a path to resolve a message, etc.

As a side note: those check for function type are not enough, you need something better to truly detect functions.

caridy commented 9 years ago

It's not, at least not formally. It's for supporting model presenters, identical to what's laid out in pragmatic-backbone.

I don't fully understand what do you mean here. Can you explain why subexpressions will not work just fine?

jimf commented 9 years ago

As a side note: those check for function type are not enough, you need something better to truly detect functions.

No problem. Handlebars exposes a Handlebars.Utils.isFunction util. I'd be happy to switch over to that if preferred.

I don't fully understand what do you mean here. Can you explain why subexpressions will not work just fine?

Sub-expressions could work, assuming I were to write a Handlebars helper to effectively call the function and return the result. The workflow I'm describing however involves sending JavaScript class instances (aka Model Presenters) as the context to the Handlebars template (for sufficiently complex views; the idea being to keep the complexity out of Handlebars and in more easily tested code) as opposed to plain objects with static properties. These classes generally expose view-centric logic on top of their corresponding models in the form of functions. In the case of handlebars-intl, this simply requires statically assigning values to this in the presenter constructor, rather than exposing a function on the prototype like we do nearly everywhere else where this pattern is used (hopefully this made sense. If it'd help, I can put a gist/fiddle together). This is totally acceptable. It'd just be cool if these helpers worked like the base ones do so there's no special treatment.

jimf commented 9 years ago

Update: I looked into subexpressions more, since truthfully I didn't know much about them. The Handlebars expression docs lead me to believe that sub-expressions evaluated as Handlebars helpers, which I'm not working with. However, to my surprise while testing out your suggestion, it turned out to actually work. I'm digging around in the Handlebars source to figure out how that all works, and I don't have my head wrapped around it fully yet, but this testcase certainly confirms that it's expected functionality. So take that for what you will. I do still like the idea of the handlebars-intl helpers functioning the same way as the built-ins, but I understand if you'd rather keep them pure in terms of expected input.

Just playing devil's advocate with myself, I can't actually find anywhere in the Handlebars docs that explicitly states that Handlebars will evaluate and return the return values of functions that are passed with the context. But that said, it's clearly supported, as well as relied upon, for even that testcase above to work.

paulfalgout commented 7 years ago

A significant difference between the handlebars sub-expressions and how it would simply call the function is scope. handlebars data functions can call other data functions, but using sub-expression loses the scope can lose scope. However this PR only partially solves this issue and doesn't address passing data to formatMessage

Edit: I think the pain point here at least for my use case is switching from some template string containing {{ aFunctionData }} to {{formatMessage (intlGet "some.template.string") aFunctionData=aFunctionData}} isn't going to work.. particularly if the function uses its scope. But turns out this'd be true for any helper in handlebars and sub-expression is likely the best solution for these situations.