Remove attribute when set to null/undefined/false

[HugoDF]

Closes #485 #280

Todo:

• test for x-bind:[attribute] on a falsy value still includes the attribute #485
• test for non-standard attribute case? Configurable list of boolean attributes #280 (comment)
• docs update

Feedback requests:

1. consensus to undo this, change should only apply to non bool attrs I've slightly altered the behaviour of boolean attributes set to non-falsy values, they used to be set to '' now they'll be set to whatever value is, which shouldn't make too much of a difference (eg. none of the tests are failing) cc @SimoTod @ryangjchandler
2. consensus to remove on null/undefined/false Should we only remove on "null" or also "undefined" and "false" (per the way Vue.js works)

value of null, undefined, or false, the disabled attribute will not even be included in the rendered <button> element

If we go with Vue's approach we could ditch the whole "Boolean HTML attribute" detection

[ryangjchandler]

I wonder if now would be a good time to actually introduce a Boolean modifier?

[HugoDF]

I wonder if now would be a good time to actually introduce a Boolean modifier?

Yeah, not in this PR though

I want Caleb to be able to decide which bits to merge and which not to

Boolean modifier is about parsing stuff, Boolean attr are a HTML spec thing.

[ryangjchandler]

I wonder if now would be a good time to actually introduce a Boolean modifier?

Yeah, not in this PR though

I want Caleb to be able to decide which bits to merge and which not to

Boolean modifier is about parsing stuff, Boolean attr are a HTML spec thing.

[ryangjchandler]

I wonder if now would be a good time to actually introduce a Boolean modifier?

Yeah, not in this PR though

I want Caleb to be able to decide which bits to merge and which not to

Boolean modifier is about parsing stuff, Boolean attr are a HTML spec thing.

Yeah for sure. Separate PR.

[HugoDF]

@richgcook what do you think of this implementation?

Do we still think it's worth checking against null/undefined/false? Or is just "null" enough?

[richgcook]

@richgcook what do you think of this implementation?

Do we still think it's worth checking against null/undefined/false? Or is just "null" enough?

Personally I think "null" is enough but I guess it should be any falsy value?

[HugoDF]

@richgcook what do you think of this implementation?

Do we still think it's worth checking against null/undefined/false? Or is just "null" enough?

Personally I think "null" is enough but I guess it should be any falsy value?

Any falsy value except '' and NaN and 0 lol

[richgcook]

@richgcook what do you think of this implementation?
Do we still think it's worth checking against null/undefined/false? Or is just "null" enough?

Personally I think "null" is enough but I guess it should be any falsy value?

Any falsy value except '' and NaN and 0 lol

🤪

[HugoDF]

I think it's sensible to make a stand and say "you have to use the null magic value to get this behaviour"

[SimoTod]

I agree Vue removes the value when false but Alpine is not Vue.
Also I find weird that Vue set the attribute to the string 'true' when the value is true but it removes it when the value is false.
I like the null only approach slightly better.
We just need to write that in the documentation and we should be good to go.

[HugoDF]

@SimoTod only caveat is this one

If we go with Vue's approach we could ditch the whole "Boolean HTML attribute" detection

[HugoDF]

Btw the tests aren't 100% representative since a removed DOM Node attribute, when accessed will return '' and not undefined... classic Web API quirks.

[SimoTod]

@SimoTod only caveat is this one

If we go with Vue's approach we could ditch the whole "Boolean HTML attribute" detection

@HugoDF
I think Vue is still acting differently when dealing with boolean attribute, though.
For example, if you have :readonly="'test'", it prints readonly="readonly"

https://codepen.io/SimoTod/pen/WNQgvqo

