feat: allow a custom escapeExpression for an env

[thekiwi]

We use handlebars.js for both XML templating and JSON templating within the same application. As such, we'd like to be able to override the escapeExpressions function on a single Handlebars environment, rather than globally. This is a small PR that allows us to do that, by first attempting to get the environment's version of escapeExpression and falling back to the Utils version if that is falsey.

Usage (or how we might end up using it):

const hb = Handlebars.create();
hb.escapeExpression = str => JSON.stringify(str).slice(1, -1);
// ... continue using 'hb' as normal

Please let me know if anything is unclear or if you'd like me to make additional changes.

• Please don't start pull requests for security issues. Instead, file a report at https://www.npmjs.com/advisories/report?package=handlebars
• Maintain the existing code style
• Are focused on a single change (i.e. avoid large refactoring or style adjustments in untouched code if not the primary goal of the pull request)
• Have good commit messages
• Have tests
• Have the typings (lib/handlebars.d.ts) updated on every API change. If you need help, updating those, please mention that in the PR description.
• Don't significantly decrease the current code coverage (see coverage/lcov-report/index.html)
• Currently, the 4.x-branch contains the latest version. Please target that branch in the PR.

[thekiwi]

Hey @nknapp, just wondering whether you had a chance to look at this? Is it a small enough addition that you would be happy to merge and release? Thanks!

[nknapp]

Sorry, I haven't had a thorough look yet. But I think something similar is part of our plans avoid security vulnerabilities in the future. I would like to defer until then to avoid unnecessary api changes.

[nknapp]

I would like to merge this, but the API is not what I would want to have. I have to do a general API discusssion with @wycats for overriding this like that (helperMissing, blockHelperMissing, escapeExpression). I don't know when I will be able to do that and it might fall in the "we need a spec" category.

I would think of an interface similar to

Handlebars.registerHook('escapeExpression', (string) => ...);
// or 
Handlebars.hooks.escapeExpression((string) => ...)

Technically, this is not a language feature, so I'm not sure ich 'we need a spec' applies here, but in the end, that @wycats decision.

[thekiwi]

@nknapp I'd be very happy with your proposed change. I made my PR change as small as possible to minimise the impact, but totally understand if you'd like to implement this as a proper supported and documented API.

[nknapp]

In #1559, I added the hooks property to the runtime. I'm still not confident about a good name for the function(s) to register a hook

• Handlebars.on('escapeExpression', (expression) => ...)) would match the common code from Node.js when it comes to events, but this is not an event.
• Handlebars.registerHook('escapeExpression', (expression) => ...)) would match the current helper-registration in a way. But it bugs me that there is a string defining the hook, because there is no arbitrary set of hooks to be replaced.
• Handlebars.hooks.escapeExpression((expression) => ...)) is not possible at the moment, because the hooks object already contains hooks, but actually, the hooks should be private, so maybe we should call them _hooks anyway. Still, the method name does not suggest that the escapeExpression-function is being replaced.
• Handlebars.replace.escapeExpression((expression) => ...)) would be more descriptive. But it feels weird having a replace property. And it would sound wrong for helperMissing.
• Handlebars.hookInto.escapeExpression() ...

I don't know. Does anyone have a good idea, or knows how other frameworks solve this?

In any case, it might be helpful to pass the old method into the hook function as well, so that the wrapper can be decide to call it. Or maybe not in the first stage.

[nknapp]

@thekiwi would it be OK for you to have a runtime option to override "escapeExpression"?

[thekiwi]

@nknapp It should be, are you able to provide a snippet demonstrating how I would use it?

[nknapp]

It would be something like

const template = Handlebars.compile('{{name}}');
const output = template({ name: 'Jonny "the hound" Walker' }, {
  hooks: {
     escapeExpression: string => string.replace(/(["'\\])/g,'\\$1')
  }
}

[KrishnaPG]

This is better than having to override the global utils.escapeExpression.

Till some other mechanism (such as hooks() etc.) is made available, this should be strongly considered for merging.

[silverwind]

Ideally I too would like to see a interface that does not require the use of Handlebars.create, but I tend to agree this is mergeable as-is, just to unblock a number of people who would like to use Handlebars for non-HTML templating with proper escaping.