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

Feature regression in 1.7.0, logic helpers ignored in select bodies #134

Closed
hexpunk opened this issue Apr 27, 2015 · 9 comments · Fixed by #135
Closed

Feature regression in 1.7.0, logic helpers ignored in select bodies #134

hexpunk opened this issue Apr 27, 2015 · 9 comments · Fixed by #135

Comments

@hexpunk
Copy link

hexpunk commented Apr 27, 2015

Given this template:

{@select key=selectKey}
    {@eq value=1}One{/eq}
    {@eq value=2}
        Two
        {@eq key=testVal value=5}
            Bug!
        {/eq}
    {/eq}
    {@eq value=3}Three{/eq}
{/select}

If selectKey is 2 and testVal is 5, "Two" is rendered but "Bug!" is not.

Here's a jsfiddle to show this in action. http://jsfiddle.net/hf67o6un/

@smfoote
Copy link
Contributor

smfoote commented Apr 27, 2015

What version of Dust are you using? I tried your example template on dustjs.com and it worked as expected, but it certainly doesn't work in your jsFiddle. I will dig into it, but knowing the Dust version may help.

image

@jimmyhchan
Copy link
Contributor

@jimmyhchan
Copy link
Contributor

@jv-PintoBobcat awesome bug report btw.

@smfoote
Copy link
Contributor

smfoote commented Apr 27, 2015

The issue is in getSelectState, which returns an outer select state for any nested @eq or @ne, etc. If the selectState is determined to be true, then the nested @eq, @ne, etc. are always ignored (see lines 110 and 111 of lib/dust-helpers.js.

Because a nested eq helper doesn't change the context, I'm not sure what the best way to get around this will be. @sethkinast any ideas?

@hexpunk
Copy link
Author

hexpunk commented Apr 27, 2015

As @jimmyhchan indicated, I was using dust 2.7.0. However, the bug still occurs using dust 2.6.2. The issue is definitely due to a change between helpers 1.6.3 and 1.7.0.

@smfoote, the dustjs.com site is still using dust 2.6.2 and helpers 1.6.2.

@sethkinast
Copy link
Contributor

I thought we had a unit test for this. I'll fix it up.

On Mon, Apr 27, 2015, 9:49 AM jv-PintoBobcat [email protected]
wrote:

As @jimmyhchan https://github.com/jimmyhchan indicated, I was using
dust 2.7.0. However, the bug still occurs using dust 2.6.2. The issue is
definitely due to a change between helpers 1.6.3 and 1.7.0.

@smfoote https://github.com/smfoote, the dustjs.com site is still using
dust 2.6.2 and helpers 1.6.2.


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

@sethkinast
Copy link
Contributor

Caused by changing the behavior for this issue:

{@select key=cherry}
  {@eq value="true" type="boolean"}cherry{/eq}
  {@eq key=berry value="true" type="boolean"}berry{/eq}
{/select}

@sethkinast
Copy link
Contributor

Thanks for the great bug report! We should have 2.7.1 and 1.7.1 out this week.

sethkinast pushed a commit to sethkinast/dustjs-helpers that referenced this issue Apr 27, 2015
sethkinast pushed a commit to sethkinast/dustjs-helpers that referenced this issue Apr 27, 2015
sethkinast pushed a commit to sethkinast/dustjs-helpers that referenced this issue Apr 27, 2015
@sethkinast
Copy link
Contributor

1.7.1 has been released to npm.

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

Successfully merging a pull request may close this issue.

4 participants