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

Empty htmlSafe string is treated as truthy #17486

Closed
buschtoens opened this issue Jan 16, 2019 · 6 comments · Fixed by #18148
Closed

Empty htmlSafe string is treated as truthy #17486

buschtoens opened this issue Jan 16, 2019 · 6 comments · Fixed by #18148

Comments

@buschtoens
Copy link
Contributor

The {{if}} (and {{unless}}, {{with}}) template helpers treat an empty htmlSafe string as truthy, while an empty string is treated as falsy and even an empty array is treated as falsy.

{{if ''             'true' 'false'}} => false
{{if (html-safe '') 'true' 'false'}} => true
{{if (array)        'true' 'false'}} => false

Ember Twiddle

Interestingly I do not understand why an empty array is treated as falsy, when looking at the code:

compute() {
let branch = this.cond.value() ? this.truthy : this.falsy;
this.branchTag.inner.update(branch.tag);
return branch.value();
}

@rwjblue
Copy link
Member

rwjblue commented Jan 16, 2019

Empty arrays are a falsey value in Handlebars across the board ( see https://handlebarsjs.com/builtin_helpers.html for example).

I haven’t really thought much about the empty safe string scenario though. I’d have to check what Handlebars does there...

@buschtoens
Copy link
Contributor Author

buschtoens commented Jan 16, 2019

Empty arrays are a falsey value in Handlebars across the board

Sure! Sorry for not expressing myself clearly enough. 😊

I know that [] is treated as false in Handlebars. I just don't understand how the compute method treats [] as false, since the ternary in L42 would treat [] as true.

I brought it up because there where this exception is implemented, we could also implement the exception for empty htmlSafe strings.

@chancancode
Copy link
Member

@buschtoens this.cond is wrapped to have the ember semantics (and return a JS boolean, not the raw value)

@chancancode
Copy link
Member

I think this is probably a bug.

https://github.com/emberjs/ember.js/blob/master/packages/%40ember/-internals/glimmer/lib/utils/to-bool.ts

(not sure what's up with that indentation)

@pzuraq
Copy link
Contributor

pzuraq commented Mar 29, 2019

This does seem like a bug, htmlSafe should be treated as transparent wrapper to strings conceptually, and an empty htmlSafe string should be falsy

@chancancode
Copy link
Member

If anyone is interested in working on a fix, I believe adding a case to the function I linked above should fix it.

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

Successfully merging a pull request may close this issue.

5 participants