Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handlebars interoperability #169

Closed
akre54 opened this issue Apr 3, 2013 · 8 comments
Closed

Handlebars interoperability #169

akre54 opened this issue Apr 3, 2013 · 8 comments
Milestone

Comments

@akre54
Copy link
Contributor

akre54 commented Apr 3, 2013

Handlebars template functions don't have a render method, which prevents the engine from working with typeahead templates. I'm pretty happy with Handlebars, and it'd be great if typeahead worked out of the box with it.

A possible fix could be to check for the presence of the render method for Hogan, and fallback to compiledTemplate for Handlebars/underscore/JST, etc in the compileTemplate fn.

// i.e. change
renderFn = utils.bind(compiledTemplate.render, compiledTemplate);

// to
renderFn = copiledTemplate.render ? utils.bind(compiledTemplate.render, compiledTemplate) : compiledTemplate;

(or something more elegant, but you get the idea)

@jharding
Copy link
Contributor

jharding commented Apr 3, 2013

Starting in v0.9.1, you can pass in compiled templates i.e.

var source = $("#entry-template").html(),
    template = Handlebars.compile(source);

$('.typeahead').typeahead({
  // ...
  template: template
});

@akre54
Copy link
Contributor Author

akre54 commented Apr 3, 2013

@jharding thanks for the fast reply. I ended up using the template function as a workaround, but it still feels hackish (particularly since I have to add the tt-suggestion class myself). It feels strange that it's possible to specify a custom templating engine but that the engine has to conform to Hogan's specific conventions (engine has to have a compile method, and its compiled template has to have render on the return value, which as far as I'm aware, most don't).

I'd much prefer to do this:

$('.typeahead').typeahead
  name: 'countries'
  template: "{{countryName}} (<strong>{{countryCode}}</strong>)"
  engine: Handlebars

to this

$('.typeahead').typeahead
  name: 'countries'
  template: (data)->
    Handlebars.compile("<div class='tt-suggestion'>{{countryName}} (<strong>{{countryCode}}</strong>)</div>", data)

Same deal for underscore, EJS, etc.

@jharding
Copy link
Contributor

jharding commented Apr 3, 2013

I ended up using the template function as a workaround, but it still feels hackish (particularly since I have to add the tt-suggestion class myself).

Thanks for mentioning this, I totally forgot about it. This was the reason why I choose not to support compiled templates in the first place, but it slipped my mind when I accepted a pull request for it a couple weeks ago. For now I'm going to update the README to not mention compiled templates and then I'll fix this in the next release. Tracking this in #170.

It feels strange that it's possible to specify a custom templating engine but that the engine has to conform to Hogan's specific conventions (engine has to have a compile method, and its compiled template has to have render on the return value, which as far as I'm aware, most don't).

I didn't want to have to deal with supporting different kinds of templating engines, so I decided the easiest approach would be to support one API and I just so happened to chose Hogan's API since that's what we use. Once the compiled template stuff is figured out, this shouldn't be an issue anymore.

@akre54
Copy link
Contributor Author

akre54 commented Apr 3, 2013

That seems like a totally reasonable solution. I like Hogan, but since I'm using Handlebars instead on this current project, it just makes things easier if everything works without fuss.

What if template could no longer be passed as a string, and the developer was left to implement the specific compile function (thereby eliminating the engine argument entirely)? The return value from this function would then be wrapped in the tt-suggestion div.

template = "{{countryName}} (<strong>{{countryCode}}</strong>)"
$('.typeahead').typeahead
  name: 'countries'
  template: Handlebars.compile(template)

$('.typeahead').typeahead
  name: 'countries'
  template: Hogan.compile(template).render

$('.typeahead').typeahead
  name: 'countries'
  template: (data) ->
    "#{data.countryName} <strong>(#{data.countryCode})</strong>"

@jharding
Copy link
Contributor

jharding commented Apr 3, 2013

I wouldn't necessarily be opposed to only supporting compiled templates, but I think some devs find it easier to pass a string template and an engine. Plus if we removed it, it'd break backwards compatibility and I'd prefer to only do that when absolutely necessary.

@akre54
Copy link
Contributor Author

akre54 commented Apr 3, 2013

I like the idea of passing an imported template and an engine too, but I can't think of any engines off the top of my head that support the .compile and .render methods in this way other than Hogan (which brings us back to the first discussion). I think I'd be happy if the output of all rendered templates, regardless of compiled origin, were wrapped before injecting in the DOM, thereby saving wrapping templates myself and ultimately preserving BC.

Let me whip up a pull really quick.

@jharding
Copy link
Contributor

jharding commented Apr 3, 2013

FYI I created #170 to track wrapping compiled templates. If you don't submit a pull request for it, I'll take care of it next time I work on typeahead.js.

@akre54
Copy link
Contributor Author

akre54 commented Apr 3, 2013

I made a first stab in 172. It takes away the flexibility of deciding not to include the tt-suggestion class in your markup (or putting it on a different tag like an li), but this seems like an easy tradeoff for a tiny edgecase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants