Does dust.js support variable substitution in helper param values?

[juliangoacher]

Given a context like this:

{
  "bar": "fubar",
  "helper": function(chunk, context, bodies, params) {
    return chunk.write(params.foo);
  }
}

and a template like this:

{#helper foo="{bar}"/}

I expected the output: "fubar"
But instead got: "function body_1(chk,ctx){return chk.reference(ctx.get("bar"),ctx,"h");}"

This happens because the dust.js compiler is detecting and compiling the template within the "foo" parameter, but dust isn't then resolving the template before passing the parameter value to the helper function.

It`s possible to resolve the parameter value within the helper function by using chunk.capture:

{
  "bar": "fubar",
  "helper": function(chunk, context, bodies, params) {
    return chunk.capture(params.foo, context, function(value, chunk) {
      chunk.write(value).end();
    });
  }
}

But this approach then runs into problems if more than one parameter value needs to be captured:

{#helper foo="{bar}" foo2="{bar}"/}

  "helper": function(chunk, context, bodies, params) {
    return chunk.capture(params.foo, context, function(value, chunk) {
      chunk.capture(params.foo2, context, function(value2, chunk) {
        chunk.write(value).write(value2).end();
      });
    });

When the above code is used, the two parameter values are output correctly but the template doesn't evaluate beyond the helper function i.e. as if chunk.end() was never called.

The fact that the compiler detects and compiles the template within the parameter value suggests that this functionality is supposed to be supported but isn't fully implemented. Does anyone know if this is the case? Note that given the situation described above, it would be better if the compiler didn't compile param values in this way, as if would then at least give the author of a helper function the option of manually evaluating param values if they were expected to support var substitution.

[vybs]

we use the tap method to resolve values.

Inside a helpers, it is upto to the one who writes the heper to resolve the values using tap/capture/map
sounds reasonable to me

take a look at the dustjshelpers repo
https://github.com/linkedin/dustjs-helpers/blob/master/lib/dust-helpers.js

[juliangoacher]

@vybs Thanks Veena, but isn't tap synchronous?

I agree that it's reasonable to expect the writer of a helper function to resolve the values using tap/capture etc. but the point I was making above is that chunk.capture doesn't seem to work for multiple params, and so if values need to be resolved asynchronously then the developer can't do anything - which has to be a bug. (btw, asynchronous value resolution should be the default assumption - the dust helpers like 'if' won't work for us because they assume all values can be resolved synchronously)

[vybs]

@juliangoacher yes tap is synchronous.

I am wondering how the dynamic partials work, it also uses chunk.capture

[juliangoacher]

@vybs partials with params resolving to async values don't seem to work. I haven't delved into the code to work out exactly what's happening, and this is probably a separated bug related to my point above that it should be possible to resolve all values asynchronously, but here's what happens anyway:

Using the 'partial /params tests // partial with inline param' test setup at http://linkedin.github.io/dustjs/test/test.html and a context like this:

{
  "n1": "Mick",
  "n2": function(chunk, context, bodies, params) {
    return chunk.map(function(chunk) {
      chunk.write("Mick");
    });
  },
  "c": 30
}

then this template works ok:

{>partial name="{n1}" count="{c}"/}

n1 can be resolved synchronously and the template eval's to "Hello Mick! You have 30 new messages."

But dust hangs with this template:

{>partial name="{n2}" count="{c}"/}

[juliangoacher]

@vybs Scratch that, I'd forgot to call end() on the n2 handler. It does actually work, I'll look deeper into the code later.

[juliangoacher]

chunk.partial just pushes its params onto the stack before evaluating the partial template, so ultimately the param values are resolved using the standard var substitution mechanism, which is a different case to helper params described originally above.

[juliangoacher]

The problem with my first attempt to read multiple params was that end() wasn't called after chunk.capture(); code that actually works looks like this:

{
  "bar": "fubar",
  "helper": function(chunk, context, bodies, params) {
    return chunk.capture(params.foo, context, function(value, chunk) {
      chunk.capture(params.foo2, context, function(value2, chunk) {
        chunk.write(value).write(value2).end();
      }).end();
    });
  }
}

A general solution would look like this:

// Capture all parameter values, pass result values to the callback.
function captureParams(chunk, context, params, cb) {
    var values = {};
    function capture( id, next ) {
      return function( chunk ) {
        if( typeof params[id] == 'function' ) {
          return chunk.capture( params[id], context, function( value, chunk ) {
            values[id] = value;
            next( chunk ).end();
          });
        }
        else {
          return chunk.map(function( chunk ) {
            values[id] = params[id];
            next( chunk ).end();
          });
        }
      };
    }
    var fn = function( chunk ) {
      process.nextTick(function() {
          cb( values );
      });
      return chunk;
    }
    for( var id in params ) {
      fn = capture( id, fn );
    }
    fn( chunk );
}

Maybe something like this should be included in the dust module as a utility function?

My original point when raising this issue was that the compiler detects and compiles templated parameter values, so I think there is still an argument that all the above should be done automatically by the engine, instead of by the helper function developer. A tangentially related issue has also been raised by issue #316 - perhaps resolving partial parameters as above would be a better approach?

[rragan]

One option would be to include this with the tap helper in dust helpers as it is an extension of what tap does.

[juliangoacher]

@rragan I'm not sure about that. The tap helper is a broken implementation because it doesn't support asynchronous value resolution. Probably doesn't cause problems for a lot of use cases but would cause problems for us. What I'm suggesting would be a complete replacement of the tap helper.

I could implement a function wrapper which would automatically convert all helper params before the helper function was invoked, e.g. looking a bit like this:

"myhelper": cap(function( chunk, ctx, bodies, params ) {
    /* ... all param values are now resolved ... */
})

I'd question whether it belongs in the dust-helpers project. We don't use that project but we'd use the function wrapper if it existed... but I guess I'll be implementing it anyway, someone let me know if linkedin would be interested and I'll create a pr.

[juliangoacher]

I've posted a gist with a working helper wrapper function: https://gist.github.com/innerfunction/6252243

[rragan]

In the scheme of things, are you also suggesting that all existing dustjs-helpers be converted to this so they always work in an asynch environment?

[juliangoacher]

I'm coding in a node.js environment where pretty much everything is asynchronous, and I think that in this case it would make sense for the conversion to be automatically be applied in all cases. But I'm aware that a lot of dust.js users seem to be working in other environments where the asynchronous requirements aren't so important, and the problem in those cases is that the conversion subtly changes the semantics of a helper function, because all helpers become asynchronous, i.e. are wrapped in a chunk.map() call, and this means that all helpers now need to call chunk.end() when they've finished writing their result, when previously this may not have been necessary. So for this reason I don't think I could suggest that the conversion becomes the default; probably the optimal solution is simply to provide the wrapper function to those developers that want to use it, provided they understand the change in semantics.

[carchrae]

👍 on this @juliangoacher - thank you for posting your async helper code. are you still using this?

[juliangoacher]

@carchrae You're welcome - yep, I'm still using it.

[sethkinast]

This could make sense as the extension to #555 that we were talking about.