Nested each with equal values skips some iterations.

graphicore commented 4 years ago

Before filing issues, please check the following points first:

[ x] Please don't open issues for security issues. Instead, file a report at https://www.npmjs.com/advisories/report?package=handlebars
[x ] Have a look at https://github.com/wycats/handlebars.js/blob/master/CONTRIBUTING.md
[x ] Read the FAQ at https://github.com/wycats/handlebars.js/blob/master/FAQ.md
[x ] Use the jsfiddle-template at https://jsfiddle.net/4nbwjaqz/4/ to reproduce problems or bugs

This will probably help you to get a solution faster. For bugs, it would be great to have a PR with a failing test-case.

Here's the jsfiddle for the issue: https://jsfiddle.net/graphicore/ge1qxvc8/

Template

_{{#each abc as | c1 |}}{{#each ../abc as | c2 |}}{{#each ../../abc as | c3 |}}{{c1}}{{c2}}{{c3}} {{else}}! {{/each}}| {{/each}}{{/each}}_

Data

{
    abc: ['A', 'B', 'C']
}

Observed Behavior

_! | ABA ABB ABC | ACA ACB ACC | BAA BAB BAC | ! | BCA BCB BCC | CAA CAB CAC | CBA CBB CBC | ! | _

Expected Behavior

_AAA AAB AAC | ABA ABB ABC | ACA ACB ACC | BAA BAB BAC | BBA BBB BBC | BCA BCB BCC | CAA CAB CAC | CBA CBB CBC | CCA CCB CCC | _

Context:

I'm porting a font testing tool written mostly in server side PHP to client side JavaScript. For some samples I need to generate grids of char combinations e.g. to let a designer test spacing uc-spacing-03.php.

I believe 279e038ba7ff7e5966a60e36d326e2d4c310f0c6 is related (commit message).

graphicore commented 4 years ago

I added a test and suggested a fix in PR #1687

nknapp commented 4 years ago

Is this a duplicate of #1300 ?

graphicore commented 4 years ago

@nknapp I looked at that issue before, but it's just now that I've noticed your workaround, and indeed it works.

https://jsfiddle.net/graphicore/shn9oyb6/1/

{{#with abc as  | abc |}}
    _{{#each abc as | c1 |}}{{#each abc as | c2 |}}{{#each abc as | c3 |}}{{c1}}{{c2}}{{c3}} {{else}}! {{/each}}| {{/each}}{{/each}}_
{{/with}}

It even gets rid of the ../ parent scope selectors, which I quite like.

I have to admit, the scoping concept of handlebars.js is a bit mysterious to me. If this would work without the {{#with abc as | abc |}} I'd understand the concept as falling back to the parent scope when a name is undefined in the current scope and ../ to make shadowed names from parent scopes accessible, but having to use with to achieve this is unexpected.

FWIW, this remains broken:

{{#with abc as  | abc |}}
_{{#each ../abc as | c1 |}}{{#each ../../abc as | c2 |}}{{#each ../../../abc as | c3 |}}{{c1}}{{c2}}{{c3}} {{else}}! {{/each}}| {{/each}}{{/each}}_
{{/with}}

nknapp commented 4 years ago

Not adding a scope when the context remains the same is something that was introduced in 4.0 and I'm not happy with it either. I actually like your solution but I think it should be completely dependent on the helper and not on whether the context changes. But this would be a breaking change, which is why I haven't attempted to implement it so far.

graphicore commented 4 years ago

but I think it should be completely dependent on the helper and not on whether the context changes. but I think it should be completely dependent on the helper and not on whether the context changes.

I agree, I kept the old condition to stay compatible. It could be removed for version 5, if the project policy allows that.

If it's possible to introduce bigger changes I may suggest another one, in another issue.

handlebars-lang / handlebars.js