Did you get rid of #if statements with conditional helpers? #421

Closed
chrisabrams opened this Issue Jan 29, 2013 · 6 comments

Comments

Projects
None yet
4 participants
@chrisabrams

If I pass in:

{{#if myFunHelper}}
<some html>
{{/if}}

Even if all myFunHelper does is return true or false the result is undefined.

@kpdecker

This comment has been minimized.

Show comment Hide comment
@kpdecker

kpdecker Jan 30, 2013

Collaborator

Do you have an example? jsbin?

Collaborator

kpdecker commented Jan 30, 2013

Do you have an example? jsbin?

@davidkaneda

This comment has been minimized.

Show comment Hide comment
@davidkaneda

davidkaneda Jan 30, 2013

To follow up on this, it's actually helpers that we were relying on (I work with Chris).

In previous versions we seemed to be able to do something like (excuse my CS):

Handlebars.registerHelper 'hasTitleOrUrl', -> @title? || @url?

Then, within our templates, we could mix that helper with conditionals, ala:

{{#if hasTitleOrUrl}} Do something {{else}} Do something else {{/if}}

We could also easily do:

{{#unless hasTitleOrUrl}} Do something {{/unless}}

Which was convenient...

Now, it doesn't appear helpers are called when combined with a conditional, so we instead have to write:

Handlebars.registerHelper 'hasTitleOrUrl', (options) ->
    if @title? || @url?
        options.fn @
    else
        options.inverse @

Likewise, if we want a negated check (unless) out of the box, we have to either do this:

{{#hasTitleOrUrl}}{{else}}Do something{{/if}}

or create a whole other helper:

Handlebars.registerHelper 'doesNotHaveTitleOrUrl', (options) ->
    if @title? || @url?
        options.inverse @
    else
        options.fn @

Does that make sense?

To follow up on this, it's actually helpers that we were relying on (I work with Chris).

In previous versions we seemed to be able to do something like (excuse my CS):

Handlebars.registerHelper 'hasTitleOrUrl', -> @title? || @url?

Then, within our templates, we could mix that helper with conditionals, ala:

{{#if hasTitleOrUrl}} Do something {{else}} Do something else {{/if}}

We could also easily do:

{{#unless hasTitleOrUrl}} Do something {{/unless}}

Which was convenient...

Now, it doesn't appear helpers are called when combined with a conditional, so we instead have to write:

Handlebars.registerHelper 'hasTitleOrUrl', (options) ->
    if @title? || @url?
        options.fn @
    else
        options.inverse @

Likewise, if we want a negated check (unless) out of the box, we have to either do this:

{{#hasTitleOrUrl}}{{else}}Do something{{/if}}

or create a whole other helper:

Handlebars.registerHelper 'doesNotHaveTitleOrUrl', (options) ->
    if @title? || @url?
        options.inverse @
    else
        options.fn @

Does that make sense?

@chrisabrams

This comment has been minimized.

Show comment Hide comment
@chrisabrams

chrisabrams Jan 30, 2013

I don't think they can see the PR that was referenced since it is private. I'm happy to share some of the code if needed, but you did a good job in pointing out the issues.

I don't think they can see the PR that was referenced since it is private. I'm happy to share some of the code if needed, but you did a good job in pointing out the issues.

@davidkaneda

This comment has been minimized.

Show comment Hide comment
@davidkaneda

davidkaneda Jan 30, 2013

Lol, I didn't mean to actually reference it... just wanted to share the link internally-

Lol, I didn't mean to actually reference it... just wanted to share the link internally-

@wagenet

This comment has been minimized.

Show comment Hide comment
@wagenet

wagenet Feb 13, 2013

Collaborator

@chrisabrams, @davidkaneda, If you are able to run a git bisect on Handlebars to find where this stopped working, that would be awesome. Knowing the exact commit would help give some insight into why this behavior was changed. Knowing @wycat's approach to Handlebars, my suspicion is that your relied upon behavior only worked accidentally.

Collaborator

wagenet commented Feb 13, 2013

@chrisabrams, @davidkaneda, If you are able to run a git bisect on Handlebars to find where this stopped working, that would be awesome. Knowing the exact commit would help give some insight into why this behavior was changed. Knowing @wycat's approach to Handlebars, my suspicion is that your relied upon behavior only worked accidentally.

@kpdecker

This comment has been minimized.

Show comment Hide comment
@kpdecker

kpdecker May 29, 2013

Collaborator

It looks like this was introduced here 727eb26.

Restoring support for this adds a fair amount of complexity in the generated output for a usecase that could be handled via something like this:

Handlebars.registerHelper('helper-exists', function(name, options) {
  if (Handlebars.helpers[name]) {
    return options.fn(this);
  } else {
    return options.inverse(this);
  }
});

Then:

{{#helper-exists "hasTitleOrUrl"}}
  {{hasTitleOrUrl}}
{{^}}
  Missing!
{{/helper-exists}}

It's not as nice conceptually as the if based solution above but does not impose a system-wide performance hit (both on exec and precompiled source size) for a very specific feature.

This also gets around the behavior where the if helper is going to attempt to exec the helper. In my attempts to blame this there were many NPEs that I saw from helper incorrectly attempting to execute.

Collaborator

kpdecker commented May 29, 2013

It looks like this was introduced here 727eb26.

Restoring support for this adds a fair amount of complexity in the generated output for a usecase that could be handled via something like this:

Handlebars.registerHelper('helper-exists', function(name, options) {
  if (Handlebars.helpers[name]) {
    return options.fn(this);
  } else {
    return options.inverse(this);
  }
});

Then:

{{#helper-exists "hasTitleOrUrl"}}
  {{hasTitleOrUrl}}
{{^}}
  Missing!
{{/helper-exists}}

It's not as nice conceptually as the if based solution above but does not impose a system-wide performance hit (both on exec and precompiled source size) for a very specific feature.

This also gets around the behavior where the if helper is going to attempt to exec the helper. In my attempts to blame this there were many NPEs that I saw from helper incorrectly attempting to execute.

@kpdecker kpdecker closed this May 29, 2013

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