Making AMD module output return compiled function when only one template is processed#377
Making AMD module output return compiled function when only one template is processed#377JamesMaroney wants to merge 2 commits intohandlebars-lang:masterfrom
Conversation
Making amd precompilation return the compiled function from the created module
modifying added return statement to only apply when a single template is being compiled
|
This seems like something that should be an explicit opt in rather than an implicit behavior based on the number of templates passed in. This could impact users who are making multiple calls to the binary but not expecting a return. |
|
Closing this as I don't think this is the right direction. Will gladly accept a pull request that does this via a flagged parameter! |
|
I can certainly look into putting in an option to trigger this functionality. It really seems so strange to have this stance though. If users are using this as an AMD module, then they should be expecting a return value from the module. That's just how AMD works. Do you have a preferred option name, or is "returnTemplateFromAmdModule" correct? Of course, then I'll need to add more code to verify that this is only used in just the right scenario, and documentation on just when to use it. As I type this, I really think the correct approach here is to remove the conditionality on compiling only one template, and decide what gets returned from the AMD module when compiling multiple templates. Giving the users options they don't care about usually makes your product seem overly complicated, rather than flexible. I would love if we could solve this, as I am currently working off of a fork of handlebars.js and I'd love to come back to the mainline. |
|
My issue here is that you are assuming that someone who is only using one template is using AMD which to me is an odd assumption to make. I.e. this will break if someone is not using amd or is otherwise concatenating the output with other code. So we currently have the -a option for generating amd output. This currently does not return anything and as you mentioned the question is what should we return. What if we always return the i.e. Would generate something like I'm not an AMD user so I don't know how well that works in practice. We are hoping to cut a release soon so we might be able to get something out on npm quite quickly if we can get this figured out in time. |
|
ah, yes, I see. It wasn't my intent to affect any behavior in the non-AMD path. This was an oversight on my part for sure. Now I much better understand your hesitation :) I'll need to take a few minutes to understand your suggestion. In the meantime, let me tell you a little bit about how we've come to use handlebars with AMD, and it might help you think of the proper way to tackle the problem. Initially, I had written a little shim to make handlebars.js act as a requirejs plugin so that we could do this: The commit that this pull request is covering is to remove the need for the While we could handle the return value being the full templates object, it would add verbosity to our only usage. It would, however, be a single, standard solution. Selfishly, I'd love it if it would return just the function if it compiled only one, or the templates object if it compiled multiple. Would that work for you? |
|
There isn't any existing behavior here to worry about breaking WRT return values. The question is if it's odd to have different return value for one case vs. the other. I think it's fine if the single case returns the template directly. |
|
ok, cool. By when should I get the pull request updated in order to get this in the next release? |
|
The plan right now is to try to roll the release tomorrow or early next On Sat, Apr 6, 2013 at 6:07 PM, James Maroney [email protected]:
|
|
ok. I think I can get some time tonight to get this in. Thanks for all the feedback. My team will be happy to come back to the mainline handlebars repo! |
Using Handlebars in an AMD environment, it would be helpful to return the compiled function from the AMD module.
To minimize impact on your current logic, I have only implemented this modification for when one template is being precompiled.