each overwrites data in the "data" property.

[janthonyeconomist]

I am using handlebars as the templating engine of a gulp-metalsmith build system. I am using the metalsmith-data-loader to load a JSON file into the a data property; however, when I go to access that property in then handlebars template it is not the value of true. It seems to be linked to the access of the @index in an each block helper.

I presume the problem is in: handlebars.js/lib/handlebars/helpers/each.js however, I don't know enough about the expected params to really grok the problem.

[nknapp]

I would be helpful if you could separated this problem from gulp-metalsmith can you reproduce the behaviour with the jsfiddle linked in CONTRIBUTING.md.

The would be helpful for examining the problem.

[janthonyeconomist]

I think this problem is because how consolidate implements handlebars. If you look at node_modules/consolidate/lib/consolidate.js there is a function:

exports.handlebars.render = function(str, options, fn) {
  return promisify(fn, function(fn) {
    var engine = requires.handlebars || (requires.handlebars = require('handlebars'));
    try {
      for (var partial in options.partials) {
        engine.registerPartial(partial, options.partials[partial]);
      }
      for (var helper in options.helpers) {
        engine.registerHelper(helper, options.helpers[helper]);
      }
      var tmpl = cache(options) || cache(options, engine.compile(str, options));
      fn(null, tmpl(options));
    } catch (err) {
      fn(err);
    }
  });
};

It is passing the data object into the compile method.

You can see it reproduced here: https://jsfiddle.net/9D88g/105/
And it works without the @Index here: https://jsfiddle.net/9D88g/106/

[janthonyeconomist]

Should I move this ticket over to the "consolidate" people?

[nknapp]

I'll have a look. Let me see if I understand what your problem ist.

[nknapp]

No, keep it here. I think you did a pretty good job extracting the problem and it definitely looks wrong to me.
You are expecting the output

0: foo
1: bar

correct?

[janthonyeconomist]

yeah. (with the obvious exception of the line break)

[nknapp]

It works, if you change "data" to "datax" (https://jsfiddle.net/9D88g/111/)
Just in case this helps, but I'm still investigating

[janthonyeconomist]

Thanks.

Luckily I discovered this before I even created the ticket. The problem seems to be the very specific property data. That is why I assumed the problem was in handlebars.js/lib/handlebars/helpers/each.js, as it uses that property name when setting up the context of a loop. This might also explain why the problem only occurs when you use the @index variable in a loop.

[nknapp]

Well, may be it is something for the consolidate people, maybe not. There room to argue...
In fact, I have just understood what you said in your comment

It is passing the data object into the compile method.

Yes, that is problematic because of this piece of code. Now, I don't fully understand why consolidate is passing the data-object to the compile-method and I would say you shouldn't do that.

On the other hand, it can be argued that Handlebars.compile should not alter the options object. I'm not sure if that's documented somewhere...

Could you open an issue at consolidate? I'd like to hear (read) their point of view before changing anything at Handlebars.

[nknapp]

Oh, I didn't see your last post... I think I will have a look what could be done for not altering the options during compilation. I don't know the code well enough yet to see if it has any impact on the interface.

But this might take a while and I can't continue right now.

[janthonyeconomist]

I'll raise the issue to the consolidate people as well and link to this issue.

[janthonyeconomist]

Perhaps the best thing is to clone the options went they are passed in, so that they can be immutable. Not ideal, but might be enough.

I have opened a ticket with consolidate: tj/consolidate.js#280

[nknapp]

I have opened a PR, if anyone would like to comment, please do it there.
I'll leave it open for a day or two and merge it then.

[nknapp]

After a broken 4.0.9-release (because Object.assign is not supported by IE), this issue is now resolved in 4.0.10