Attribute updateValue of undefined renders as value="undefined"

[paulie4]

Description:

Why is the default updateDelegate for a "value" attribute updateValue() instead of updateStringValue()? This causes problems with a keypath whose value is undefined, since that is rendered as value="undefined".

Versions affected:

0.9 build 120

Platforms affected:

all

Reproduction:

https://jsfiddle.net/c4nay9me/

Code:

var r = new Ractive({
  el: '#container',
  template: '<custom-element value="{{value}}"></custom-element>',
  data: {
    value: undefined
  }
});

[evs-chris]

That's a fair question. Does anyone know if there's a good way to detect string vs value attributes on a given element? I think at this point it's largely just hard coded around form elements.

[paulie4]

I thought every time node.setAttribute('attr-name', someVal) was called, the browser would always convert it to a string (except for special boolean, on/off attributes), so I'm not sure what your "string vs value attributes" distinction means. Note: it looks like updateValue() is the only update* method in this file that does not use the safeToStringValue() method:
https://github.com/ractivejs/ractive/blob/dev/src/view/items/element/attribute/getUpdateDelegate.js

[evs-chris]

I suppose any non-input value attributes should probably just use updateAttribute, which could be accomplished by moving the return for that section to the inside of the preceding conditional. There are input types where a non-string value is used, but that code predates web components where other elements might have a useful value attribute.

[paulie4]

Do you know of an example input whose "value" attribute is not a string? I thought the browser would put the non-string value in the input.value property but that input.getAttribute('value') would always return a string. MDN's docs on setAttribute() and getAttribute() sound like an element's attribute value is always a string (or sometimes null). This webcomponents issue was closed with this comment: "Attributes are strings. Making them something else would make them no longer attributes. You can use properties (and attributes that reflect them) to store other data."

[fskreuz]

Before making any assumptions, there might be some insight on the PR that added the line https://github.com/ractivejs/ractive/pull/2022/files

[evs-chris]

I was conflating the value property and value attribute a bit. I think it's safe to say that we can change updateValue to use safeToStringValue for the setAttribute, as it's going to get stringified either way. As long as the value property setting is correct, everything should continue to work properly.

[paulie4]

Sounds great, thanks!