acetate icon indicating copy to clipboard operation
acetate copied to clipboard

Method signatures for template helpers

Open patrickarlt opened this issue 10 years ago • 11 comments

I've put quite a bit of work into making Acetates existing helper API as simple as possible. Mostly by removing things from what you can do with Nunjucks. Right now you can do this.

// create a link helper that will also add a .active 
// class to the linked page is also teh current page
acetate.helper('link', function(context, url, text){
  // context is the current local variables on the page
  var className = context.url === url ? 'active' : 'inactive';

  // build a relative url to the page we are
  // linking to using the relativeUrl variable
  var relativeUrl = context.relativePath + url;

  // Nunjucks template for the link
  var template = '<a href="{{relativeUrl}}" class="{{className}}">{{text}}</a>';

  // finally render the nunjucks string with the variables
  return acetate.nunjucks.renderString(template, {
    relativeUrl: relativeUrl,
    className: className,
    text: text
  });
});

and use it in your templates like {% link "/", "Home" %}.

You can also do the same thing for blocks. This API is simple but also leaves out some important parts that a few people like have already run into these are.

  • Named arguments. Nunjucks lets you do named args so things like the following things {% link "/", "Home" class="main-link" %} keyword args are passed as an object in the custom tag API
  • Async helpers Nunjucks can wait for async operations like reading external files in custom tags.

The main reason that both of these things do not exist is that they make the method signature for helper functions sort of terrible. Consider our humble {% link %} helper above with the async and keyword args added it would look like this:

acetate.helperAsync('link', function(context, url, text, keywordArgs, callback) {
  // return a string
});

This means that every helper function always gets between 2 and 3 arguments (keywords and page context and maybe the callback fro async helpers) plus anything the user might want to pass in. This has a really bad code smell but I would like to support both those features. Any ideas on what this might look like?

The only thing I can think of is to make the method signature be flexible with a few rules.

  1. Callback (for asyncHelper and asyncBlock is always the last parameter)
  2. options (Nunjucks calls them keyword args) are aways the last parameter (helper and block) or the second to last (asyncHelper and asyncBlock) IF named arguments are detected.
  3. context is always first.
  4. Everything else get stuck in the middle.

This would kind of be a mess but allows for more concise method sigs.


Another option would be to do something like:

acetate.helperAsync('link', function(context, args, options, callback) {
  var url = args[0];
  var text = args[1];

  // return a string
});

This would simple pass all unnamed arguments as an array (args) array and all named arguments as options. I'm not sure if I would want anything to optional here so you could use just args or options but I'm open to it either way.


The third option would be to not support Nunjucks flavor of keyword/named args and just add asyncHelper and asyncBlock with the callback as the last param. If you make some params optional so be it.

acetate.helperAsync('link', function(context, url, text, callback) { // if you make text optional be prepared because the callback will always be passed as a the last param. callback('string'); });

If people want to do keyword args on their own they can pass an object literal and handle it themselves.

{% link '/', 'Home', {class:'main-nav'} %}

I'm mostly in favor of taking the third option and adding asyncHelper and asyncBlock because it does not involve any breaking changes and keeps the APIs separate and simple.

patrickarlt avatar May 11 '15 19:05 patrickarlt

Looking for comment from @paulcpederson @ngoldman and @nikolaswise @bsvensson and @lheberlie.

patrickarlt avatar May 11 '15 19:05 patrickarlt

@patrickarlt is this just monkey patching nunjucks? why not let nunjucks handle helpers and add the ability to swap nunjucks out for another templating engine? There are already so many. It seems unnecessary and a little odd to me to build novel template engine method signatures into a static site generator.

ungoldman avatar May 11 '15 21:05 ungoldman

is this just monkey patching nunjucks?

It's just wrapping Nunjucks custom tags for a few specifici use cases. Right now it is really, really hard to create custom helpers/tags in Nunjucks so I want to make it easier for people.

why not let nunjucks handle helpers

Because its really complicated https://github.com/patrickarlt/acetate/blob/master/lib/mixins/nunjucks.js#L44-L84

add the ability to swap nunjucks out for another templating engine?

I've thought about this quite a bit and there are are a few reasons I have stuck with Nunjucks as the sole templating engine:

  • built-in support for layout and partials
  • supports custom custom behavior for loading templates
  • ability to customize markup with helpers
  • easy integration and error handling

Right now there are only a few other templating languages that also have all of these features.

  • Dust.js - kinda gross but not horrible
  • Jade - totally gross to force this on people

Supporting a more common templating language like Handlebars would also require using handlebars layout and registering each page as a partial with Handlebars which complicates integration. Things like underscore templates or lighter weight options are even worse since they require building in support for partials and layouts on my own.

patrickarlt avatar May 12 '15 17:05 patrickarlt

Hey @patrickarlt,

I haven't done much other than stick to the standard helper implementation:

acetate.helper("sample_code_highlight_helper", function (context, /*String*/ syntax_language)

{% sample_code_header_helper "js" %}

I'm guessing I just haven't run into any advanced cases where the standard isn't working for me.

lheberlie avatar May 15 '15 18:05 lheberlie

After using this for a bit, here's my two cents:

  • It seems like the acetate.helper works pretty well as implemented.
  • An async helper seems pretty unnecessary (what's the use case for that?)
  • Named args don't feel like much of a value add

My advice is to leave it as is until somebody clamors for async or named args as a feature ¯_(ツ)_/¯

paulcpederson avatar Oct 29 '15 05:10 paulcpederson

@paulcpederson I can see use cases for async helpers where you might want to do something like:

  • Read a raw file into your template
  • Make a dynamic request for something and render it in
  • Call other random node stuff that HAS to be async

Right now to do these things you have to write extensions which are great for large heavy duty solutions things. Dynamic data files work well but cant be page specific.

I might try async helper for a later release.

patrickarlt avatar Nov 06 '15 04:11 patrickarlt

@paulcpederson I guess my question is this.

If you want page specific async functions right now you have to write extensions which seem a little heavy handed. I think an easier approach might be asyncHelper and asyncBlock which are easier for smaller use cases. Extensions still have a place but work at a larger scale.

If this makes sense I think the following method signature makes sense:

acetate.asyncHelper('name', function (context, arg1, arg2, callback) {
  // return your text to callback or an error
});

If you want to make arg1 and arg2 optional then you have to manage your arguments yourself.

patrickarlt avatar Nov 06 '15 04:11 patrickarlt

@patrickarlt ok. that seems reasonable. Like I said I haven't needed that yet, but those are valid use cases and it seems like this would be an easy add.

Another option, though breaking, would be to just make the helper and block methods async by default and pass a callback to them?

paulcpederson avatar Nov 06 '15 17:11 paulcpederson

but that makes them a bit more complicated, so probably a new method is the best bet.

paulcpederson avatar Nov 06 '15 17:11 paulcpederson

@patrickarlt also wouldn't acetate.helperAsync make more sense than acetate.asyncHelper. The former is more common in node I think. (ie. fs.readFileSync & fs.readFile)

They're pretty much the same signature but one has a callback.

paulcpederson avatar Nov 06 '15 17:11 paulcpederson

Love it!

ungoldman avatar Oct 14 '16 20:10 ungoldman