merge helpers in compiled template #473

Closed
wants to merge 1 commit into from

2 participants

@lepture

The compiled template handle helpers:

helpers = helpers || Handlebars.helpers;

This patch will merge the helpers:

helpers = helpers || {};
for (var key in Handlebars.helpers) {
  helpers[key] = helpers[key] || Handlebars.helpers[key];
}
@lepture lepture pushed a commit to spmjs/grunt-cmd-transport that referenced this pull request Mar 25, 2013
Hsiaoming Yang patch Handlebars.precompile. Shoule be removed after wycats/handlebar…
…s.js#473 merged.
69661a3
@kpdecker
Collaborator

What is the use case here? Also I worry that this will cause mutation of the passed in handler which is not a good thing.

@lepture

@kpdecker This is a fallback strategy.

My template is:

{{# each items }}
  {{ _ this }}
{{/ each }}

After the precompile, I will get a function:

fn({items: ['foo', 'bar']}, {
  helpers: {
    _: function(key) {
      return 'i18n'
    }
  }
});

I don't want to pass each helper in the function, it should use the built-in each helper.

@lepture

@kpdecker

Also I worry that this will cause mutation of the passed in handler

It will not.

 helpers[key] = helpers[key] || Handlebars.helpers[key];

This is a fallback, like underscore's _.defaults.

@kpdecker
Collaborator

So you want to be able to pass custom helpers for a particular call but have it pick up the global helpers in that call as well?

I don't think it's a good idea to do this in the generated code as it adds a lot of overhead in the precompiled output. I would do this in the Handlebars.VM.template method.

On the mutation concern, this will cause changes, consider:

Handlebars.helpers = {
  bar: function() {}
};

var helpers = {foo: function() {}};
fn({} {helpers: helpers});

Unless I'm horribly mistaken helpers will have a field bar assigned to it after the template executes. It won't overwrite anything but it will modify the object which could cause unintended consequences for code that is expecting the field to be undefined.

@lepture

@kpdecker

So you want to be able to pass custom helpers for a particular call but have it pick up the global helpers in that call as well?

Exactly!

I can merge helpers before passing them to the template function, but it should be better if the template function can merge the helpers itself.

I don't worry about this feature. Thank god, Handlebars is a singleton, I can patch it in my own build tools:

spmjs/grunt-cmd-transport@69661a3

I hope you can have a review, it doesn't hurt anyone.

@kpdecker
Collaborator

I'm not comfortable with this fix for the performance and side-effects reasons listed above. I've noted the request for this functionality in #163 which is requesting similar behavior. I hope to have a pull request for this shortly going the route of modifying the Handlebars.VM.template method.

@kpdecker kpdecker closed this Apr 7, 2013
@lepture lepture deleted the unknown repository branch Apr 8, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment