svelte icon indicating copy to clipboard operation
svelte copied to clipboard

Inconsistent element attributes when value is undefined

Open hibiscuscoffee opened this issue 3 years ago • 4 comments

Describe the bug

The docs say that:

Boolean attributes are included on the element if their value is truthy and excluded if it's falsy. All other attributes are included unless their value is nullish (null or undefined).

I encountered two cases where this is not the case. A Button component with an optional value prop might be defined as:

<!-- Button.svelte -->
 <script>
   export let value = undefined;
 </script>

<button {value}></button>

and will render:

 <button value="undefined"></button>

The same is true for the new style directive:

<!-- Icon.svelte -->
<script>
export let name = "heart";
export let color = undefined;
</script>

<svg style:--icon-color={color}>
<use href="#icon-{name}" />
</svg>

will result in:

<svg style="--icon-color: undefined;"> 
  <use href="#icon-heart" />
</svg>

The expected behavior in both cases would be that the attributes are not added to the element at all when they are undefined. It seems like some values/attributes are always stringified. I'm not entirely sure if this behavior is intended. If it is then it should probably be clarified in the docs and if it is not then we might want to fix this.

Reproduction

https://svelte.dev/repl/ca1b34d3cd454b01ad15aa3e5e025caf?version=3.46.2

Logs

No response

System Info

System:
    OS: Linux 5.4 Ubuntu 20.04.3 LTS (Focal Fossa)
    CPU: (8) x64 Intel(R) Core(TM) i3-10100 CPU @ 3.60GHz
    Memory: 480.74 MB / 15.57 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 16.13.2 - /usr/bin/node
    npm: 8.1.2 - /usr/bin/npm
  Browsers:
    Chrome: 97.0.4692.71
    Firefox: 95.0.1

Severity

annoyance

hibiscuscoffee avatar Jan 15 '22 18:01 hibiscuscoffee

What would you expect to happen if you did:

<script>
    let value = undefined;
</script>

<div value={value} />

ghostdevv avatar Jan 15 '22 20:01 ghostdevv

Personally I would prefer it to be consistent (if that's possible) so no matter the element or attribute I would expect a value of undefined to always remove the attribute entirely.

<div />

ghost avatar Jan 15 '22 22:01 ghost

I am currently experiencing similar issue while using aria-current and disabled attributes. I'd like to test values of both against null (i.e. non-present) and not against false.

Just a side note: Vue.js in version 3 makes a good move to normalize boolean attribute values so the list of such is out of maintaining scope and developers' experience is somewhat more consistent.

Please pardon my ignorance if I've missed something.

EDIT: OK, disregard this. I've found out, svelte was kindly teaching me not to apply disabled to an anchor. I'd only wish I have found it out earlier without a riddle why the server-side render applies disabled as expected and I get disabled="false" in browser right after hydration.

pooledge avatar Feb 03 '22 14:02 pooledge

I'm having similar problems when using the <progress> element (and attempting to utilize the indeterminate state of progress bars).

<script>
	export let max = 1;
	export let value = null;
</script>
<progress {max} {value}/>

renders to:

<progress max="1" value="0"/>

so instead I needed the workaround of:

<script>
	export let max = 1;
	export let value = null;
</script>
{#if value === null}  
	<progress {max}/>
{:else}
	<progress {max} {value}/>
{/if}

Roger322 avatar May 19 '22 10:05 Roger322

I looked into this a bit and it appears that this is because the value attribute is handled specially. Instead of being removed for undefined, null or false like other attributes the value is set directly via HTMLElement.value = . This is apparently do to support binding to the value more naturally for things like input elements however it causes the issues mentioned above. For example if trying to remove the value attribute for the progress element (very useful when doing something indeterminate like establishing a connection before starting a transfer) null and false just get assigned to value which is coerced to 0 which is not what is intended. undefined raises an exception "Failed to set the 'value' property on 'HTMLProgressElement': The provided double value is non-finite.". What I expect is for the attribute to be removed like other attributes would be. This special casing of value can be very surprising.

Of course it is too late to go back and use something like $value to handle the special-case value. But maybe this can be updated to only be done on a whitelist of elements such as input to avoid the problems seen with button and progress.

kevincox avatar Oct 26 '22 18:10 kevincox