Input value set as attribute

[peterwmwong]

Consider the following failing test case...

IncrementalDOM.patch(document.body, function(){
  IncrementalDOM.elementVoid('input', null, null, 'value', 'initial');
});

// Simulates typing some words in the <input>... (same results if you manually type in the input)
document.querySelector('input').value = "NOT RIGHT";

IncrementalDOM.patch(document.body, function(){
  IncrementalDOM.elementVoid('input', null, null, 'value', 'Hello World');
});

Input text is still "NOT RIGHT" and not "Hello World"

Currently, IncrementalDOM decides how to assign an attribute based on the value's type.
Function and Objects are assigned as properties (ex. node.onclick = function(){}). Everything
else is assigned with setAttribute()/removeAttribute. Docs

Unfortunately, setting an <input>'s value (or checked) must be set as a property after the user
types in the input.

Currently, I'm working around the issue by wrapping the attribute's value with new String()...

  IncrementalDOM.elementVoid('input', null, ['type', 'text'], 'value', new String('Hello World'));

... this works as typeof (new String('Hello World')) === 'object'.

Not sure if a change is needed or just requires docs to note a workaround.

Here's the requirebin's for the above testcase and workaround:
http://requirebin.com/?gist=100e687ddffd8de6e59c
http://requirebin.com/?gist=7c30ebcf7613c5afc947

[jridgewell]

React has a lot of code to support these attribute vs property cases, particularly /src/renderers/dom/shared/HTMLDOMPropertyConfig.js#L44-L193. Might be worth porting over?

[NekR]

@jridgewell sounds good. Then I can use it too for jsx html/dom renderers, so one JSX input could be transformed to equivalent (in terms of end tree) HTML, DOM or Incremental DOM output.

[NekR]

We probably can has separate npm package so all can depend on it and this way almost all tests will be one place, + same update for everyone.

[webcss]

Maybe we could test for property existance on the element and read/write from/to this, otherwise use get/setAttribute...? Related to #57

[NekR]

I am curious why setting for example attr('_test', 'val') should be attribute instead of property? I believe setting/accessing property is faster than the attribute.

[jridgewell]

See #45 for a bit more context.

[sparhami]

For value and checked (and any others?), I don't think setting those as a property would be too bad, although it introduces a slight inconsistency. We would also need to introduce a defaultValue/defaultChecked and anyone translating templates would need to know that value should map to defaultValue etc. when compiling.

Setting arbitrary things as properties would definitely be faster, but there already exists a lot of markup (and logic relying on that markup) that uses non data- attributes on non-custom elements. so it is a no-go, at least for now. It would be possible to have some configuration options, but that makes things quite chaotic in the long run.

[NekR]

@sparhami configuration options sounds good, exactly that I thought. Also, about data- attributes--I believe you should treat any dashed attribute as a attribute anyway because there is not useful case to treat them as properties. Although, there are cases for simple names. Consider this example with JSX:

<div class="list-item"
  data-bind="item"
  itemData={ item.data }
>
  ...
</div>

In this case it would be good to translate data-bind attribute to real attribute since it has dash, and translate itemData to element's property. I know that objects already translates to props, but that might be itemIndex with number value, or something other.

[webcss]

We could deliver a dedicated props object together with staticattrs / attrs and allow for properties to be set only through this. All other keyvalues would then be set as attributes. This would offload responsibility to the developer or transformer logic, but would also simplify the whole process of knowing what's a property and what is an attribute.

[mjesun]

I've actually ran into the same problem as @peterwmwong initially mentioned, and I've been thinking about it.

My initial approach was just to change the setAttribute of applyAttr to setting the property. However this caused problems when writing at an <input>: as soon as you change the value property, the cursor position would be altered. It also creates problems with the actual DOM representation (attribute nodes not being changed).

After a few iterations, I've came up with this solution, which I think it's easier to maintain than having a complete map of attributes and their way of being updated. Basically we first update the attribute, and after, if the corresponding property hasn't been updated, we change it as well.

However, I'm pretty sure it has some caveats, and might not fit all cases, but just to give some other options on it. 😃

[mjesun]

So any updates on this? @jridgewell @sparhami

[jridgewell]

as soon as you change the value property, the cursor position would be altered

Doesn't your solution also cause issues?

With the current codebase, you can set mutators.attributes[symbols.all] to do this:

iDOM.mutators.attributes[iDOM.symbols.all] = function(el, name, value) {
    var type = typeof value;
    var propertyName;

    if (PROPERTY_MAP.hasOwnProperty(name)) {
      propertyName = PROPERTY_MAP[name];
    } else {
      propertyName = name;
    }

    if (value === undefined) {
      el.removeAttribute(name);
    } else if ((typeof value !== 'function') && (typeof value !== 'object')) {
      el.setAttribute(name, value);
    }

    // If, after changing the attribute, the corresponding property is not updated,
    // also update it.
    if (el[propertyName] !== value) {
      el[propertyName] = value;
    }
};

[jridgewell]

Also, iDOM.mutators.attributes is a mouthful. Want to change it to just iDOM.attributes (or something else) since we're not putting the DOM notifications in the same namespace?

[sparhami]

Also, iDOM.mutators.attributes is a mouthful. Want to change it to just iDOM.attributes (or something else) since we're not putting the DOM notifications in the same namespace?

Sounds good to me.

[mjesun]

Referenced code (this one) does not cause issues, because, when writing, the value property is altered by the browser, so when checking its value (in line 53), both the input.value and the new value will match, and the property will not be changed (which is what causes the losing focus / unaligning).

On the other side, if you change the value externally, and then you patch, the === will not match, and then the code will assign the new property value to value, thus reflecting the change on the DOM. In other words, we just change attributes and properties only if something else didn't already change it.

Although the code supports mutators now, I would say that the fix should be included in the core of the library, rather than as an additional "plugin"; mainly because I think this is the default behavior you expect. mutators seems for me something more with a "client" perspective, rather than a "library" thing (but maybe I'm wrong).