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

"falsey" conditional {?.x} is not treating 0 as false #180

Closed
billinghamj opened this issue Oct 17, 2012 · 14 comments
Closed

"falsey" conditional {?.x} is not treating 0 as false #180

billinghamj opened this issue Oct 17, 2012 · 14 comments

Comments

@billinghamj
Copy link

As per #127 (comment), heavily simplified

Using the tool with this for the template:

{?.key}
a
{:else}
b
{/.key}

And this for the JSON:

{"key": 0}

Output I'm getting:

a

Output I'm expecting to get:

b

Occurring the same in both of the following tools:
http://akdubya.github.com/dustjs/
http://linkedin.github.com/dustjs/test/test.html

For reference, I have tested in Safari on OS X and Chrome on Windows.

The following "falsey" values give the correct response - b:

{}
{"key": null}
{"key": false}
{"key": ""}

The integer value 0 (in Javascript) is also considered as "falsey", as per the following:

(0 == false) returns true

Whether intentional or not, I feel this is a bug because zero is a "falsey" value.

@billinghamj
Copy link
Author

https://github.com/linkedin/dustjs/blob/master/dist/dust-core-1.1.1.js#L448

That's the code which does it ^ Edit: no it's not - sorry

Inclined to think that it's incorrect. Any cons to it treating 0 as false?

@vybs
Copy link
Contributor

vybs commented Oct 17, 2012

Chunk.prototype.exists = function(elem, context, bodies) {
var body = bodies.block,
skip = bodies['else'];

if (!dust.isEmpty(elem)) {
if (body) return body(this, context);
} else if (skip) {
return skip(this, context);
}
return this;
};

it is the isEmpty method.

dust.isEmpty = function(value) {
if (dust.isArray(value) && !value.length) return true;
if (value === 0) return false;
return (!value);

@vybs
Copy link
Contributor

vybs commented Oct 17, 2012

it is the .notation that trips it, else it works fine for me ( so please update your the problem the same code example as before.

@billinghamj
Copy link
Author

I'm getting exactly the same result with and without the dot before "key", so stripped it out to simplify as much as possible. Will re-add anyway.

@vybs
Copy link
Contributor

vybs commented Oct 17, 2012

if (value === 0) return false;

this is the code ( hence it returns false for isEmpty )

Change this line of code, lot of unit tests fail !

@billinghamj
Copy link
Author

Therefore, it is not a "falsy" conditional as stated in the docs.

@vybs
Copy link
Contributor

vybs commented Oct 17, 2012

Good point. that

if you try {#key} , it treats it falsey I bet?

@billinghamj
Copy link
Author

Doesn't seem to. Seems to be the same.
http://akdubya.github.com/dustjs/

{#key}
a
{:else}
b
{/key}
{
  "key": 0
}

again returns

a

Same with dots.

@vybs
Copy link
Contributor

vybs commented Oct 17, 2012

ok, that makes me a little happy ! , atleast it is consistent. Should we just update the doc?

I did spend some time with unit tests for these, let me dig around a bit later today.

@billinghamj
Copy link
Author

Hmm... the problem here is that this makes it impossible to use conditionals based on zero values. The only way to make it evaluate as false is to remove the value altogether.

In languages where you are directly converting a static type to JSON (like C#), this is rather difficult to do.

For example:

{ id: 1, rating: 2 }
{ id: 2, rating: 0 }

Say your app uses 0 to represent a lack of rating. The only way to make dust understand this is:

{ id: 1, rating: 2 }
{ id: 2 }

But when your model is this:

class Entity
{
    int id {get;set;}
    int rating {get;set;}
}

You can't just remove one of the properties. It's not feasible. Therefore, I think it's important to allow for checking for zero values.

@billinghamj
Copy link
Author

That said, arguably, a nullable int would be more appropriate. Though I think there are still cases where conditionals around 0 values would be useful.

@vybs
Copy link
Contributor

vybs commented Nov 13, 2012

here are two other special section notations that provide conditional testing:

{?name/}
not only tests the existence of name in the current context, but also evaluates the value of name in the JSON model
{^name/}
not only tests the non-existence of name in the current context, but also evaluates the value of name in the JSON model
Note that the value of name is evaluated as follows -> "" or ' ' will evaluate to false, boolean false, null, or undefined will evaluate to false, numeric 0 evaluates to true, so does, string "0", string "null", string "undefined" and string "false". Also note that empty array -> [] is evaluated to false and empty object -> {} and non-empty object are evaluated to true.

@vybs vybs closed this as completed Nov 13, 2012
@vybs
Copy link
Contributor

vybs commented Nov 13, 2012

wiki has the details already, hence closing :https://github.com/linkedin/dustjs/wiki/Dust-Tutorial#wiki-Logic_in_Templates

@dominykas
Copy link

"numeric 0 evaluates to true" <-- I do not think this is intuitive. What makes this hurt even more is that there are no keywords for true/false that we could pass as parameters - meaning that the only universal falsy value (other than non-existence/undefined) is "" (empty string).

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