handlebars.js icon indicating copy to clipboard operation
handlebars.js copied to clipboard

Nested each with equal values skips some iterations.

Open graphicore opened this issue 5 years ago • 5 comments

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 avatar May 05 '20 01:05 graphicore

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

graphicore avatar May 05 '20 03:05 graphicore

Is this a duplicate of #1300 ?

nknapp avatar May 05 '20 20:05 nknapp

@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}}

graphicore avatar May 06 '20 13:05 graphicore

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.

nknapp avatar May 07 '20 07:05 nknapp

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.

graphicore avatar May 09 '20 10:05 graphicore