I'd be tempted to leave the current behaviour for boolean attributes and just change the last else to 'if null remove it else set value as attribute`

[calebporzio]

Hey @HugoDF, thanks for this. Yes I think we shoud just skip the "boolean" attribute thing for attribute removal right? And just mirror Vue: If undefined, null, or falsy, remove it - right?

[adamwathan]

Regarding the boolean stuff, is there any reason to just not be strict about true and false as values always implying boolean attribute-style behavior? Really the only valid type for any HTML attribute is a string.

<div :turd-sandwich="true">
// => <div turd-sandwich>

<div :turd-sandwich="'true'">
// => <div turd-sandwich="true">

<div :turd-sandwich="false">
// => <div>

<div :turd-sandwich="scrumptious">
// => <div turd-sandwich="scrumptious">

[calebporzio]

Ah, sorry I didn't read @SimoTod's comment - yeah, keeping boolean attribute detection if that's how vue does it applying this new removal behavior otherwise is great: remove on undefined, null, or falsey

[calebporzio]

Yeah @adamwathan, like:

• Is it a string? use the value
• Is it null, undefined, or a boolean? Use the truthy value to toggle the attribute on an off.

Curious how this would apply to Boolean attributes like "selected" or "disabled" though.

[ryangjchandler]

Yeah @adamwathan, like:

• Is it a string? use the value
• Is it null, undefined, or a boolean? Use the truthy value to toggle the attribute on an off.

Curious how this would apply to Boolean attributes like "selected" or "disabled" though.

Boolean attributes behave the same with or without a value, right?

[HugoDF]

Yeah @adamwathan, like:

• Is it a string? use the value
• Is it null, undefined, or a boolean? Use the truthy value to toggle the attribute on an off.

Curious how this would apply to Boolean attributes like "selected" or "disabled" though.

Boolean attributes behave the same with or without a value, right?

No that's the problem
If a Boolean attr has any value (even "") it counts as enabled. So to disable you have to actively remove the attribute.

[HugoDF]

@SimoTod only caveat is this one

If we go with Vue's approach we could ditch the whole "Boolean HTML attribute" detection

@HugoDF
I think Vue is still acting differently when dealing with boolean attribute, though.
For example, if you have :readonly="'test'", it prints readonly="readonly"

https://codepen.io/SimoTod/pen/WNQgvqo

I'd be tempted to leave the current behaviour for boolean attributes and just change the last else to 'if null remove it else set value as attribute`

Yeah so we never had this, up until now Boolean attributes when enabled would be set to ''

[HugoDF]

Ok cool so I'll keep the Boolean attribute logic and add a check for undefined, false in addition to null

[ryangjchandler]

Yeah @adamwathan, like:

• Is it a string? use the value
• Is it null, undefined, or a boolean? Use the truthy value to toggle the attribute on an off.

Curious how this would apply to Boolean attributes like "selected" or "disabled" though.

Boolean attributes behave the same with or without a value, right?

No that's the problem
If a Boolean attr has any value (even "") it counts as enabled. So to disable you have to actively remove the attribute.

Yeah sorry, I meant having the attribute there with a value isn't a problem

[HugoDF]

Ah yes @ryangjchandler although I can half see the "bug report" coming in for it 😂😂😂

[SimoTod]

Yeah so we never had this, up until now Boolean attributes when enabled would be set to ''

True, but our version is actually following the standard.
If you look at the MDN page they say:

Boolean attributes are considered to be true if they're present on the element at all, regardless of their actual value; as a rule, you should specify the empty string ("") in value (some people use the attribute's name; this works but is non-standard). 

If we decide to go with Adam's suggestion, I believe we need to use the empty string since there's no way to programmatically set an attribute without a value.

I still slightly prefer to keep boolean attributes as they are but the other solution is fine if we are happy with that as long as it's consistent and not half-half like vue where true becomes the string 'true' but false remove the attribute (and 0, falsy value, adds the string '0'). :)

Apart from that detail, PR looks good, well done 👍

[HugoDF]

Ok I'll fix it up so that:

• Boolean truthy binding gets set as ''
• null/undefined/false value always remove the attribute (bool or not)

This won't change too much and also gives us the option to remove Boolean attribute detection at some point (but not required)

[HugoDF]

Ok changes are done @SimoTod @calebporzio @adamwathan thanks for the feedback

One for @ryangjchandler re "boolean" modifier, we should also have a "string" modifier so that if someone (for whatever reason) wants to bind attributes to "null", "false", "undefined" they can 😂 (see the model test I had to modify)

[ryangjchandler]

Ok changes are done @SimoTod @calebporzio @adamwathan thanks for the feedback

One for @ryangjchandler re "boolean" modifier, we should also have a "string" modifier so that if someone (for whatever reason) wants to bind attributes to "null", "false", "undefined" they can 😂 (see the model test I had to modify)

Yeah agree with that (I think)

[calebporzio]

Thanks so much, @HugoDF and everyone else who weighed in!

[HugoDF]

Thanks @calebporzio

Did we end up going with disabled="disabled" when a Boolean attribute is on?