Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Each not pushing on a context level #57

Open
charleso opened this issue Jun 9, 2016 · 11 comments
Open

Each not pushing on a context level #57

charleso opened this issue Jun 9, 2016 · 11 comments

Comments

@charleso
Copy link
Contributor

charleso commented Jun 9, 2016

{{#each foo}}
  {{#each bar}}
    {{../title}}
  {{/each}}
{{/each}

../title will blow up, but title works fine

@charleso charleso changed the title Each not push the context correct Each not pushing on a context level Jun 9, 2016
@damncabbage
Copy link
Contributor

damncabbage commented Jun 9, 2016

Uh, this is missing some context. Can you state the inputs to this template?

Edit: I'm assuming a structure like:

{
  "foo": [
    { "title": "hi", "bar": [{ "ignored": true }] }
  ]
}

@damncabbage
Copy link
Contributor

damncabbage commented Jun 10, 2016

AFAICT it's missing a withContext that would change the scope. I can't even get your example to run (to reach the ../title error) without doing this instead:

{{#each foo as |f| }}
  {{#each f.bar as |b| }}
    {{ f.title }}
    {{ ../title }}
  {{/each}}
{{/each}}

Note! My example above works on tryhandlebars. Yes, that's right: it's scoped (../title), but not really (f; ../f doesn't work, FYI). Goddamnit.

@damncabbage
Copy link
Contributor

damncabbage commented Jun 10, 2016

Shorter failing case we can't currently work around AFAIK (because a workaround for BMX would break Handlebars version, and vice-versa):

{{ title }}
{{#each foo }}
  {{ ../title }}
{{/each}}
{
  "title": "hi",
  "foo": [{}]
}

@charleso
Copy link
Contributor Author

Shorter failing case we can't currently work around AFAIK

Yeah, sorry your last test case is actually the one I hit (so couldn't even assign a name to the other context). Thanks for filling in more details.

@thumphries
Copy link
Contributor

I don't follow, why would each push a new context? all it does is define 'this' and some data variables

What does handlebars do?

@thumphries
Copy link
Contributor

AFAICT it's missing a withContext

yep you can just wrap it in withContext and it should do what you want

@damncabbage
Copy link
Contributor

What does handlebars do?

each in Handlebars pushes a new context. BUT, for some reason, I can also see the named params, as if they were in the same scope. It makes no damn sense.

Take this example that I tried on tryhandlebars:

{{#each foo as |f| }}
  {{#each f.bar as |b| }}
    {{#each b.quux as |c| }}
      -{{ f.title }}-      {{!-- ../../f.title doesn't work --}}
      ={{ ../../title }}=  {{!-- title doesn't work, neither does ../title --}}
    {{/each}}
  {{/each}}
{{/each}}
{ "foo": [
    {
      "title": "hi",
      "bar": [
        { "quux": [{}] }
      ]
    }
]}

@markhibberd
Copy link
Contributor

So I think an even simpler case that I just hit was:

{
  foo: [{"bar": "bar", "baz":"baz"}]
}
{{#each foo}}
  {{bar}}
  {{baz}}
{{/each}}

@damncabbage
Copy link
Contributor

damncabbage commented Jun 19, 2016

That's definitely a simple way to demonstrate the not-pushing-a-scope-ness. The real kicker about this bug is that we're damned if we push a scope (an as |x|'s x variable will be in the wrong scope) and damned if we don't (the ../'d and implicit-this [your example] references don't work).

@markhibberd
Copy link
Contributor

@damncabbage Interestingly I use an explicit this successfully in a non-bikeshed context, just using bmx https://github.com/ambiata/boris/blob/master/boris-http/template/builds.hbs#L32-L36 I am kind of surprised that works when others fail.

@thumphries
Copy link
Contributor

Just confirming what y'all have probably figured out by now:

The current implementation of each only sets this, @first, @last, @index, @key. Doesn't do anything else. so in the example above I'd expect this to be used:

{{each foo}}
  {{this.bar}}
  {{this.baz}}
{{/each}}

This is a mistake, a bug for sure. Anything that gets rid of this or .. in everyday use is an improvement.

I think we should stick to a definition that keeps us mustache-compliant, i.e. fix the JS block parameter scoping insanity and then make BMX push a context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants