block params context is incorrect

[dontgitit]

Hi,

It seems helpers require block params if any ancestor helper required a block param. For instance, take this template:

{{#each foo as |fooValue fooIndex|}}
  {{#each bar as |barValue barIndex|}}
    {{fooValue}} {{fooIndex}} {{barValue}} {{barIndex}}
    {{../this}} {{../@index}} {{this}} {{@index}}
  {{/each}}
{{/each}}

with the following data:

{ 
  "foo": ["a", "b", "c"], 
  "bar": ["d", "e", "f"]
}

as expected, this outputs:

a 0 d 0
a 0 d 0
a 0 e 1
a 0 e 1
a 0 f 2
a 0 f 2

b 1 d 0
b 1 d 0
b 1 e 1
b 1 e 1
b 1 f 2
b 1 f 2

c 2 d 0
c 2 d 0
c 2 e 1
c 2 e 1
c 2 f 2
c 2 f 2

now, remove JUST the second block param for bar:

{{#each foo as |fooValue fooIndex|}}
  {{#each bar}}
    {{fooValue}} {{fooIndex}} {{barValue}} {{barIndex}}
    {{../this}} {{../@index}} {{this}} {{@index}}
  {{/each}}
{{/each}}

I would expect the same output as above, except I would expect empty value for barValue and barIndex since they're never set. However, this actually completely messes up the entire context!

a 0
a 0 a 0
a 0
a 0 a 0
a 0
a 0 a 0

b 1
b 1 b 1
b 1
b 1 b 1
b 1
b 1 b 1

c 2
c 2 c 2
c 2 
c 2 c 2
c 2
c 2 c 2

The lines generated by {{fooValue}} {{fooIndex}} {{barValue}} {{barIndex}} are as I would expect. However, the lines generated by {{../this}} {{../@index}} {{this}} {{@index}} are all wrong! Specifically, {{this}} and {{@index}} are referring to the parent block context (rather than the real current context)! It seems like since there was no block context in the second each, it used the parent context as its own, rather than creating a new context with no block parameters.

[dontgitit]

I think this may be suspect? https://github.com/jknack/handlebars.java/blob/master/handlebars/src/main/java/com/github/jknack/handlebars/Options.java#L532-L539

[dontgitit]

ah, actually I think it's this! https://github.com/jknack/handlebars.java/blob/master/handlebars/src/main/java/com/github/jknack/handlebars/Context.java#L96-L98

when the second EachHelper creates its child context (https://github.com/jknack/handlebars.java/blob/master/handlebars/src/main/java/com/github/jknack/handlebars/helper/EachHelper.java#L64), the child context will be a ParentFirst context (because the current context from https://github.com/jknack/handlebars.java/blob/master/handlebars/src/main/java/com/github/jknack/handlebars/helper/EachHelper.java#L60 is a BlockParam context. Why does BlockParam overwrite newChildContext to create a ParentFirst rather than the usual Context?

[jknack]

don't remember exactly, but think is was bc of handlebars.js

[dontgitit]

hmm... are you saying this is expected/correct behavior, and same in handlebars.js? it seems strange that not using block params completely messes up the whole context...

[dontgitit]

it works fine in handlebars.js so i think it's a bug here?

fiddle: https://jsfiddle.net/L3emzo2k/5/

[jknack]

there was a reason.. do me a favor change the childcontext to do a normal Context, not a ParentFirst. Then run the tests, you will see there some odd use cases which were all inherited from handlebarjs (if my memory doesn't fail)

[dontgitit]

i see... this test fails: https://github.com/jknack/handlebars.java/blob/master/handlebars/src/test/java/com/github/jknack/handlebars/BlockParamsTest.java#L70-L87

[jknack]

I'm surprise it is only one, sadly I copied that tests from handlebarsjs tests :S

[dontgitit]

Looking at the test, I'm not quite sure why it expects that output....

it registers the helper goodbyes:

$("goodbyes", new Helper<Object>() {
          int value = 1;

          @Override
          public Object apply(final Object context, final Options options)
              throws IOException {
            if (options.blockParams.size() > 0) {
              return options.apply(options.fn, $("value", "bar"),
                  Arrays.<Object> asList(value++, value++));
            }
            return options.fn($("value", "bar"));
          }
        })

this is the template... i added # value = EXPECTED to what I would think the value should be...

# value = foo
{{#goodbyes as |value|}} # value = 1
  {{#goodbyes}} # value = bar
    {{value}} # bar - why are we expecting 1 here? the goodbyes above just set value to bar... why should its parent block params take precedence?
    {{#goodbyes as |value|}}
        {{value}}
    {{/goodbyes}}
  {{/goodbyes}}
{{/goodbyes}}
{{value}}

going to try to run this in handlebars.js and see what happens... this doesn't seem correct....

[dontgitit]

hmm... you're right.. handlebars.js has this weird behavior.... https://jsfiddle.net/L3emzo2k/6/

[jknack]

:( doesn't make sense, never understand why they did it like this

[dontgitit]

what i find strange though is that the each sample works in handlebars.js but breaks here.... can't figure out why...

[dontgitit]

opened an issue against hbars... https://github.com/wycats/handlebars.js/issues/1484

[dontgitit]

sorry to bring this up again, but do you have any other thoughts/ideas? the behavior here is definitely a bit different than handlebars.js. Here's a sample fiddle: https://jsfiddle.net/ybow1gnr/

As expected, it outputs foo, whereas the same template+input generates top level in handebars.java