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

{?exists/} behavior different when using promises #752

Closed
Tracked by #757
samuelms1 opened this issue Nov 7, 2016 · 5 comments
Closed
Tracked by #757

{?exists/} behavior different when using promises #752

samuelms1 opened this issue Nov 7, 2016 · 5 comments
Milestone

Comments

@samuelms1
Copy link
Contributor

samuelms1 commented Nov 7, 2016

My team noticed recently that the {?exists/} behavior is different when promises. The only difference we've seen so far is that if you provide an empty array to an {?exists/} block that it will skip over the logic inside unless you are using promises in which case an empty array provided to an {?exists/} block will still cause the content inside to render. If you know this behavior then a work around is simple, but it'd be preferable to have consistent behavior whether or not we are using promises.

I made a jsfiddle to demonstrate both an {?exists/} block with an empty array and an {?exists/} block with an empty array provided by a promise. Both of these should yield the same result but they do not :(

image

@sethkinast
Copy link
Contributor

Thanks for the nice repro case. This is somewhat similar to the ?exists behavior for context helpers, in which the helper itself isn't evaluated for truthiness, but its very existence makes it truthy-- even if the context helper returned something falsy like an empty array.

@jchan @smfoote what do you think? I can definitely see the value of being able to test the true value of the Promise immediately instead of having to {#promise}{?.}yes{/.}{/promise} but is there value in just checking for the existence of the Promise itself?

@sethkinast
Copy link
Contributor

Thinking about it a little more, a Promise doesn't have the side effects a context helper may have so evaluating it earlier just changes where we wait for resolution. I think I'm in favor of making this change.

@samuelms1
Copy link
Contributor Author

I made a pull request to resolve the issue: #753. I'm not very familiar with the code in the repo, but I think this resolves things nicely. Either way, thanks for looking into things!

@samuelms1
Copy link
Contributor Author

@sethkinast are you planning a release soon? I'd like to get this fix via npm

@sethkinast
Copy link
Contributor

Yes there should be a 2.8.0 soon, see #757

@sethkinast sethkinast mentioned this issue Dec 13, 2016
5 tasks
@jimmyhchan jimmyhchan added this to the 2.8 milestone Dec 14, 2016
jrrbru pushed a commit to jrrbru/dustjs that referenced this issue Nov 25, 2019
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

3 participants