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

Parent path continues to drill down depth with multiple conditionals #583

Closed
joshpangell opened this issue Jul 26, 2013 · 9 comments
Closed

Comments

@joshpangell
Copy link

There is some confusion with depth. The parent path behaves differently when in a loop and when displaying a single piece of data. Additionally, the way that depth is created is not very clear.

The expected result of nesting one conditional within another is that the depth does not increase. If a loop was nested in a loop, then the increase of depth would make sense.

Here is an example of how depth works in a loop. When referencing the model value of options it is expected to use the ../ context to reach the root model. However, when nesting an additional (same model) conditional, it is unexpected to travel further down the depth to the ../../ context. Finally, once we've reached our final depth, the looped model still uses the root context (expected), but the same options model, when used as a value needs to have a context one level further down to ../../../, this is also unexpected.

{{#persons}}
    <li>
        {{#if ../options.show_first_name}} <!-- one level -->
            {{#if ../../options.show_last_name}}  <!-- two levels -->
                {{#if ../../../options.show_phone_number}} <!-- three levels -->
                    {{last_name}}, {{first_name}} {{phone_number}}<!-- first level -->
                    <br>Show phone number? {{../../../../options.show_phone_number}}<!-- four levels? -->
                {{/if}}
            {{/if}}
        {{/if}}
    </li>
{{/persons}}

Here is an example of a single model with nested conditionals. Like the looped model, each nested conditional is required to drill one level deeper in it's context. However, unlike the looped mode, once we reach the final depth, the options model can be used at the root depth.

{{#if options.show_first_name}}<!-- top level -->
    {{#if ../options.show_last_name}}  <!-- one level -->
        {{#if ../../options.show_phone_number}}<!-- two levels -->
            {{single.last_name}}, {{single.first_name}} {{single.phone_number}}<!-- top level -->
            <br>Show first name? {{options.show_first_name}}<!-- top level? -->
        {{/if}}
    {{/if}}
{{/if}}

Here is a fiddle with both of these examples outlined.

I believe the ideal solution for this would be to keep the depth all in the same context. As an example, this would be loop:

{{#persons}}
    <li>
        {{#if ../options.show_first_name}}
            {{#if ../options.show_last_name}} 
                {{#if ../options.show_phone_number}}
                    {{last_name}}, {{first_name}} {{phone_number}}
                    <br>Show phone number? {{../options.show_phone_number}}
                {{/if}}
            {{/if}}
        {{/if}}
    </li>
{{/persons}}

And this a single

{{#if options.show_first_name}}
    {{#if options.show_last_name}}
        {{#if options.show_phone_number}}
            {{single.last_name}}, {{single.first_name}} {{single.phone_number}}
            <br>Show first name? {{options.show_first_name}}
        {{/if}}
    {{/if}}
{{/if}}
@azee
Copy link

azee commented Oct 21, 2013

Agree with the issue. Really annoying.

@kpdecker
Copy link
Collaborator

kpdecker commented Nov 3, 2013

Each helper controls the context that it uses. For if the same context is reused. Basically when executing if, both {{bar}} and {{../bar}} hit the same data, the main difference being that the former is harder to maintain and has more overhead.

http://jsfiddle.net/9mhVF/

Closing this as the templates that you listed in the requested section should work fine. If they do not then feel free to post a failing jsfiddle and we can reopen.

@kpdecker kpdecker closed this as completed Nov 3, 2013
@kzap
Copy link

kzap commented Jul 20, 2015

really annoying

@benjamine
Copy link

what about making children scopes inherit all members of ancestors? that would make this problem go away, and it's easy to implement using javascript object prototype.

@kpdecker
Copy link
Collaborator

kpdecker commented Feb 8, 2016

This behavior was changed under 4.0. See #1028

@benjamine
Copy link

@kpdecker from what I understand the change is now .. will keep looking upwards, but my suggestion is to get rid of .. altogether, which I think is what others in the mustache family do too.

I want to do:

data = {
  baseUrl: 'site.com/',
  pages: [
    { url: 'faq' },
    { url: 'contact' },
  ]
}

var template = "{{#each pages}}    {{baseUrl}}{{url}}    {{/each}"

and have it work as expected, without having to specify `{{../baseUrl}}``, that easily achievable if each context inherits from its parent.

@kpdecker
Copy link
Collaborator

kpdecker commented Feb 8, 2016

That particular behavior was implemented quite a while ago but is behind a
flag as it has significant performance impact. See the compat flag here
http://handlebarsjs.com/reference.html.

Many use cases don't need this level of performance, so each project can
make the decision on if the tradeoffs are worth it for each case.

On Mon, Feb 8, 2016 at 3:38 PM Benjamín Eidelman notifications@github.com
wrote:

@kpdecker https://github.com/kpdecker from what I understand the change
is now .. will keep looking upwards, but my suggestion is to get rid of ..
altogether, which I think is what others in the mustache family do too.

I want to do:

data = {
baseUrl: 'site.com/',
pages: [
{ url: 'faq' },
{ url: 'contact' },
]
}

var template = "{{#each pages}} {{baseUrl}}{{url}} {{/each}"

and have it work as expected, without having to specify {{../baseUrl}}`,
that easily achievable if each context inherits from its parent.


Reply to this email directly or view it on GitHub
#583 (comment)
.

@benjamine
Copy link

oh nice! compat? sweet.
I see apparently this is not implemented using object prototype in which case cost would be insignificant, unfortunately I don't understand enough of the source code, but it's probably a huge refactor.
thanks!

@kpdecker
Copy link
Collaborator

kpdecker commented Feb 8, 2016

I looked into proto for the cases where it was possible (most aren't
because we are dealing with arbitrary objects, not custom objects that we
control end to end) as well as a few other cases while implementing this to
try to optimize. The implementation that is there is the fastest, at least
for the engines that were available for test at the time of writing.

On Mon, Feb 8, 2016 at 3:50 PM Benjamín Eidelman notifications@github.com
wrote:

oh nice! compat? sweet.
I see apparently this is not implemented using object prototype in which
case cost would be insignificant, unfortunately I don't understand enough
of the source code, but it's probably a huge refactor.
thanks!


Reply to this email directly or view it on GitHub
#583 (comment)
.

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

No branches or pull requests

5 participants