haunted icon indicating copy to clipboard operation
haunted copied to clipboard

Boolean attribute behavior confusing

Open sunesimonsen opened this issue 5 years ago • 7 comments

The current boolean attribute behavior might be intentional, but I find it really confusing that something that is used a string attribute turns into a boolean attribute when the value is an empty string:

boolean attribute

It could be that I just need to use some other mechanism that I don't know about. But I just wanted to say that I found this behavior surprising.

I would still expect an attribute without any value to be true and a missing attribute to be undefined when retrieving the value.

The code that is doing this conversion can be found here: https://github.com/matthewp/haunted/blob/master/src/component.js#L34

sunesimonsen avatar Mar 20 '19 07:03 sunesimonsen

An attribute without an empty value has a value of an empty string. This is how we detect boolean attributes.

<my-el attr>
myEl.getAttribute('attr') === '';

You probably don't want to bind an input's value directly to an attribute. Instead you might want to convert empty strings to false.

matthewp avatar Mar 20 '19 11:03 matthewp

Actually, I guess the problem you're seeing is that it was a string attribute but becomes a boolean attribute. I guess we could maybe fix this by checking the old value and if the old value was a string then leave it as a string. Would that fix your issue?

matthewp avatar Mar 20 '19 11:03 matthewp

Actually, I guess the problem you're seeing is that it was a string attribute but becomes a boolean attribute. I guess we could maybe fix this by checking the old value and if the old value was a string then leave it as a string. Would that fix your issue?

Yes that is the problem I'm seeing, but I think the fix sounds a little magical. I guess that is why lit-element has a concept the property type: https://lit-element.polymer-project.org/guide/properties

Let me try to think about it, I'll get back to you soon.

sunesimonsen avatar Mar 20 '19 16:03 sunesimonsen

By the way - the library it really awesome, exactly the right level of convenience, size and power.

sunesimonsen avatar Mar 20 '19 16:03 sunesimonsen

There's an issue https://github.com/matthewp/haunted/issues/74 to discuss property types and coercion, etc. I'm just thinking about what is the right solution with the way Haunted currently works.

matthewp avatar Mar 20 '19 17:03 matthewp

@matthewp I think something like property descriptors would be appropriate. But I would probably try to make it simpler than lit-element that is becoming quite bloated. So I would probably just have a collection of property descriptors that take a name and returns a descriptor that can do the coercion:

import { html } from 'https://unpkg.com/lit-html/lit-html.js'

import { 
  component, 
  stringProperty,
  integerProperty,
  enumProperty
} from "https://unpkg.com/[email protected]/haunted.js";

const User = ({ name, age, type }) => html`
  <div class="user ${type.toLowerCase()}">${name} ${age}</div>
`;

User.properties = [
  stringProperty('name'),
  integerProperty('age', 0, 120),
  enumProperty('type', ['ADMIN', "END_USER"])
]

customElements.define("x-user", component(User));

If you want more you can always pass data with the property binding in lit-html if that is what you want: <x-greeting .user=${user}></x-greeting>. (You probably know that 😄)

Feel free to close this issue - you can also take the concerns into the property discussion.

Sorry if I'm just rambling :-)

sunesimonsen avatar Mar 20 '19 19:03 sunesimonsen

Keeping this issue open for now as I think making the property not be switched from a string to a boolean is a good way to go. Property discussion will go in the other issue. Thanks for commenting!

matthewp avatar Mar 20 '19 20:03 matthewp